You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "Cyrill (via GitHub)" <gi...@apache.org> on 2025/06/16 12:33:49 UTC
[PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Cyrill opened a new pull request, #6053:
URL: https://github.com/apache/ignite-3/pull/6053
https://issues.apache.org/jira/browse/IGNITE-25543
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2543751619
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -788,7 +794,73 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
*/
public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners, long sequenceToken) {
RaftNodeId raftNodeId = new RaftNodeId(replicaGrpId, new Peer(localNodeConsistentId));
- ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners, sequenceToken);
+ Loza loza = (Loza) raftManager;
+ Status status = loza.resetPeers(raftNodeId, peersAndLearners, sequenceToken);
+
+ // Stale configuration change will not be retried.
+ if (!status.isOk() && status.getRaftError() == RaftError.ESTALE) {
+ // TODO: proper error.
Review Comment:
fixed
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2541124528
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -788,7 +794,73 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
*/
public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners, long sequenceToken) {
Review Comment:
We still use resetPeers instead of resetWithRetry in ReplicaStateManager, is it intended?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2177773640
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -790,9 +794,49 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
* @param replicaGrpId Replication group ID.
* @param peersAndLearners New node configuration.
*/
- public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
+ public boolean resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
RaftNodeId raftNodeId = new RaftNodeId(replicaGrpId, new Peer(localNodeConsistentId));
- ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ return ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ }
+
+ /**
+ * Performs a {@code resetPeers} operation on raft node with retries.
+ *
+ * @param replicaGrpId Replication group ID.
+ * @param assignments New assignments.
+ */
+ public CompletableFuture<Void> resetWithRetry(ReplicationGroupId replicaGrpId, Assignments assignments) {
+ return supplyAsync(() -> inBusyLock(busyLock, () -> {
+ assert isReplicaStarted(replicaGrpId) : "The local node is outside of the replication group: " + replicaGrpId;
+
+ return resetPeers(replicaGrpId, fromAssignments(assignments.nodes()));
+ }), replicaStartStopExecutor)
Review Comment:
Done
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -790,9 +794,49 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
* @param replicaGrpId Replication group ID.
* @param peersAndLearners New node configuration.
*/
- public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
+ public boolean resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
RaftNodeId raftNodeId = new RaftNodeId(replicaGrpId, new Peer(localNodeConsistentId));
- ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ return ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ }
+
+ /**
+ * Performs a {@code resetPeers} operation on raft node with retries.
+ *
+ * @param replicaGrpId Replication group ID.
+ * @param assignments New assignments.
+ */
+ public CompletableFuture<Void> resetWithRetry(ReplicationGroupId replicaGrpId, Assignments assignments) {
Review Comment:
Yes we do
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2543760213
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/PartitionReplicaLifecycleManager.java:
##########
@@ -1306,6 +1305,13 @@ private CompletableFuture<Void> handleChangePendingAssignmentEvent(Entry pending
zonePartitionId,
pendingAssignments.nodes(),
revision);
+ }).whenComplete((v, ex) -> {
+ if (ex != null) {
Review Comment:
made it debug. There is no other place where errors from change pending assignments futures are logged.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2543499540
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/PartitionReplicaLifecycleManager.java:
##########
@@ -1442,6 +1430,23 @@ private CompletableFuture<Void> handleChangePendingAssignmentEvent(
}), ioExecutor);
}
+ private static Assignments getComputedStableAssignments(@Nullable Assignments stableAssignments, Assignments pendingAssignments) {
Review Comment:
right. Extracted this branchy block of code to a separate method
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2543750733
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -788,7 +794,73 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
*/
public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners, long sequenceToken) {
RaftNodeId raftNodeId = new RaftNodeId(replicaGrpId, new Peer(localNodeConsistentId));
- ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners, sequenceToken);
+ Loza loza = (Loza) raftManager;
+ Status status = loza.resetPeers(raftNodeId, peersAndLearners, sequenceToken);
+
+ // Stale configuration change will not be retried.
+ if (!status.isOk() && status.getRaftError() == RaftError.ESTALE) {
+ // TODO: proper error.
+ throw new IgniteException(INTERNAL_ERR, new RaftException(status.getRaftError(), status.getErrorMsg()));
+ }
+ }
+
+ /**
+ * Performs a {@code resetPeers} operation on raft node with retries.
+ *
+ * @param replicaGrpId Replication group ID.
+ * @param peersAndLearners New peers and learners.
+ * @param sequenceToken Sequence token.
+ */
+ public CompletableFuture<Void> resetWithRetry(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners, long sequenceToken) {
+ var result = new CompletableFuture<Void>();
+
+ resetWithRetry(replicaGrpId, peersAndLearners, result, sequenceToken, 1);
+
+ return result;
+ }
+
+ private void resetWithRetry(
Review Comment:
added
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -788,7 +794,73 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
*/
public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners, long sequenceToken) {
Review Comment:
fixed
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc merged PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2524061465
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -790,9 +794,49 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
* @param replicaGrpId Replication group ID.
* @param peersAndLearners New node configuration.
*/
- public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
+ public boolean resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
RaftNodeId raftNodeId = new RaftNodeId(replicaGrpId, new Peer(localNodeConsistentId));
- ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ return ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ }
+
+ /**
+ * Performs a {@code resetPeers} operation on raft node with retries.
+ *
+ * @param replicaGrpId Replication group ID.
+ * @param assignments New assignments.
+ */
+ public CompletableFuture<Void> resetWithRetry(ReplicationGroupId replicaGrpId, Assignments assignments) {
+ return supplyAsync(() -> inBusyLock(busyLock, () -> {
+ assert isReplicaStarted(replicaGrpId) : "The local node is outside of the replication group: " + replicaGrpId;
+
+ return resetPeers(replicaGrpId, fromAssignments(assignments.nodes()));
+ }), replicaStartStopExecutor)
Review Comment:
renamed it
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -790,9 +794,49 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
* @param replicaGrpId Replication group ID.
* @param peersAndLearners New node configuration.
*/
- public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
+ public boolean resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
RaftNodeId raftNodeId = new RaftNodeId(replicaGrpId, new Peer(localNodeConsistentId));
- ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ return ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ }
+
+ /**
+ * Performs a {@code resetPeers} operation on raft node with retries.
+ *
+ * @param replicaGrpId Replication group ID.
+ * @param assignments New assignments.
+ */
+ public CompletableFuture<Void> resetWithRetry(ReplicationGroupId replicaGrpId, Assignments assignments) {
+ return supplyAsync(() -> inBusyLock(busyLock, () -> {
Review Comment:
Fixed by introducing sequence tokens
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2163877770
##########
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:
done
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "denis-chudov (via GitHub)" <gi...@apache.org>.
denis-chudov commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2164521810
##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/TableManagerRecoveryTest.java:
##########
@@ -325,6 +336,52 @@ public void raftListenersAreRecoveredOnRecovery() throws Exception {
.loadTableListenerToZoneReplica(any(), anyInt(), any(), any(), any(), eq(true));
}
+ @Test
+ public void testResetPeersRetry() throws Exception {
Review Comment:
now the test failed :)
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2166016815
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -790,9 +794,49 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
* @param replicaGrpId Replication group ID.
* @param peersAndLearners New node configuration.
*/
- public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
+ public boolean resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
RaftNodeId raftNodeId = new RaftNodeId(replicaGrpId, new Peer(localNodeConsistentId));
- ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ return ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ }
+
+ /**
+ * Performs a {@code resetPeers} operation on raft node with retries.
+ *
+ * @param replicaGrpId Replication group ID.
+ * @param assignments New assignments.
+ */
+ public CompletableFuture<Void> resetWithRetry(ReplicationGroupId replicaGrpId, Assignments assignments) {
+ return supplyAsync(() -> inBusyLock(busyLock, () -> {
Review Comment:
How do you guarantee proper ordering of resettingPeers?
I mean following situation:
1. pendingAssignments = [A] force -> reset is called in a async manner, thread hangs.
2. pendingAssignments = [B] force // e.g. from manual reset -> reset called in an async manner, applied
3. reset([A]) unhangs -> and overrides proper [B].
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2541059121
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -788,7 +794,73 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
*/
public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners, long sequenceToken) {
RaftNodeId raftNodeId = new RaftNodeId(replicaGrpId, new Peer(localNodeConsistentId));
- ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners, sequenceToken);
+ Loza loza = (Loza) raftManager;
+ Status status = loza.resetPeers(raftNodeId, peersAndLearners, sequenceToken);
+
+ // Stale configuration change will not be retried.
+ if (!status.isOk() && status.getRaftError() == RaftError.ESTALE) {
+ // TODO: proper error.
Review Comment:
TODO without link.
##########
modules/partition-distribution/src/main/java/org/apache/ignite/internal/partitiondistribution/PartitionDistributionUtils.java:
##########
@@ -76,4 +79,19 @@ public static Set<Assignment> calculateAssignmentForPartition(
return assignments.get(partitionId);
}
+ /**
+ * Checks if an error is recoverable, so we can retry a rebalance intent.
+ *
+ * @param t The throwable.
+ * @return {@code True} if this is a recoverable exception.
+ */
+ public static boolean recoverable(Throwable t) {
+ if (hasCause(t, NodeStoppingException.class, ComponentStoppingException.class)) {
+ return false;
+ }
+ // As long as we don't have a general failure handler, we assume that all errors are recoverable.
+ String message = t.getMessage();
+ return message == null || !message.contains("ESTALE:Provided configuration is stale");
Review Comment:
No way - it's fragile. If we need to detect whether it's a ESTALE we may introduce special internal exception class for that or any other robust approach that won't check exception message.
BTW, the comment `// As long as we don't have a general failure handler, we assume that all errors are recoverable.` is no longer valid.
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/PartitionReplicaLifecycleManager.java:
##########
@@ -1442,6 +1430,23 @@ private CompletableFuture<Void> handleChangePendingAssignmentEvent(
}), ioExecutor);
}
+ private static Assignments getComputedStableAssignments(@Nullable Assignments stableAssignments, Assignments pendingAssignments) {
Review Comment:
Just a move, nothing 've changed, right?
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/PartitionReplicaLifecycleManager.java:
##########
@@ -1306,6 +1305,13 @@ private CompletableFuture<Void> handleChangePendingAssignmentEvent(Entry pending
zonePartitionId,
pendingAssignments.nodes(),
revision);
+ }).whenComplete((v, ex) -> {
+ if (ex != null) {
Review Comment:
It's a tmp code that'll be reworked in https://issues.apache.org/jira/browse/IGNITE-23633, right?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2166009172
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -790,9 +794,49 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
* @param replicaGrpId Replication group ID.
* @param peersAndLearners New node configuration.
*/
- public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
+ public boolean resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
RaftNodeId raftNodeId = new RaftNodeId(replicaGrpId, new Peer(localNodeConsistentId));
- ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ return ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ }
+
+ /**
+ * Performs a {@code resetPeers} operation on raft node with retries.
+ *
+ * @param replicaGrpId Replication group ID.
+ * @param assignments New assignments.
+ */
+ public CompletableFuture<Void> resetWithRetry(ReplicationGroupId replicaGrpId, Assignments assignments) {
+ return supplyAsync(() -> inBusyLock(busyLock, () -> {
+ assert isReplicaStarted(replicaGrpId) : "The local node is outside of the replication group: " + replicaGrpId;
+
+ return resetPeers(replicaGrpId, fromAssignments(assignments.nodes()));
+ }), replicaStartStopExecutor)
Review Comment:
It's a less loaded one. And the real executor is actually the same as it was before.
Perhaps a better name for it would reduce the confusion.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2166036602
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageManagerImpl.java:
##########
@@ -1116,7 +1116,7 @@ private void onSafeTimeAdvanced(HybridTimestamp time) {
/**
Review Comment:
I guess we also call resetPeers in MetaStorageManagerImpl, we either should implement retry logic here or create a todo. Or which might be slightly better idea, move retry logic to the raft level, like we do in common sendWithRetry in order not to duplicate retry code.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2165977566
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -790,9 +794,49 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
* @param replicaGrpId Replication group ID.
* @param peersAndLearners New node configuration.
*/
- public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
+ public boolean resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
RaftNodeId raftNodeId = new RaftNodeId(replicaGrpId, new Peer(localNodeConsistentId));
- ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ return ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ }
+
+ /**
+ * Performs a {@code resetPeers} operation on raft node with retries.
+ *
+ * @param replicaGrpId Replication group ID.
+ * @param assignments New assignments.
+ */
+ public CompletableFuture<Void> resetWithRetry(ReplicationGroupId replicaGrpId, Assignments assignments) {
+ return supplyAsync(() -> inBusyLock(busyLock, () -> {
+ assert isReplicaStarted(replicaGrpId) : "The local node is outside of the replication group: " + replicaGrpId;
+
+ return resetPeers(replicaGrpId, fromAssignments(assignments.nodes()));
+ }), replicaStartStopExecutor)
Review Comment:
Why do we usr **startStop**Executor in order to send resetPeers?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2164802634
##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/TableManagerRecoveryTest.java:
##########
@@ -325,6 +336,52 @@ public void raftListenersAreRecoveredOnRecovery() throws Exception {
.loadTableListenerToZoneReplica(any(), anyInt(), any(), any(), any(), eq(true));
}
+ @Test
+ public void testResetPeersRetry() throws Exception {
Review Comment:
Grrr...Fixed that.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2546562577
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/PartitionReplicaLifecycleManager.java:
##########
@@ -1306,6 +1305,13 @@ private CompletableFuture<Void> handleChangePendingAssignmentEvent(Entry pending
zonePartitionId,
pendingAssignments.nodes(),
revision);
+ }).whenComplete((v, ex) -> {
+ if (ex != null) {
Review Comment:
It's a tmp code.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2524067399
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageManagerImpl.java:
##########
@@ -1116,7 +1116,7 @@ private void onSafeTimeAdvanced(HybridTimestamp time) {
/**
Review Comment:
I don't think it makes any sense now when MS groups do not have sequence token - bare retry will make things worse.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2166010980
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -790,9 +794,49 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
* @param replicaGrpId Replication group ID.
* @param peersAndLearners New node configuration.
*/
- public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
+ public boolean resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
RaftNodeId raftNodeId = new RaftNodeId(replicaGrpId, new Peer(localNodeConsistentId));
- ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ return ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ }
+
+ /**
+ * Performs a {@code resetPeers} operation on raft node with retries.
+ *
+ * @param replicaGrpId Replication group ID.
+ * @param assignments New assignments.
+ */
+ public CompletableFuture<Void> resetWithRetry(ReplicationGroupId replicaGrpId, Assignments assignments) {
Review Comment:
Do we block MS thread with resetWithRetry 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "denis-chudov (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2166018171
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -790,9 +794,49 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
* @param replicaGrpId Replication group ID.
* @param peersAndLearners New node configuration.
*/
- public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
+ public boolean resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
RaftNodeId raftNodeId = new RaftNodeId(replicaGrpId, new Peer(localNodeConsistentId));
- ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ return ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ }
+
+ /**
+ * Performs a {@code resetPeers} operation on raft node with retries.
+ *
+ * @param replicaGrpId Replication group ID.
+ * @param assignments New assignments.
+ */
+ public CompletableFuture<Void> resetWithRetry(ReplicationGroupId replicaGrpId, Assignments assignments) {
+ return supplyAsync(() -> inBusyLock(busyLock, () -> {
+ assert isReplicaStarted(replicaGrpId) : "The local node is outside of the replication group: " + replicaGrpId;
+
+ return resetPeers(replicaGrpId, fromAssignments(assignments.nodes()));
+ }), replicaStartStopExecutor)
Review Comment:
Should we add throttling here, e.g. like we do in raft sendWithRetry?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2167398274
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -790,9 +794,49 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
* @param replicaGrpId Replication group ID.
* @param peersAndLearners New node configuration.
*/
- public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
+ public boolean resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
RaftNodeId raftNodeId = new RaftNodeId(replicaGrpId, new Peer(localNodeConsistentId));
- ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ return ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ }
+
+ /**
+ * Performs a {@code resetPeers} operation on raft node with retries.
+ *
+ * @param replicaGrpId Replication group ID.
+ * @param assignments New assignments.
+ */
+ public CompletableFuture<Void> resetWithRetry(ReplicationGroupId replicaGrpId, Assignments assignments) {
+ return supplyAsync(() -> inBusyLock(busyLock, () -> {
+ assert isReplicaStarted(replicaGrpId) : "The local node is outside of the replication group: " + replicaGrpId;
+
+ return resetPeers(replicaGrpId, fromAssignments(assignments.nodes()));
+ }), replicaStartStopExecutor)
+ .handleAsync((resetSuccessful, ex) -> {
+ if (ex != null) {
+ if (isRetriable(ex)) {
+ LOG.debug("Failed to reset peers. Retrying [groupId={}]. ", replicaGrpId, ex);
Review Comment:
added
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2163877295
##########
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:
Changed to debug
##########
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:
done
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2164310807
##########
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:
done
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2166019992
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -790,9 +794,49 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
* @param replicaGrpId Replication group ID.
* @param peersAndLearners New node configuration.
*/
- public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
+ public boolean resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners) {
RaftNodeId raftNodeId = new RaftNodeId(replicaGrpId, new Peer(localNodeConsistentId));
- ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ return ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners);
+ }
+
+ /**
+ * Performs a {@code resetPeers} operation on raft node with retries.
+ *
+ * @param replicaGrpId Replication group ID.
+ * @param assignments New assignments.
+ */
+ public CompletableFuture<Void> resetWithRetry(ReplicationGroupId replicaGrpId, Assignments assignments) {
+ return supplyAsync(() -> inBusyLock(busyLock, () -> {
+ assert isReplicaStarted(replicaGrpId) : "The local node is outside of the replication group: " + replicaGrpId;
+
+ return resetPeers(replicaGrpId, fromAssignments(assignments.nodes()));
+ }), replicaStartStopExecutor)
+ .handleAsync((resetSuccessful, ex) -> {
+ if (ex != null) {
+ if (isRetriable(ex)) {
+ LOG.debug("Failed to reset peers. Retrying [groupId={}]. ", replicaGrpId, ex);
Review Comment:
I'd also add a counter and add warn logging on each 100 or 1000, just in case.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2541499892
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -788,7 +794,73 @@ public CompletableFuture<Replica> replica(ReplicationGroupId replicationGroupId)
*/
public void resetPeers(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners, long sequenceToken) {
RaftNodeId raftNodeId = new RaftNodeId(replicaGrpId, new Peer(localNodeConsistentId));
- ((Loza) raftManager).resetPeers(raftNodeId, peersAndLearners, sequenceToken);
+ Loza loza = (Loza) raftManager;
+ Status status = loza.resetPeers(raftNodeId, peersAndLearners, sequenceToken);
+
+ // Stale configuration change will not be retried.
+ if (!status.isOk() && status.getRaftError() == RaftError.ESTALE) {
+ // TODO: proper error.
+ throw new IgniteException(INTERNAL_ERR, new RaftException(status.getRaftError(), status.getErrorMsg()));
+ }
+ }
+
+ /**
+ * Performs a {@code resetPeers} operation on raft node with retries.
+ *
+ * @param replicaGrpId Replication group ID.
+ * @param peersAndLearners New peers and learners.
+ * @param sequenceToken Sequence token.
+ */
+ public CompletableFuture<Void> resetWithRetry(ReplicationGroupId replicaGrpId, PeersAndLearners peersAndLearners, long sequenceToken) {
+ var result = new CompletableFuture<Void>();
+
+ resetWithRetry(replicaGrpId, peersAndLearners, result, sequenceToken, 1);
+
+ return result;
+ }
+
+ private void resetWithRetry(
Review Comment:
Please add a comment explaining why it's safe to perform resetWithRetry in async manner without awaiting the result.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25543 Implement retry mechanism for reset [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #6053:
URL: https://github.com/apache/ignite-3/pull/6053#discussion_r2543761141
##########
modules/partition-distribution/src/main/java/org/apache/ignite/internal/partitiondistribution/PartitionDistributionUtils.java:
##########
@@ -76,4 +79,19 @@ public static Set<Assignment> calculateAssignmentForPartition(
return assignments.get(partitionId);
}
+ /**
+ * Checks if an error is recoverable, so we can retry a rebalance intent.
+ *
+ * @param t The throwable.
+ * @return {@code True} if this is a recoverable exception.
+ */
+ public static boolean recoverable(Throwable t) {
+ if (hasCause(t, NodeStoppingException.class, ComponentStoppingException.class)) {
+ return false;
+ }
+ // As long as we don't have a general failure handler, we assume that all errors are recoverable.
+ String message = t.getMessage();
+ return message == null || !message.contains("ESTALE:Provided configuration is stale");
Review Comment:
Created a new error 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org