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]

Reply via email to