Skip to content

[Improvement][Master]add worker group checking steps before workflow executor#12136

Closed
DarkAssassinator wants to merge 2 commits intoapache:devfrom
DarkAssassinator:checkWorkerGroupBeforeRunning
Closed

[Improvement][Master]add worker group checking steps before workflow executor#12136
DarkAssassinator wants to merge 2 commits intoapache:devfrom
DarkAssassinator:checkWorkerGroupBeforeRunning

Conversation

@DarkAssassinator
Copy link
Contributor

Purpose of the pull request

fix #12001

Brief change log

Just add a validator before task instance was put to dispatch queue.

  1. When use start a ProcessInstance or re-run this processInstance, this PR will check if the worker group exists.

Verify this pull request

Comment on lines +384 to +386
long subProcessCode = Long
.parseLong(JSONUtils.getNodeString(taskDefinition.getTaskParams(),
Constants.CMD_PARAM_SUB_PROCESS_DEFINE_CODE));

Check notice

Code scanning / CodeQL

Missing catch of NumberFormatException

Potential uncaught 'java.lang.NumberFormatException'.
@DarkAssassinator
Copy link
Contributor Author

@caishunfeng @SbloodyS could u help to review this PR? thx

Copy link
Contributor

@hstdream hstdream left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

List<String> availableWorkerGroupList = workerGroups.stream() .map(WorkerGroup::getName) .collect(Collectors.toList()); if (!availableWorkerGroupList.contains(Constants.DEFAULT_WORKER_GROUP)) { availableWorkerGroupList.add(Constants.DEFAULT_WORKER_GROUP); }

@DarkAssassinator DarkAssassinator force-pushed the checkWorkerGroupBeforeRunning branch from 99b8bae to 80ba0ee Compare October 7, 2022 08:51
@codecov-commenter
Copy link

codecov-commenter commented Oct 9, 2022

Codecov Report

Merging #12136 (80ba0ee) into dev (7beff83) will decrease coverage by 0.00%.
The diff coverage is 39.02%.

@@             Coverage Diff              @@
##                dev   #12136      +/-   ##
============================================
- Coverage     39.70%   39.70%   -0.01%     
- Complexity     4190     4195       +5     
============================================
  Files          1016     1016              
  Lines         38049    38090      +41     
  Branches       4365     4371       +6     
============================================
+ Hits          15108    15124      +16     
- Misses        21196    21219      +23     
- Partials       1745     1747       +2     
Impacted Files Coverage Δ
...duler/api/service/impl/WorkerGroupServiceImpl.java 58.00% <0.00%> (-2.84%) ⬇️
...cheduler/api/service/impl/ExecutorServiceImpl.java 43.73% <43.75%> (+<0.01%) ⬆️
.../org/apache/dolphinscheduler/api/enums/Status.java 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 9, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

16.8% 16.8% Coverage
0.0% 0.0% Duplication

Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer we can set the task to fail if it cannot find the workGroup, rather than check the workgroup, since the workflow may be executed by the scheduler rather than manual.

@github-actions
Copy link

github-actions bot commented Sep 6, 2023

This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Sep 6, 2023
@github-actions
Copy link

This pull request has been closed because it has not had recent activity. You could reopen it if you try to continue your work, and anyone who are interested in it are encouraged to continue work on this pull request.

@github-actions github-actions bot closed this Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement][Master] Process instance will keep running when cannot find the worker group.

4 participants