MINOR: fix indentation and add builders in some KRaft tests (#12720)

Add builders for LocalLogManagerTestEnv and QuorumControllerTestEnv, since the constructor
overloads were starting to get unwieldy.

Make indentation more consistent in QuorumControllerTest. Take advantage of the fact that you can
initialize multiple resources in a Java try-with-resources block to avoid excessive indentation in a few
cases.

Reviewers: José Armando García Sancio <jsancio@apache.org>
This commit is contained in:
Colin Patrick McCabe 2022-10-07 13:53:41 -07:00 committed by GitHub
parent e8d32563f3
commit 1c07095cbd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 601 additions and 484 deletions

View File

@ -18,7 +18,6 @@
package org.apache.kafka.controller;
import org.apache.kafka.clients.ApiVersions;
import org.apache.kafka.controller.QuorumController.Builder;
import org.apache.kafka.metadata.bootstrap.BootstrapMetadata;
import org.apache.kafka.metalog.LocalLogManagerTestEnv;
import org.apache.kafka.raft.LeaderAndEpoch;
@ -44,28 +43,51 @@ public class QuorumControllerTestEnv implements AutoCloseable {
private final MockFaultHandler fatalFaultHandler = new MockFaultHandler("fatalFaultHandler");
private final MockFaultHandler metadataFaultHandler = new MockFaultHandler("metadataFaultHandler");
public QuorumControllerTestEnv(
LocalLogManagerTestEnv logEnv,
Consumer<QuorumController.Builder> builderConsumer
) throws Exception {
this(logEnv, builderConsumer, OptionalLong.empty(), OptionalLong.empty(),
BootstrapMetadata.fromVersion(MetadataVersion.latest(), "test-provided version"));
public static class Builder {
private final LocalLogManagerTestEnv logEnv;
private Consumer<QuorumController.Builder> controllerBuilderInitializer = __ -> { };
private OptionalLong sessionTimeoutMillis = OptionalLong.empty();
private OptionalLong leaderImbalanceCheckIntervalNs = OptionalLong.empty();
private BootstrapMetadata bootstrapMetadata = BootstrapMetadata.
fromVersion(MetadataVersion.latest(), "test-provided version");
public Builder(LocalLogManagerTestEnv logEnv) {
this.logEnv = logEnv;
}
public Builder setControllerBuilderInitializer(Consumer<QuorumController.Builder> controllerBuilderInitializer) {
this.controllerBuilderInitializer = controllerBuilderInitializer;
return this;
}
public Builder setSessionTimeoutMillis(OptionalLong sessionTimeoutMillis) {
this.sessionTimeoutMillis = sessionTimeoutMillis;
return this;
}
public Builder setLeaderImbalanceCheckIntervalNs(OptionalLong leaderImbalanceCheckIntervalNs) {
this.leaderImbalanceCheckIntervalNs = leaderImbalanceCheckIntervalNs;
return this;
}
public Builder setBootstrapMetadata(BootstrapMetadata bootstrapMetadata) {
this.bootstrapMetadata = bootstrapMetadata;
return this;
}
public QuorumControllerTestEnv build() throws Exception {
return new QuorumControllerTestEnv(
logEnv,
controllerBuilderInitializer,
sessionTimeoutMillis,
leaderImbalanceCheckIntervalNs,
bootstrapMetadata);
}
}
public QuorumControllerTestEnv(
LocalLogManagerTestEnv logEnv,
Consumer<Builder> builderConsumer,
OptionalLong sessionTimeoutMillis,
OptionalLong leaderImbalanceCheckIntervalNs,
MetadataVersion metadataVersion
) throws Exception {
this(logEnv, builderConsumer, sessionTimeoutMillis, leaderImbalanceCheckIntervalNs,
BootstrapMetadata.fromVersion(metadataVersion, "test-provided version"));
}
public QuorumControllerTestEnv(
private QuorumControllerTestEnv(
LocalLogManagerTestEnv logEnv,
Consumer<Builder> builderConsumer,
Consumer<QuorumController.Builder> controllerBuilderInitializer,
OptionalLong sessionTimeoutMillis,
OptionalLong leaderImbalanceCheckIntervalNs,
BootstrapMetadata bootstrapMetadata
@ -87,7 +109,7 @@ public class QuorumControllerTestEnv implements AutoCloseable {
});
builder.setFatalFaultHandler(fatalFaultHandler);
builder.setMetadataFaultHandler(metadataFaultHandler);
builderConsumer.accept(builder);
controllerBuilderInitializer.accept(builder);
this.controllers.add(builder.build());
}
} catch (Exception e) {

View File

@ -26,7 +26,6 @@ import org.junit.jupiter.api.Timeout;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.OptionalInt;
import java.util.stream.Collectors;
@ -44,8 +43,10 @@ public class LocalLogManagerTest {
*/
@Test
public void testCreateAndClose() throws Exception {
try (LocalLogManagerTestEnv env =
LocalLogManagerTestEnv.createWithMockListeners(1, Optional.empty())) {
try (
LocalLogManagerTestEnv env = new LocalLogManagerTestEnv.Builder(1).
buildWithMockListeners();
) {
env.close();
assertEquals(null, env.firstError.get());
}
@ -56,8 +57,10 @@ public class LocalLogManagerTest {
*/
@Test
public void testClaimsLeadership() throws Exception {
try (LocalLogManagerTestEnv env =
LocalLogManagerTestEnv.createWithMockListeners(1, Optional.empty())) {
try (
LocalLogManagerTestEnv env = new LocalLogManagerTestEnv.Builder(1).
buildWithMockListeners();
) {
assertEquals(new LeaderAndEpoch(OptionalInt.of(0), 1), env.waitForLeader());
env.close();
assertEquals(null, env.firstError.get());
@ -69,8 +72,10 @@ public class LocalLogManagerTest {
*/
@Test
public void testPassLeadership() throws Exception {
try (LocalLogManagerTestEnv env =
LocalLogManagerTestEnv.createWithMockListeners(3, Optional.empty())) {
try (
LocalLogManagerTestEnv env = new LocalLogManagerTestEnv.Builder(3).
buildWithMockListeners();
) {
LeaderAndEpoch first = env.waitForLeader();
LeaderAndEpoch cur = first;
do {
@ -123,8 +128,10 @@ public class LocalLogManagerTest {
*/
@Test
public void testCommits() throws Exception {
try (LocalLogManagerTestEnv env =
LocalLogManagerTestEnv.createWithMockListeners(3, Optional.empty())) {
try (
LocalLogManagerTestEnv env = new LocalLogManagerTestEnv.Builder(3).
buildWithMockListeners();
) {
LeaderAndEpoch leaderInfo = env.waitForLeader();
int leaderId = leaderInfo.leaderId().orElseThrow(() ->
new AssertionError("Current leader is undefined")

View File

@ -66,31 +66,59 @@ public class LocalLogManagerTestEnv implements AutoCloseable {
*/
private final List<LocalLogManager> logManagers;
public static LocalLogManagerTestEnv createWithMockListeners(
int numManagers,
Optional<RawSnapshotReader> snapshot
) throws Exception {
LocalLogManagerTestEnv testEnv = new LocalLogManagerTestEnv(numManagers, snapshot);
try {
for (LocalLogManager logManager : testEnv.logManagers) {
logManager.register(new MockMetaLogManagerListener(logManager.nodeId().getAsInt()));
}
} catch (Exception e) {
testEnv.close();
throw e;
public static class Builder {
private final int numManagers;
private Optional<RawSnapshotReader> snapshotReader = Optional.empty();
private Consumer<SharedLogData> sharedLogDataInitializer = __ -> { };
public Builder(int numManagers) {
this.numManagers = numManagers;
}
public Builder setSnapshotReader(RawSnapshotReader snapshotReader) {
this.snapshotReader = Optional.of(snapshotReader);
return this;
}
public Builder setSharedLogDataInitializer(Consumer<SharedLogData> sharedLogDataInitializer) {
this.sharedLogDataInitializer = sharedLogDataInitializer;
return this;
}
public LocalLogManagerTestEnv build() {
return new LocalLogManagerTestEnv(
numManagers,
snapshotReader,
sharedLogDataInitializer);
}
public LocalLogManagerTestEnv buildWithMockListeners() {
LocalLogManagerTestEnv env = build();
try {
for (LocalLogManager logManager : env.logManagers) {
logManager.register(new MockMetaLogManagerListener(logManager.nodeId().getAsInt()));
}
} catch (Exception e) {
try {
env.close();
} catch (Exception t) {
log.error("Error while closing new log environment", t);
}
throw e;
}
return env;
}
return testEnv;
}
public LocalLogManagerTestEnv(
private LocalLogManagerTestEnv(
int numManagers,
Optional<RawSnapshotReader> snapshot,
Consumer<SharedLogData> dataSetup
) throws Exception {
Optional<RawSnapshotReader> snapshotReader,
Consumer<SharedLogData> sharedLogDataInitializer
) {
clusterId = Uuid.randomUuid().toString();
dir = TestUtils.tempDirectory();
shared = new SharedLogData(snapshot);
dataSetup.accept(shared);
shared = new SharedLogData(snapshotReader);
sharedLogDataInitializer.accept(shared);
List<LocalLogManager> newLogManagers = new ArrayList<>(numManagers);
try {
for (int nodeId = 0; nodeId < numManagers; nodeId++) {
@ -112,13 +140,6 @@ public class LocalLogManagerTestEnv implements AutoCloseable {
this.logManagers = newLogManagers;
}
public LocalLogManagerTestEnv(
int numManagers,
Optional<RawSnapshotReader> snapshot
) throws Exception {
this(numManagers, snapshot, __ -> { });
}
/**
* Append some records to the log. This method is meant to be called before the
* controllers are started, to simulate a pre-existing metadata log.