vldpyatkov commented on code in PR #2820:
URL: https://github.com/apache/ignite-3/pull/2820#discussion_r1390985251
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/PartitionListener.java:
##########
@@ -148,7 +159,17 @@ public void onWrite(Iterator<CommandClosure<WriteCommand>>
iterator) {
iterator.forEachRemaining((CommandClosure<? extends WriteCommand> clo)
-> {
Command command = clo.command();
- // LOG.info("CMD {}", command.getClass().getName());
+ if (command instanceof SafeTimePropagatingCommand) {
+ SafeTimePropagatingCommand cmd = (SafeTimePropagatingCommand)
command;
+ long proposedSafeTime = cmd.safeTime().longValue();
+
+ if (proposedSafeTime > maxObservableSafeTimeVerifier) {
+ maxObservableSafeTimeVerifier = proposedSafeTime;
+ } else {
+ assert false : "Safe time reordering detected [current=" +
maxObservableSafeTimeVerifier
+ + ", proposed=" + proposedSafeTime + "]";
+ }
Review Comment:
Why do you make the code so complicated? I suspect you should write like
this:
```
assert roposedSafeTime <= maxObservableSafeTimeVerifier : "Safe time
reordering detected [current=" + maxObservableSafeTimeVerifier + ", proposed="
+ proposedSafeTime + "].";
maxObservableSafeTimeVerifier = proposedSafeTime;
```
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java:
##########
@@ -187,6 +190,9 @@ public class PartitionReplicaListener implements
ReplicaListener {
/** Factory for creating replica command messages. */
private static final ReplicaMessagesFactory REPLICA_MESSAGES_FACTORY = new
ReplicaMessagesFactory();
+ /** Replication retries limit. */
+ private static final int MAX_RETIES_ON_SAFE_TIME_REORDERING = 1000;
Review Comment:
Why is it meant to limit attempts if this is not a recoverable case?
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/PartitionListener.java:
##########
@@ -148,7 +159,17 @@ public void onWrite(Iterator<CommandClosure<WriteCommand>>
iterator) {
iterator.forEachRemaining((CommandClosure<? extends WriteCommand> clo)
-> {
Command command = clo.command();
- // LOG.info("CMD {}", command.getClass().getName());
+ if (command instanceof SafeTimePropagatingCommand) {
+ SafeTimePropagatingCommand cmd = (SafeTimePropagatingCommand)
command;
+ long proposedSafeTime = cmd.safeTime().longValue();
+
+ if (proposedSafeTime > maxObservableSafeTimeVerifier) {
+ maxObservableSafeTimeVerifier = proposedSafeTime;
+ } else {
+ assert false : "Safe time reordering detected [current=" +
maxObservableSafeTimeVerifier
+ + ", proposed=" + proposedSafeTime + "]";
+ }
Review Comment:
If the `maxObservableSafeTimeVerifier` is needed only for verification, it
would be better to remove it altogether.
--
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]
