shubhamvishu commented on code in PR #12799:
URL: https://github.com/apache/lucene/pull/12799#discussion_r1399120194


##########
lucene/core/src/java/org/apache/lucene/util/hnsw/HnswConcurrentMergeBuilder.java:
##########
@@ -77,42 +75,17 @@ public OnHeapHnswGraph build(int maxOrd) throws IOException 
{
           HNSW_COMPONENT,
           "build graph from " + maxOrd + " vectors, with " + workers.length + 
" workers");
     }
-    List<Future<?>> futures = new ArrayList<>();
+    List<Callable<Void>> futures = new ArrayList<>();
     for (int i = 0; i < workers.length; i++) {
       int finalI = i;
       futures.add(
-          exec.submit(
-              () -> {
-                try {
-                  workers[finalI].run(maxOrd);
-                } catch (IOException e) {
-                  throw new RuntimeException(e);
-                }
-              }));
-    }
-    Throwable exc = null;
-    for (Future<?> future : futures) {
-      try {
-        future.get();
-      } catch (InterruptedException e) {
-        var newException = new ThreadInterruptedException(e);
-        if (exc == null) {
-          exc = newException;
-        } else {
-          exc.addSuppressed(newException);
-        }
-      } catch (ExecutionException e) {
-        if (exc == null) {
-          exc = e.getCause();
-        } else {
-          exc.addSuppressed(e.getCause());
-        }
-      }
-    }
-    if (exc != null) {
-      // The error handling was copied from TaskExecutor. should we just use 
TaskExecutor instead?
-      throw IOUtils.rethrowAlways(exc);
+          () -> {
+            workers[finalI].run(maxOrd);
+            return null;
+          });
     }
+    TaskExecutor taskExecutor = new TaskExecutor(exec);

Review Comment:
   > Would it make sense though to carry around the TaskExecutor instance 
instead of the executor service, and create it earlier as soon as the executor 
is provided initially as a constructor argument?
   
   Yes, I would prefer it that way(its also similar to `IndexSearcher`). I have 
that change ready.



-- 
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-unsubscr...@lucene.apache.org

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

Reply via email to