jlprat commented on code in PR #16807:
URL: https://github.com/apache/kafka/pull/16807#discussion_r1705135463
##########
metadata/src/test/java/org/apache/kafka/controller/FeatureControlManagerTest.java:
##########
@@ -213,7 +212,7 @@ public void testUpdateFeaturesErrorCases() {
}
@Test
- public void testReplayRecords() throws Exception {
Review Comment:
This method doesn't throw exceptions
##########
metadata/src/test/java/org/apache/kafka/controller/FeatureControlManagerTest.java:
##########
@@ -54,7 +54,6 @@
@Timeout(value = 40)
public class FeatureControlManagerTest {
- @SuppressWarnings("unchecked")
Review Comment:
This was not really needed
##########
server-common/src/main/java/org/apache/kafka/timeline/SnapshotRegistry.java:
##########
@@ -205,6 +205,18 @@ public Snapshot getOrCreateSnapshot(long epoch) {
return snapshot;
}
+ /**
+ * Creates a new snapshot at the given epoch.
+ * <br>
+ * If {@code epoch} already exists, and it is the last snapshot then this
operation will do nothing.
+ *
+ * @param epoch The epoch to create the snapshot at. The
current epoch
+ * will be advanced to one past this epoch.
+ */
+ public void idempotentCreateSnapshot(long epoch) {
+ getOrCreateSnapshot(epoch);
+ }
+
Review Comment:
This is the new method that calls the existing `getOrCreateSnapshot` but
doesn't return anything, as it would leak the class outside its visibility
scope.
I called it idempotent as this is what `getOrCreate` does effectively.
##########
server-common/src/main/java/org/apache/kafka/timeline/SnapshottableHashTable.java:
##########
@@ -408,37 +408,6 @@ Iterator<T> snapshottableIterator(long epoch) {
}
}
- String snapshottableToDebugString() {
- StringBuilder bld = new StringBuilder();
- bld.append(String.format("SnapshottableHashTable{%n"));
- bld.append("top tier: ");
- bld.append(baseToDebugString());
- bld.append(String.format(",%nsnapshot tiers: [%n"));
- String prefix = "";
- for (Iterator<Snapshot> iter = snapshotRegistry.iterator();
iter.hasNext(); ) {
- Snapshot snapshot = iter.next();
- bld.append(prefix);
- bld.append("epoch ").append(snapshot.epoch()).append(": ");
- HashTier<T> tier = snapshot.getDelta(this);
- if (tier == null) {
- bld.append("null");
- } else {
- bld.append("HashTier{");
- bld.append("size=").append(tier.size);
- bld.append(", deltaTable=");
- if (tier.deltaTable == null) {
- bld.append("null");
- } else {
- bld.append(tier.deltaTable.baseToDebugString());
- }
- bld.append("}");
- }
- bld.append(String.format("%n"));
- }
- bld.append(String.format("]}%n"));
- return bld.toString();
- }
-
Review Comment:
This wasn't used at all
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
