Skip to content

[Fix-17232][master] handle the problem that master nodes gains the same slot id #17233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
ruanwenjun merged 7 commits into from
Jun 5, 2025
Merged

[Fix-17232][master] handle the problem that master nodes gains the same slot id #17233

ruanwenjun merged 7 commits into from
Jun 5, 2025

Conversation

@ZhongJinHacker
Copy link
Contributor

@ZhongJinHacker ZhongJinHacker commented Jun 3, 2025

Purpose of the pull request

close #17232

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

Pull Request Notice

Pull Request Notice

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

@SbloodyS SbloodyS changed the title [fix-17232][master]: handle the problem that master nodes gains the same slot id [Fix-17232][master] handle the problem that master nodes gains the same slot id Jun 4, 2025
@SbloodyS SbloodyS added the bug Something isn't working label Jun 4, 2025
@SbloodyS SbloodyS added this to the 3.3.1 milestone Jun 4, 2025
@ruanwenjun
Copy link
Member

ruanwenjun commented Jun 4, 2025 •
edited
Loading

@ZhongJinHacker Thanks for your patch, good catch!

It's better to sort the master list at MasterSlotManager#doReBalance, this component is used to manage the slot, other component don't need to take care of the slot(master order)

dolphinscheduler/dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/cluster/MasterSlotManager.java

Lines 61 to 86 in 6f12d1d

public void doReBalance(List<MasterServerMetadata> normalMasterServers) {
int tmpCurrentSlot = -1;
for (int i = 0; i < normalMasterServers.size(); i++) {
if (normalMasterServers.get(i).getAddress().equals(masterConfig.getMasterAddress())) {
tmpCurrentSlot = i;
break;
}
}
if (tmpCurrentSlot == -1) {
log.warn(
"Do rebalance failed, cannot found the current master: {} in the normal master clusters: {}. Please check the current master server status",
masterConfig.getMasterAddress(), normalMasterServers);
currentSlot = -1;
return;
}
if (totalSlots == normalMasterServers.size() && currentSlot == tmpCurrentSlot) {
log.debug("No need to rebalance, the currentSlot: {}, totalSlots: {} doesn't changed", currentSlot,
totalSlots);
return;
}
totalSlots = normalMasterServers.size();
currentSlot = tmpCurrentSlot;
log.info("Do rebalance success, current master slot: {}, total master slots: {}", currentSlot, totalSlots);
}

@ZhongJinHacker
Copy link
Contributor Author

ZhongJinHacker commented Jun 4, 2025

ok, I will fix it

ruanwenjun
ruanwenjun previously approved these changes Jun 4, 2025
View reviewed changes
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.

LGTM

@ruanwenjun
Copy link
Member

ruanwenjun commented Jun 5, 2025

Please run mvn spotless:apply to fix the code style.

@ZhongJinHacker
Copy link
Contributor Author

ZhongJinHacker commented Jun 5, 2025

done

SbloodyS
SbloodyS approved these changes Jun 5, 2025
View reviewed changes
Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

+1

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 5, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

ruanwenjun
ruanwenjun approved these changes Jun 5, 2025
View reviewed changes
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.

LGTM

@ruanwenjun ruanwenjun merged commit 7266e3f into apache:dev Jun 5, 2025
72 of 74 checks passed
hackergin pushed a commit to hackergin/dolphinscheduler that referenced this pull request Jun 15, 2025
@ZhongJinHacker @hackergin
[Fix-17232][master] handle the problem that master nodes gains the sa…
012f3f4 
…me slot id (apache#17233)
davidzollo pushed a commit to davidzollo/dolphinscheduler that referenced this pull request Oct 27, 2025
@ZhongJinHacker @davidzollo
[Fix-17232][master] handle the problem that master nodes gains the sa…
7230b7e 
…me slot id (apache#17233)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SbloodyS SbloodyS SbloodyS approved these changes

@ruanwenjun ruanwenjun ruanwenjun approved these changes

@caishunfeng caishunfeng Awaiting requested review from caishunfeng caishunfeng is a code owner

Assignees

@ZhongJinHacker ZhongJinHacker

Labels

backend bug Something isn't working priority:high test

Projects

None yet

Milestone

3.3.1

Development

Successfully merging this pull request may close these issues.

[Bug] [Master] when use K8S to deploy master nodes use scroll batch deploy mode, the master nodes may gain the same slot id

3 participants

@ZhongJinHacker @ruanwenjun @SbloodyS