yifan-c commented on a change in pull request #1168:
URL: https://github.com/apache/cassandra/pull/1168#discussion_r806268141
##########
File path: src/java/org/apache/cassandra/db/commitlog/CommitLogReplayer.java
##########
@@ -275,11 +278,22 @@ public void runMayThrow()
assert !newPUCollector.isEmpty();
Keyspace.open(newPUCollector.getKeyspaceName()).apply(newPUCollector.build(),
false, true, false);
+
+ // We should reload in memory schema once we re-apply
schema mutations
+ // so tables/keyspaces are available for the next
mutations.
+ if
(SchemaConstants.isSchemaKeyspace(mutation.getKeyspaceName()))
+ Schema.instance.reloadSchema();
+
commitLogReplayer.keyspacesReplayed.add(keyspace);
}
}
};
- return Stage.MUTATION.submit(runnable, serializedSize);
+ return Stage.MUTATION.submit(() -> {
+ try (OpOrder.Group opOrder =
commitLogReplayer.writeOrder.start())
Review comment:
Revisiting the PR after so long.. (sorry for the delay)..
I think there could be a scenario that the mutations submitted _before_ the
schema change get started _after_ the schema change. It would be a violation of
correctness. Imagine the following scenario,
1. The replayer has submitted 1000 tasks (not triggering
`FBUtilities.waitOnFuture` in `handleMutation`) and sees a schema mutation.
2. The `MUTATION` executor by default process 32 concurrent writes, so there
are over 950 pending tasks in the queue. All of those tasks has not been
started, so `commitLogReplayer.writeOrder.start()` has not been called.
3. Since the replayer now sees a schema mutation, it issues a barrier and
wait. Only the 32 inflight tasks will be grouped before the barrier, and the
rest of the mutations are grouped into the new one incorrectly.
I do not have test to approve it. Given the current implementation, I think
the above scenario is possible. WDYT?
We probably do not need `OpOrder` to serialize the replay. Just drain the
executor queue when seeing a schema change and wait, since replayer
(`handleMutation`) is single threaded already. Like
https://github.com/yifan-c/cassandra/commit/c71faf0b2a363a787f3a0c8fc4db01c0c35f4d7b
##########
File path: src/java/org/apache/cassandra/db/commitlog/CommitLogReplayer.java
##########
@@ -69,7 +69,7 @@
private final Set<Keyspace> keyspacesReplayed;
private final Queue<Future<Integer>> futures;
- private final AtomicInteger replayedCount;
+ public final AtomicInteger replayedCount;
Review comment:
it can remain `private`
--
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]
