[GitHub] [lucene] zhaih commented on a diff in pull request #11822: PrimaryNode: add configurable timeout to waitForAllRemotesToClose

[GitBox <gi...@apache.org>]

zhaih commented on code in PR #11822:
URL: https://github.com/apache/lucene/pull/11822#discussion_r980742864


##########
lucene/replicator/src/java/org/apache/lucene/replicator/nrt/PrimaryNode.java:
##########
@@ -61,6 +61,7 @@ public abstract class PrimaryNode extends Node {
   private CopyState copyState;
 
   protected final long primaryGen;
+  private int remoteCloseTimeout = -1;

Review Comment:
   It'd better to name it `remoteCloseTimeoutMs`? And also change the getter and setter correspondingly?



##########
lucene/replicator/src/java/org/apache/lucene/replicator/nrt/PrimaryNode.java:
##########
@@ -196,6 +197,21 @@ public synchronized long getLastCommitVersion() {
     throw new AssertionError("missing VERSION_KEY");
   }
 
+  /**
+   * @return the number of milliseconds to wait during shutdown for remote replicas to close
+   */
+  public int getRemoteCloseTimeout() {
+    return remoteCloseTimeout;
+  }
+
+  /**
+   * Set the number of milliseconds to wait during shutdown for remote replicas to close. {@code -1}
+   * (the default) means forever, and {@code 0} means don't wait at all.
+   */
+  public void setRemoteCloseTimeout(int remoteCloseTimeout) {

Review Comment:
   Can we add some unit tests for at least `remoteCloseTimeout = 0` case? Like produce some error or just interrupt the replicaNode and then close the primaryNode?



-- 
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: issues-unsubscribe@lucene.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org