You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kafka.apache.org by js...@apache.org on 2022/08/02 12:39:16 UTC
[kafka] branch trunk updated: MINOR; Use right enum value for broker registration change (#12236)
This is an automated email from the ASF dual-hosted git repository.
jsancio pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/trunk by this push:
new f26842ca0b MINOR; Use right enum value for broker registration change (#12236)
f26842ca0b is described below
commit f26842ca0b564196793ea9f7ca7155314e7bfd35
Author: dengziming <de...@gmail.com>
AuthorDate: Tue Aug 2 20:38:52 2022 +0800
MINOR; Use right enum value for broker registration change (#12236)
The code used BrokerRegistrationFencingChange.FENCE when unfencing a broker and used BrokerRegistrationFencingChange.UNFENCE when fencing a broker, this is confusing. This commit flips the values of the two enums and changes their usage at all of the call sites.
Reviewers: José Armando García Sancio <js...@users.noreply.github.com>
---
.../apache/kafka/controller/ClusterControlManager.java | 4 ++--
.../kafka/controller/ReplicationControlManager.java | 4 ++--
.../main/java/org/apache/kafka/image/ClusterDelta.java | 4 ++--
.../metadata/BrokerRegistrationFencingChange.java | 4 ++--
.../kafka/controller/ClusterControlManagerTest.java | 6 +++---
.../metadata/BrokerRegistrationFencingChangeTest.java | 8 ++++----
...kerRegistrationInControlledShutdownChangeTest.java} | 18 ++++++++----------
7 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java b/metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java
index 41b9728cda..235f077cff 100644
--- a/metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java
+++ b/metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java
@@ -422,7 +422,7 @@ public class ClusterControlManager {
record,
record.id(),
record.epoch(),
- BrokerRegistrationFencingChange.UNFENCE.asBoolean(),
+ BrokerRegistrationFencingChange.FENCE.asBoolean(),
BrokerRegistrationInControlledShutdownChange.NONE.asBoolean()
);
}
@@ -432,7 +432,7 @@ public class ClusterControlManager {
record,
record.id(),
record.epoch(),
- BrokerRegistrationFencingChange.FENCE.asBoolean(),
+ BrokerRegistrationFencingChange.UNFENCE.asBoolean(),
BrokerRegistrationInControlledShutdownChange.NONE.asBoolean()
);
}
diff --git a/metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java b/metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java
index a5e0667bf3..3a3788c41a 100644
--- a/metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java
+++ b/metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java
@@ -1172,7 +1172,7 @@ public class ReplicationControlManager {
if (featureControl.metadataVersion().isBrokerRegistrationChangeRecordSupported()) {
records.add(new ApiMessageAndVersion(new BrokerRegistrationChangeRecord().
setBrokerId(brokerId).setBrokerEpoch(brokerRegistration.epoch()).
- setFenced(BrokerRegistrationFencingChange.UNFENCE.value()),
+ setFenced(BrokerRegistrationFencingChange.FENCE.value()),
(short) 0));
} else {
records.add(new ApiMessageAndVersion(new FenceBrokerRecord().
@@ -1215,7 +1215,7 @@ public class ReplicationControlManager {
if (featureControl.metadataVersion().isBrokerRegistrationChangeRecordSupported()) {
records.add(new ApiMessageAndVersion(new BrokerRegistrationChangeRecord().
setBrokerId(brokerId).setBrokerEpoch(brokerEpoch).
- setFenced(BrokerRegistrationFencingChange.FENCE.value()),
+ setFenced(BrokerRegistrationFencingChange.UNFENCE.value()),
(short) 0));
} else {
records.add(new ApiMessageAndVersion(new UnfenceBrokerRecord().setId(brokerId).
diff --git a/metadata/src/main/java/org/apache/kafka/image/ClusterDelta.java b/metadata/src/main/java/org/apache/kafka/image/ClusterDelta.java
index 110e68fb89..39d6fdb3d7 100644
--- a/metadata/src/main/java/org/apache/kafka/image/ClusterDelta.java
+++ b/metadata/src/main/java/org/apache/kafka/image/ClusterDelta.java
@@ -94,7 +94,7 @@ public final class ClusterDelta {
public void replay(FenceBrokerRecord record) {
BrokerRegistration curRegistration = getBrokerOrThrow(record.id(), record.epoch(), "fence");
changedBrokers.put(record.id(), Optional.of(curRegistration.cloneWith(
- BrokerRegistrationFencingChange.UNFENCE.asBoolean(),
+ BrokerRegistrationFencingChange.FENCE.asBoolean(),
Optional.empty()
)));
}
@@ -102,7 +102,7 @@ public final class ClusterDelta {
public void replay(UnfenceBrokerRecord record) {
BrokerRegistration curRegistration = getBrokerOrThrow(record.id(), record.epoch(), "unfence");
changedBrokers.put(record.id(), Optional.of(curRegistration.cloneWith(
- BrokerRegistrationFencingChange.FENCE.asBoolean(),
+ BrokerRegistrationFencingChange.UNFENCE.asBoolean(),
Optional.empty()
)));
}
diff --git a/metadata/src/main/java/org/apache/kafka/metadata/BrokerRegistrationFencingChange.java b/metadata/src/main/java/org/apache/kafka/metadata/BrokerRegistrationFencingChange.java
index 1c2729355f..ab0bfc6a81 100644
--- a/metadata/src/main/java/org/apache/kafka/metadata/BrokerRegistrationFencingChange.java
+++ b/metadata/src/main/java/org/apache/kafka/metadata/BrokerRegistrationFencingChange.java
@@ -25,9 +25,9 @@ import java.util.stream.Collectors;
public enum BrokerRegistrationFencingChange {
- FENCE(-1, Optional.of(false)),
+ FENCE(1, Optional.of(true)),
NONE(0, Optional.empty()),
- UNFENCE(1, Optional.of(true));
+ UNFENCE(-1, Optional.of(false));
private final byte value;
diff --git a/metadata/src/test/java/org/apache/kafka/controller/ClusterControlManagerTest.java b/metadata/src/test/java/org/apache/kafka/controller/ClusterControlManagerTest.java
index c0b163180e..97d6c88377 100644
--- a/metadata/src/test/java/org/apache/kafka/controller/ClusterControlManagerTest.java
+++ b/metadata/src/test/java/org/apache/kafka/controller/ClusterControlManagerTest.java
@@ -111,7 +111,7 @@ public class ClusterControlManagerTest {
clusterControl.replay(unfenceBrokerRecord);
} else {
BrokerRegistrationChangeRecord changeRecord =
- new BrokerRegistrationChangeRecord().setBrokerId(1).setBrokerEpoch(100).setFenced((byte) -1);
+ new BrokerRegistrationChangeRecord().setBrokerId(1).setBrokerEpoch(100).setFenced(BrokerRegistrationFencingChange.UNFENCE.value());
clusterControl.replay(changeRecord);
}
assertFalse(clusterControl.unfenced(0));
@@ -123,7 +123,7 @@ public class ClusterControlManagerTest {
clusterControl.replay(fenceBrokerRecord);
} else {
BrokerRegistrationChangeRecord changeRecord =
- new BrokerRegistrationChangeRecord().setBrokerId(1).setBrokerEpoch(100).setFenced((byte) 1);
+ new BrokerRegistrationChangeRecord().setBrokerId(1).setBrokerEpoch(100).setFenced(BrokerRegistrationFencingChange.FENCE.value());
clusterControl.replay(changeRecord);
}
assertFalse(clusterControl.unfenced(0));
@@ -234,7 +234,7 @@ public class ClusterControlManagerTest {
registrationChangeRecord = new BrokerRegistrationChangeRecord()
.setBrokerId(0)
.setBrokerEpoch(100)
- .setFenced(BrokerRegistrationFencingChange.FENCE.value());
+ .setFenced(BrokerRegistrationFencingChange.UNFENCE.value());
clusterControl.replay(registrationChangeRecord);
assertTrue(clusterControl.unfenced(0));
diff --git a/metadata/src/test/java/org/apache/kafka/metadata/BrokerRegistrationFencingChangeTest.java b/metadata/src/test/java/org/apache/kafka/metadata/BrokerRegistrationFencingChangeTest.java
index 9266192f24..8f48923c99 100644
--- a/metadata/src/test/java/org/apache/kafka/metadata/BrokerRegistrationFencingChangeTest.java
+++ b/metadata/src/test/java/org/apache/kafka/metadata/BrokerRegistrationFencingChangeTest.java
@@ -29,16 +29,16 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
public class BrokerRegistrationFencingChangeTest {
@Test
public void testValues() {
- assertEquals((byte) -1, BrokerRegistrationFencingChange.FENCE.value());
+ assertEquals((byte) 1, BrokerRegistrationFencingChange.FENCE.value());
assertEquals((byte) 0, BrokerRegistrationFencingChange.NONE.value());
- assertEquals((byte) 1, BrokerRegistrationFencingChange.UNFENCE.value());
+ assertEquals((byte) -1, BrokerRegistrationFencingChange.UNFENCE.value());
}
@Test
public void testAsBoolean() {
- assertEquals(Optional.of(false), BrokerRegistrationFencingChange.FENCE.asBoolean());
+ assertEquals(Optional.of(true), BrokerRegistrationFencingChange.FENCE.asBoolean());
assertEquals(Optional.empty(), BrokerRegistrationFencingChange.NONE.asBoolean());
- assertEquals(Optional.of(true), BrokerRegistrationFencingChange.UNFENCE.asBoolean());
+ assertEquals(Optional.of(false), BrokerRegistrationFencingChange.UNFENCE.asBoolean());
}
@Test
diff --git a/metadata/src/test/java/org/apache/kafka/metadata/BrokerRegistrationFencingChangeTest.java b/metadata/src/test/java/org/apache/kafka/metadata/BrokerRegistrationInControlledShutdownChangeTest.java
similarity index 63%
copy from metadata/src/test/java/org/apache/kafka/metadata/BrokerRegistrationFencingChangeTest.java
copy to metadata/src/test/java/org/apache/kafka/metadata/BrokerRegistrationInControlledShutdownChangeTest.java
index 9266192f24..7f6b69031c 100644
--- a/metadata/src/test/java/org/apache/kafka/metadata/BrokerRegistrationFencingChangeTest.java
+++ b/metadata/src/test/java/org/apache/kafka/metadata/BrokerRegistrationInControlledShutdownChangeTest.java
@@ -24,27 +24,25 @@ import java.util.Optional;
import static org.junit.jupiter.api.Assertions.assertEquals;
-
@Timeout(40)
-public class BrokerRegistrationFencingChangeTest {
+public class BrokerRegistrationInControlledShutdownChangeTest {
+
@Test
public void testValues() {
- assertEquals((byte) -1, BrokerRegistrationFencingChange.FENCE.value());
- assertEquals((byte) 0, BrokerRegistrationFencingChange.NONE.value());
- assertEquals((byte) 1, BrokerRegistrationFencingChange.UNFENCE.value());
+ assertEquals((byte) 0, BrokerRegistrationInControlledShutdownChange.NONE.value());
+ assertEquals((byte) 1, BrokerRegistrationInControlledShutdownChange.IN_CONTROLLED_SHUTDOWN.value());
}
@Test
public void testAsBoolean() {
- assertEquals(Optional.of(false), BrokerRegistrationFencingChange.FENCE.asBoolean());
- assertEquals(Optional.empty(), BrokerRegistrationFencingChange.NONE.asBoolean());
- assertEquals(Optional.of(true), BrokerRegistrationFencingChange.UNFENCE.asBoolean());
+ assertEquals(Optional.empty(), BrokerRegistrationInControlledShutdownChange.NONE.asBoolean());
+ assertEquals(Optional.of(true), BrokerRegistrationInControlledShutdownChange.IN_CONTROLLED_SHUTDOWN.asBoolean());
}
@Test
public void testValueRoundTrip() {
- for (BrokerRegistrationFencingChange change : BrokerRegistrationFencingChange.values()) {
- assertEquals(Optional.of(change), BrokerRegistrationFencingChange.fromValue(change.value()));
+ for (BrokerRegistrationInControlledShutdownChange change : BrokerRegistrationInControlledShutdownChange.values()) {
+ assertEquals(Optional.of(change), BrokerRegistrationInControlledShutdownChange.fromValue(change.value()));
}
}
}