rpuch commented on code in PR #5991:
URL: https://github.com/apache/ignite-3/pull/5991#discussion_r2131949726


##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/PartitionReplicaLifecycleManager.java:
##########
@@ -1576,47 +1580,41 @@ public CompletableFuture<Void> 
stopAsync(ComponentContext componentContext) {
     }
 
     /**
-     * Stops zone replica, executes a given action and fires a given local 
event after.
+     * Stops zone replica, executes a given action and fires given local 
events.
      *
-     * @param zonePartitionId Zone's replication group identifier.
-     * @param afterReplicaStopAction A consumer that will be executed if it is 
not null after zone replica stop process will be finished and
-     *      stopping result will be given to the consumer.
-     * @param afterReplicaStoppedEvent A local event type that if not null 
should be fired in the end of the method.
-     * @param afterReplicaStoppedEventRevision A revision parameter of the 
local event if the last is presented. Must not be null if the
-     *      event isn't null too.
-     *
-     * @return Future that will be completed after zone replica was stopped 
and all given non-null actions are done, the future's result
-     *      answers was replica was stopped correctly or not.
+     * @return Future that will be completed after zone replica was stopped 
and event handling was finished.
      */
     @VisibleForTesting
     public CompletableFuture<Void> stopPartitionInternal(
             ZonePartitionId zonePartitionId,
+            LocalPartitionReplicaEvent beforeReplicaStoppedEvent,
             LocalPartitionReplicaEvent afterReplicaStoppedEvent,
-            long afterReplicaStoppedEventRevision,
+            long eventRevision,
             Consumer<Boolean> afterReplicaStopAction
     ) {
+        // Not using the busy lock here, because this method is called on 
component stop.
         return executeUnderZoneWriteLock(zonePartitionId.zoneId(), () -> {
-            try {
-                return replicaMgr.stopReplica(zonePartitionId)
-                        .thenCompose(replicaWasStopped -> {
-                            afterReplicaStopAction.accept(replicaWasStopped);
+            var eventParameters = new 
LocalPartitionReplicaEventParameters(zonePartitionId, eventRevision, false);
 
-                            if (!replicaWasStopped) {
-                                return nullCompletedFuture();
-                            }
+            return fireEvent(beforeReplicaStoppedEvent, eventParameters)
+                    .thenCompose(v -> {
+                        try {
+                            return replicaMgr.stopReplica(zonePartitionId);
+                        } catch (NodeStoppingException e) {
+                            throw new CompletionException(e);

Review Comment:
   Why are we rethrowing it here? `NodeStoppingException` is not a problem when 
attempting to stop a replica, and previous version of the code just ignored the 
exception



##########
modules/partition-replicator/src/test/java/org/apache/ignite/internal/partition/replicator/PartitionReplicaLifecycleManagerTest.java:
##########
@@ -260,4 +266,53 @@ void testStopOrder() throws NodeStoppingException {
         inOrder.verify(zoneResourcesManager, 
timeout(1_000)).destroyZonePartitionResources(zonePartitionId);
         inOrder.verify(replicaManager, 
timeout(1_000)).destroyReplicationProtocolStorages(zonePartitionId, false);
     }
+
+    @Test
+    void producesEventsOnPartitionRestart() {
+        var beforeReplicaStoppedFuture = new 
CompletableFuture<LocalPartitionReplicaEventParameters>();
+        var afterReplicaStoppedFuture = new 
CompletableFuture<LocalPartitionReplicaEventParameters>();
+
+        partitionReplicaLifecycleManager.listen(BEFORE_REPLICA_STOPPED, 
EventListener.fromConsumer(beforeReplicaStoppedFuture::complete));
+        partitionReplicaLifecycleManager.listen(AFTER_REPLICA_STOPPED, 
EventListener.fromConsumer(params -> {
+            beforeReplicaStoppedFuture.thenRun(() -> 
afterReplicaStoppedFuture.complete(params));

Review Comment:
   Why is 'after' future chained to 'before' future?



-- 
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: notifications-unsubscr...@ignite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to