This is an automated email from the ASF dual-hosted git repository.
cmccabe pushed a commit to branch 3.3
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/3.3 by this push:
new 98643608b0 MINOR; Use right enum value for broker registration change
(#12236)
98643608b0 is described below
commit 98643608b0e16c0d9b7b1e800b65267dcb72d40d
Author: dengziming <[email protected]>
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 <[email protected]>
---
.../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()));
}
}
}
