denis-chudov commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2163770597
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/PartitionReplicaLifecycleManager.java:
##########
@@ -1410,6 +1409,38 @@ private CompletableFuture<Void>
handleChangePendingAssignmentEvent(
}), ioExecutor);
}
+ private CompletableFuture<Void> resetWithRetry(ZonePartitionId
replicaGrpId, Assignments assignments) {
+ return supplyAsync(() ->
+ inBusyLock(busyLock, () -> replicaMgr.resetPeers(replicaGrpId,
fromAssignments(assignments.nodes()))), ioExecutor)
+ .handleAsync((resetSuccessful, ex) -> {
+ if (ex != null) {
+ if (isRetriable(ex)) {
+ LOG.error("Failed to reset peers. Retrying
[groupId={}]. ", replicaGrpId, ex);
+
+ return resetWithRetry(replicaGrpId, assignments);
+ }
+
+ return CompletableFuture.<Void>failedFuture(ex);
+ }
+
+ if (!resetSuccessful) {
+ LOG.error("Reset peers unsuccessful. Retrying
[groupId={}]. ", replicaGrpId);
Review Comment:
Same with log level here.
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/PartitionReplicaLifecycleManager.java:
##########
@@ -1410,6 +1409,38 @@ private CompletableFuture<Void>
handleChangePendingAssignmentEvent(
}), ioExecutor);
}
+ private CompletableFuture<Void> resetWithRetry(ZonePartitionId
replicaGrpId, Assignments assignments) {
+ return supplyAsync(() ->
+ inBusyLock(busyLock, () -> replicaMgr.resetPeers(replicaGrpId,
fromAssignments(assignments.nodes()))), ioExecutor)
+ .handleAsync((resetSuccessful, ex) -> {
+ if (ex != null) {
+ if (isRetriable(ex)) {
+ LOG.error("Failed to reset peers. Retrying
[groupId={}]. ", replicaGrpId, ex);
Review Comment:
I think log level here can be changed, most likely to debug, to avoid spam.
It definitely shouldn't be `error` or `warn` because it doesn't assume any
actions from the user.
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/PartitionReplicaLifecycleManager.java:
##########
@@ -1410,6 +1409,38 @@ private CompletableFuture<Void>
handleChangePendingAssignmentEvent(
}), ioExecutor);
}
+ private CompletableFuture<Void> resetWithRetry(ZonePartitionId
replicaGrpId, Assignments assignments) {
+ return supplyAsync(() ->
+ inBusyLock(busyLock, () -> replicaMgr.resetPeers(replicaGrpId,
fromAssignments(assignments.nodes()))), ioExecutor)
+ .handleAsync((resetSuccessful, ex) -> {
+ if (ex != null) {
+ if (isRetriable(ex)) {
+ LOG.error("Failed to reset peers. Retrying
[groupId={}]. ", replicaGrpId, ex);
+
+ return resetWithRetry(replicaGrpId, assignments);
+ }
+
+ return CompletableFuture.<Void>failedFuture(ex);
+ }
+
+ if (!resetSuccessful) {
+ LOG.error("Reset peers unsuccessful. Retrying
[groupId={}]. ", replicaGrpId);
+
+ return resetWithRetry(replicaGrpId, assignments);
+ }
+
+ return CompletableFutures.<Void>nullCompletedFuture();
+ }, ioExecutor)
+ .thenCompose(identity());
+ }
+
+ private static boolean isRetriable(Throwable ex) {
+ if (ex instanceof ExecutionException || ex instanceof
CompletionException) {
+ ex = ex.getCause();
+ }
Review Comment:
You can just use `ExceptionUtils#unwrapCause`
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -2511,6 +2507,42 @@ private CompletableFuture<Void>
handleChangePendingAssignmentEvent(
}), ioExecutor);
}
+
+ private CompletableFuture<Void> resetWithRetry(TablePartitionId
replicaGrpId, Assignments assignments) {
Review Comment:
Do we really need code duplication? Maybe move it in some third class?
--
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
