[Improvement-17157][Master] Support setting max.concurrent.workflow.instances #17159

tusaryan · 2025-05-01T14:49:20Z

Purpose of the pull request

This Pull Request implements a new configuration option max.concurrent.workflow.instances in the master server, allowing administrators to set a limit on the number of concurrently running workflow instances on a single master. This addresses the issue of potential overload when multiple workflows failover to a single master, as described in #17157.

close #17157

Brief change log

Feature: Implemented a new configuration property, master.server-load-protection.max-concurrent-workflow-instances, to limit concurrent workflows. The master is marked as busy if the number of running workflows is greater than or equal to this value.
Enhancement: Refactored MasterServerLoadProtection to incorporate system resource thresholds based on community feedback. The following configuration properties are now utilized to provide a more robust server protection mechanism:
- maxSystemCpuUsagePercentageThresholds
- maxJvmCpuUsagePercentageThresholds
- maxSystemMemoryUsagePercentageThresholds
- maxDiskUsagePercentageThresholds
Bug Fix: Resolved a NullPointerException that occurred during MasterServer startup by correctly using @Autowired to inject the MasterServerLoadProtection bean into MasterConfig.
Refactor:
- Created MasterServerLoadProtectionConfig to manage the bean's lifecycle and properties via Spring.
- Replaced the use of WorkflowCacheRepository with the IWorkflowRepository interface for better abstraction and testability.

Documentation: Updated the application.yml file and the configuration.md contributor documentation to include all new load protection options with clear descriptions. Example:

master:
  server-load-protection:
    # Master max concurrent workflow instances. When the count reaches or exceeds this value, the master is marked busy.
    max-concurrent-workflow-instances: 100  #Set your desired limit

Replace 100 with any integer value you want as the limit.

Verify this pull request

This change is covered by updated unit tests in MasterServerLoadProtectionTest.java to include comprehensive test cases verifying the new functionality for both the workflow instance limit and the system resource thresholds:

Workflow Limit Test: A dedicated test (isOverloadWithMaxConcurrentWorkflowInstances) verifies the new limit:
- When workflow count (e.g., 5) is less than the limit (e.g., 10), the master is not overloaded.
- When workflow count (e.g., 5) is equal to or greater than the limit (e.g., 5 or 3), the master is overloaded.
System Metrics Test: Tests have been updated to confirm that the master is correctly marked as overloaded when any of the system resource thresholds (CPU, memory, disk) are exceeded.

fix issue: [Improvement][Master] Support set max.concurrent.workflow.instances in master #17157

Pull Request Notice

If your pull request contains incompatible change, you should also add it to docs/docs/en/guide/upgrade/incompatible.md

sonarqubecloud · 2025-05-05T10:38:04Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
96.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

ruanwenjun

LGTM, and it's better to expose the activeWorkflowInstanceCount in MasterHeartBeat.

...c/main/java/org/apache/dolphinscheduler/server/master/config/MasterServerLoadProtection.java

tusaryan · 2025-05-07T21:11:17Z

Can anyone review the changes again

...c/main/java/org/apache/dolphinscheduler/server/master/config/MasterServerLoadProtection.java

ruanwenjun · 2025-05-08T03:57:33Z

.../java/org/apache/dolphinscheduler/server/master/config/MasterServerLoadProtectionConfig.java

+    @PostConstruct
+    public void init() {
+        MasterServerLoadProtection serverLoadProtection = masterConfig.getServerLoadProtection();
+        serverLoadProtection.setWorkflowRepository(workflowRepository);


It's better to inject at the constructor.

Is this resolved? Why mark this as resolved?

...c/main/java/org/apache/dolphinscheduler/server/master/config/MasterServerLoadProtection.java

Copilot

Pull Request Overview

This PR introduces a new configuration option "max.concurrent.workflow.instances" to limit the number of concurrently running workflow instances on a single master. Key changes include adding a new property in the YAML configuration, updating MasterServerLoadProtection to use IWorkflowRepository for counting workflow instances, and incorporating tests to verify overload behavior.

Added test cases in MasterServerLoadProtectionTest.java to validate overload conditions.
Updated MasterServerLoadProtection.java to enforce the new concurrent workflow limit.
Modified MasterServerLoadProtectionConfig.java to inject the IWorkflowRepository dependency.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
dolphinscheduler-master/src/test/java/org/apache/dolphinscheduler/server/master/config/MasterServerLoadProtectionTest.java	Added test cases for the overload behavior with the new configuration option.
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/config/MasterServerLoadProtectionConfig.java	Updated to inject IWorkflowRepository into the load protection component.
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/config/MasterServerLoadProtection.java	Integrated the new "max.concurrent.workflow.instances" property checking and logging for overload conditions.

Comments suppressed due to low confidence (1)

dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/config/MasterServerLoadProtection.java:52

[nitpick] The log message contains a minor typo with 'over then' which could be corrected to 'exceeds' for better clarity.

log.info("OverLoad: the workflow instance count: {} is over then the maxConcurrentWorkflowInstances {}", currentWorkflowInstanceCount, maxConcurrentWorkflowInstances);

ruanwenjun · 2025-05-10T04:48:53Z

Please solve the code check style by mvn spotless:apply

tusaryan · 2025-05-10T08:37:27Z

I wanted to let you know that I'm currently traveling. Because of this, I might be a bit delayed in addressing the feedback on this pull request. I will definitely get to it as soon as things settle down for me.

tusaryan · 2025-05-16T06:37:54Z

Please solve the code check style by mvn spotless:apply

Code style issues have been fixed by running mvn spotless:apply. Please let me know if there's anything else I can do!

ruanwenjun

LGTM

ruanwenjun · 2025-05-27T01:36:11Z

.../java/org/apache/dolphinscheduler/server/master/config/MasterServerLoadProtectionConfig.java

+    @Bean
+    public MasterServerLoadProtection masterServerLoadProtection(
+                                                                 IWorkflowRepository workflowRepository,
+                                                                 @Value("${master.server-load-protection.max-concurrent-workflow-instances:2147483647}") int maxConcurrentWorkflowInstances) {


We need to add this config in doc, and you need to set other fields in MasterServerLoadProtection, e.g. maxSystemCpuUsagePercentageThresholds, maxJvmCpuUsagePercentageThresholds.

We need to add this config in doc, and you need to set other fields in MasterServerLoadProtection, e.g. maxSystemCpuUsagePercentageThresholds, maxJvmCpuUsagePercentageThresholds.

Thanks for the feedback! I’ll get to it as soon as possible.

Sorry for the delay. I will update it within 1-2 days.

tusaryan · 2025-06-18T12:25:02Z

Please check the failed CI @tusaryan

Okay, I'll check it out.

tusaryan · 2025-06-27T18:47:34Z

I've reviewed the logs, and the "Backend-Build" error appears unrelated to my changes. Could someone please help me with this?

apply mvn spotless:apply

@bean

- Refactored MasterServerLoadProtection to require IWorkflowRepository and maxConcurrentWorkflowInstances via constructor injection. - Updated MasterServerLoadProtectionConfig to provide these dependencies using Spring @bean and @value with a default of 2147483647. - Removed direct instantiation of MasterServerLoadProtection in MasterConfig to avoid missing constructor arguments.

…LoadProtection

…onTest Stub IWorkflowRepository mock in tests to prevent NullPointerException when checking server load.

…terServerLoadProtection - Updated Spring test configuration in MasterConfigTest to properly autowire MasterServerLoadProtection and its dependencies. - Added mock bean for IWorkflowRepository in tests. - Provided test property overrides for load protection thresholds.

sonarqubecloud · 2025-07-02T02:32:01Z

Please retry analysis of this Pull-Request directly on SonarQube Cloud

ruanwenjun

LGTM

SbloodyS

+1

…nstances (apache#17159)

tusaryan requested review from SbloodyS, caishunfeng and ruanwenjun as code owners May 1, 2025 14:49

github-actions bot added backend test labels May 1, 2025

github-actions bot assigned tusaryan May 1, 2025

tusaryan marked this pull request as draft May 1, 2025 14:50

tusaryan marked this pull request as ready for review May 1, 2025 14:50

SbloodyS changed the title ~~[Fix-17157][Master] Support setting max.concurrent.workflow.instances~~ [Improvement-17157][Master] Support setting max.concurrent.workflow.instances May 5, 2025

SbloodyS added the improvement make more easy to user or prompt friendly label May 5, 2025

SbloodyS added this to the 3.3.1 milestone May 5, 2025

ruanwenjun reviewed May 6, 2025

View reviewed changes

tusaryan requested a review from ruanwenjun May 7, 2025 21:29

ruanwenjun reviewed May 8, 2025

View reviewed changes

tusaryan requested a review from ruanwenjun May 8, 2025 08:53

yingh0ng reviewed May 9, 2025

View reviewed changes

...c/main/java/org/apache/dolphinscheduler/server/master/config/MasterServerLoadProtection.java Show resolved Hide resolved

nielifeng requested a review from Copilot May 9, 2025 09:44

Copilot AI reviewed May 9, 2025

View reviewed changes

ruanwenjun force-pushed the improvement/17157-master-concurrent-limit branch from 0e05e2b to 8b5361f Compare May 16, 2025 10:24

ruanwenjun previously approved these changes May 22, 2025

View reviewed changes

ruanwenjun force-pushed the improvement/17157-master-concurrent-limit branch from bd4d946 to 4fdac71 Compare May 27, 2025 01:27

ruanwenjun reviewed May 27, 2025

View reviewed changes

ruanwenjun force-pushed the improvement/17157-master-concurrent-limit branch from 4fdac71 to 3f0ec17 Compare June 9, 2025 01:34

tusaryan dismissed ruanwenjun’s stale review via 356633c June 17, 2025 15:53

tusaryan requested a review from EricGao888 as a code owner June 17, 2025 15:53

tusaryan dismissed stale reviews from Gallardot, SbloodyS, and ruanwenjun via 5fa60cd June 18, 2025 15:35

tusaryan requested review from Gallardot, SbloodyS and ruanwenjun June 19, 2025 12:40

tusaryan and others added 15 commits July 2, 2025 09:44

[Fix-17157][Master] Support setting max.concurrent.workflow.instances

f72fc1e

Update MasterServerLoadProtection.java

8b60f52

Update MasterServerLoadProtectionTest.java

bb6b270

Update MasterServerLoadProtectionConfig.java

d89a1db

Update MasterServerLoadProtection.java

ff26d8a

Update MasterServerLoadProtectionConfig.java

3a5a705

Update MasterServerLoadProtection.java

f31e436

Update MasterServerLoadProtectionConfig.java

20f2466

apply mvn spotless:apply

fix typo in MasterServerLoadProtection

e60c384

Apply spotless formatting

0352ff3

Add system metrics threshold configs and update docs for MasterServer…

dd92e71

…LoadProtection

Fix[Master]: Correct NullPointerException in MasterServerLoadProtecti…

ed65592

…onTest Stub IWorkflowRepository mock in tests to prevent NullPointerException when checking server load.

apply Spotless for code style and formatting

2aedb46

ruanwenjun force-pushed the improvement/17157-master-concurrent-limit branch from 2ebd911 to 2aedb46 Compare July 2, 2025 01:44

ruanwenjun approved these changes Jul 2, 2025

View reviewed changes

SbloodyS approved these changes Jul 2, 2025

View reviewed changes

SbloodyS merged commit 3b1d631 into apache:dev Jul 2, 2025
69 of 70 checks passed

eco8848 pushed a commit to eco8848/dolphinscheduler that referenced this pull request Aug 8, 2025

[Improvement-17157][Master] Support setting max.concurrent.workflow.i…

d40a931

…nstances (apache#17159)

davidzollo pushed a commit to davidzollo/dolphinscheduler that referenced this pull request Oct 27, 2025

[Improvement-17157][Master] Support setting max.concurrent.workflow.i…

666f161

…nstances (apache#17159)

[Improvement-17157][Master] Support setting max.concurrent.workflow.instances #17159

[Improvement-17157][Master] Support setting max.concurrent.workflow.instances #17159

Uh oh!

Conversation

tusaryan commented May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of the pull request

Brief change log

Verify this pull request

Pull Request Notice

Uh oh!

sonarqubecloud bot commented May 5, 2025

Quality Gate passed

Uh oh!

ruanwenjun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tusaryan commented May 7, 2025

Uh oh!

Uh oh!

ruanwenjun May 8, 2025

Choose a reason for hiding this comment

Uh oh!

ruanwenjun May 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

ruanwenjun commented May 10, 2025

Uh oh!

tusaryan commented May 10, 2025

Uh oh!

tusaryan commented May 16, 2025

Uh oh!

ruanwenjun left a comment

Choose a reason for hiding this comment

Uh oh!

ruanwenjun May 27, 2025

Choose a reason for hiding this comment

Uh oh!

tusaryan May 27, 2025

Choose a reason for hiding this comment

Uh oh!

tusaryan Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

tusaryan commented Jun 18, 2025

Uh oh!

tusaryan commented Jun 27, 2025

Uh oh!

sonarqubecloud bot commented Jul 2, 2025

Uh oh!

ruanwenjun left a comment

Choose a reason for hiding this comment

Uh oh!

SbloodyS left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tusaryan commented May 1, 2025 •

edited

Loading