mikemccand commented on a change in pull request #1585:
URL: https://github.com/apache/lucene-solr/pull/1585#discussion_r441559509
##########
File path: lucene/core/src/java/org/apache/lucene/index/MergePolicy.java
##########
@@ -196,6 +200,7 @@ final void setMergeThread(Thread owner) {
*
* @lucene.experimental */
public static class OneMerge {
+ private final CompletableFuture<Boolean> completable = new
CompletableFuture<>();
Review comment:
Could we rename the variable to `completed` or maybe `mergeCompleted`?
##########
File path: lucene/core/src/java/org/apache/lucene/index/MergePolicy.java
##########
@@ -196,6 +200,7 @@ final void setMergeThread(Thread owner) {
*
* @lucene.experimental */
public static class OneMerge {
Review comment:
Maybe we should (later, in dedicated PR) pull this out into its own java
source?
##########
File path: lucene/core/src/java/org/apache/lucene/index/MergePolicy.java
##########
@@ -362,6 +366,30 @@ public void checkAborted() throws MergeAbortedException {
public OneMergeProgress getMergeProgress() {
return mergeProgress;
}
+
+ /**
+ * Waits for this merge to be completed
+ * @return true if the merge finished within the specified timeout
+ */
+ boolean await(long timeout, TimeUnit timeUnit) {
+ try {
+ completable.get(timeout, timeUnit);
+ return true;
+ } catch (InterruptedException e) {
+ Thread.interrupted();
+ return false;
+ } catch (ExecutionException | TimeoutException e) {
+ return false;
+ }
+ }
+
+ boolean isDone() {
Review comment:
Maybe add javadoc about the lack of thread safety here? I.e. this could
return `false` and shortly thereafter it becomes `true`.
##########
File path: lucene/core/src/java/org/apache/lucene/index/MergePolicy.java
##########
@@ -362,6 +366,30 @@ public void checkAborted() throws MergeAbortedException {
public OneMergeProgress getMergeProgress() {
return mergeProgress;
}
+
+ /**
+ * Waits for this merge to be completed
+ * @return true if the merge finished within the specified timeout
+ */
+ boolean await(long timeout, TimeUnit timeUnit) {
+ try {
+ completable.get(timeout, timeUnit);
+ return true;
+ } catch (InterruptedException e) {
+ Thread.interrupted();
+ return false;
+ } catch (ExecutionException | TimeoutException e) {
+ return false;
+ }
+ }
+
+ boolean isDone() {
+ return completable.isDone();
+ }
+
+ boolean isCommitted() {
Review comment:
Maybe rename to `isCompleted`? `committed` is overloaded term -- could
try to mean its files got `fsync'`d :)
##########
File path: lucene/core/src/java/org/apache/lucene/index/MergePolicy.java
##########
@@ -362,6 +366,30 @@ public void checkAborted() throws MergeAbortedException {
public OneMergeProgress getMergeProgress() {
return mergeProgress;
}
+
+ /**
+ * Waits for this merge to be completed
+ * @return true if the merge finished within the specified timeout
+ */
+ boolean await(long timeout, TimeUnit timeUnit) {
+ try {
+ completable.get(timeout, timeUnit);
+ return true;
+ } catch (InterruptedException e) {
+ Thread.interrupted();
+ return false;
+ } catch (ExecutionException | TimeoutException e) {
+ return false;
+ }
+ }
+
+ boolean isDone() {
+ return completable.isDone();
+ }
+
+ boolean isCommitted() {
+ return completable.getNow(Boolean.FALSE);
Review comment:
Javadoc here too?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
