ifesdjeen commented on code in PR #3597:
URL: https://github.com/apache/cassandra/pull/3597#discussion_r1815557290
##########
test/distributed/org/apache/cassandra/fuzz/topology/TopologyMixupTestBase.java:
##########
@@ -400,16 +422,20 @@ protected static class State<S extends SchemaSpec>
implements AutoCloseable
final TopologyHistory topologyHistory;
final Cluster cluster;
final S schemaSpec;
+ final List<BiFunction<State<S>, Gen<Command<State<S>, Void, ?>>,
Gen<Command<State<S>, Void, ?>>>> commandsTransformers = new ArrayList<>();
Review Comment:
Nit: should we maybe alias this to something? Like an interface, maybe
Command Generator and a named function.
##########
test/distributed/org/apache/cassandra/fuzz/topology/TopologyMixupTestBase.java:
##########
@@ -336,12 +353,15 @@ private EnumSet<TopologyChange>
possibleTopologyChanges(State<S> state)
// so up is enough to know the topology size
int up = state.topologyHistory.up().length;
int down = state.topologyHistory.down().length;
+ int[] upAndSafe = state.upAndSafe();
int total = up + down;
if (total < state.topologyHistory.maxNodes)
possibleTopologyChanges.add(TopologyChange.AddNode);
- if (up > state.topologyHistory.quorum())
+ if (upAndSafe.length > 0)
{
- if (up > TARGET_RF)
+ // can't remove the node if all nodes are CMS nodes
+ var nonCMS = Sets.difference(asSet(upAndSafe),
asSet(state.cmsGroup));
+ if (!nonCMS.isEmpty())
Review Comment:
Also, _technically_ we could resize CMS, but we don't have to do this now.
##########
test/distributed/org/apache/cassandra/fuzz/topology/TopologyMixupTestBase.java:
##########
@@ -336,12 +353,15 @@ private EnumSet<TopologyChange>
possibleTopologyChanges(State<S> state)
// so up is enough to know the topology size
int up = state.topologyHistory.up().length;
int down = state.topologyHistory.down().length;
+ int[] upAndSafe = state.upAndSafe();
int total = up + down;
if (total < state.topologyHistory.maxNodes)
possibleTopologyChanges.add(TopologyChange.AddNode);
- if (up > state.topologyHistory.quorum())
+ if (upAndSafe.length > 0)
{
- if (up > TARGET_RF)
+ // can't remove the node if all nodes are CMS nodes
+ var nonCMS = Sets.difference(asSet(upAndSafe),
asSet(state.cmsGroup));
+ if (!nonCMS.isEmpty())
Review Comment:
Nit: could probably fold `var` into `if`.
##########
test/distributed/org/apache/cassandra/fuzz/topology/TopologyMixupTestBase.java:
##########
@@ -464,19 +490,46 @@ public String ipAddress(int nodeNum)
result.asserts().success();
logger.info("CMS reconfigure: {}", result.getStdout());
}
- preActions.add(new Runnable()
- {
+ commandsTransformers.add(new BiFunction<State<S>,
Gen<Command<State<S>, Void, ?>>, Gen<Command<State<S>, Void, ?>>>() {
// in order to remove this action, an anonymous class is
needed so "this" works, lambda "this" is the parent class
@Override
- public void run()
- {
- if (topologyHistory.up().length == TARGET_RF)
- {
+ public Gen<Command<State<S>, Void, ?>> apply(State<S> state,
Gen<Command<State<S>, Void, ?>> commandGen) {
+ if (topologyHistory.up().length < TARGET_RF)
+ return commandGen;
+ SimpleCommand<State<S>> reconfig = new
SimpleCommand<>("nodetool cms reconfigure " + TARGET_RF, ignore -> {
NodeToolResult result =
cluster.get(1).nodetoolResult("cms", "reconfigure",
Integer.toString(TARGET_RF));
result.asserts().success();
logger.info("CMS reconfigure: {}", result.getStdout());
- preActions.remove(this);
- }
+ });
+ SimpleCommand<State<S>> fixDistributedSchemas = new
SimpleCommand<>("Set system distributed keyspaces to RF=" + TARGET_RF, ignore ->
+ fixDistributedSchemas(cluster));
+ SimpleCommand<State<S>> fixTestKeyspace = new
SimpleCommand<>("Set " + KEYSPACE + " keyspace to RF=" + TARGET_RF, s -> {
+ cluster.schemaChange("ALTER KEYSPACE " + KEYSPACE + "
WITH replication = {'class': 'SimpleStrategy', 'replication_factor': " +
TARGET_RF + "}");
+ rf = new
TokenPlacementModel.SimpleReplicationFactor(TARGET_RF);
+ });
+ var self = this;
Review Comment:
I _think_ you can do `this` without this ref by using
`<name_of_enclosing_class>.this`
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
