Skip to content
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.

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

[ISSUE #7626] Topic perm was mistakenly changed to 4 in dledger mode #7661

Merged
merged 3 commits into from
Dec 15, 2023

Conversation

LittleBoy18
Copy link
Contributor

1、判断isPrimeSlave条件添加当前broker列表是否包含leader节点的判断
2、添加注释

Which Issue(s) This PR Fixes

Fixes #7626

Brief Description

How Did You Test This Change?

@RongtongJin RongtongJin changed the title dledger模式follower节点加入集群,由于选举延迟问题导致topic perm被改为4 [ISSUE #7626] Topic perm was mistakenly changed to 4 in dledger mode Dec 15, 2023
@@ -293,8 +293,10 @@ public RegisterBrokerResult registerBroker(
registerFirst = registerFirst || (StringUtils.isEmpty(oldAddr));

boolean isMaster = MixAll.MASTER_ID == brokerId;

// isPrimeSlave < Slave Acting Master模式下,Namesrv将把brokerId最小的存活Slave视为“代理”Master,并将brokerPermission修改为4(Read-Only)>
Copy link
Contributor

Choose a reason for hiding this comment

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

注释不能用中文

boolean isPrimeSlave = !isOldVersionBroker && !isMaster
&& brokerId == Collections.min(brokerAddrsMap.keySet());
&& brokerId == Collections.min(brokerAddrsMap.keySet()) && !brokerAddrsMap.containsKey(0L);
Copy link
Contributor

Choose a reason for hiding this comment

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

我觉得最好不要在这里修改,而是再擦除写权限的时候,看看是不是brokerData是不是打开enableActingMaster的

Comment on lines 310 to 312
if (isPrimeSlave) {
// Wipe write perm for prime slave
topicConfig.setPerm(topicConfig.getPerm() & (~PermName.PERM_WRITE));
// In Slave Acting Master mode, Namesrv will regard the surviving Slave with the smallest brokerId as the "agent" Master, and modify the brokerPermission to read-only.
if (brokerData.isEnableActingMaster()){
Copy link
Contributor

Choose a reason for hiding this comment

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

这两个if可以放在一起

@RongtongJin RongtongJin merged commit 7e17867 into apache:develop Dec 15, 2023
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] the topic perm to be modified to 4 and could not be automatically restored to 6
2 participants