sanpwc commented on code in PR #3540:
URL: https://github.com/apache/ignite-3/pull/3540#discussion_r1553849374
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -227,6 +230,8 @@ public ReplicaManager(
new LinkedBlockingQueue<>(),
NamedThreadFactory.create(nodeName, "replica", LOG)
);
+
+ placementDriver.listen(PrimaryReplicaEvent.PRIMARY_REPLICA_ELECTED,
this::onPrimaryReplicaElected);
Review Comment:
I don't think that it's the proper place. I'd rather expect TxManager to
listen PRIMARY_REPLICA_ELECTED and send cleanupRecoveryRequest to the one.
Internally cleanupRecoveryRequest will scan corresponding TxStateStorage and
trigger the common cleanup. Something like that.
##########
modules/client/src/test/java/org/apache/ignite/client/fakes/FakeTxManager.java:
##########
@@ -195,6 +196,15 @@ public CompletableFuture<Void> cleanup(
return nullCompletedFuture();
}
+ @Override
+ public CompletableFuture<Void> cleanup(
+ Collection<TablePartitionId> enlistedPartitions,
+ boolean commit,
+ @Nullable HybridTimestamp commitTimestamp, UUID txId
Review Comment:
Please move txId to the separate line.
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/Replica.java:
##########
@@ -128,6 +128,13 @@ public Replica(
raftClient.subscribeLeader(this::onLeaderElected);
}
+ /**
+ * Returns an instance of replica listener, associated with current
replica.
+ */
+ ReplicaListener replicaListener() {
Review Comment:
No way. We should not expose the listener.
--
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]