dlmarion commented on code in PR #5058:
URL: https://github.com/apache/accumulo/pull/5058#discussion_r1841003260
##########
core/src/main/java/org/apache/accumulo/core/lock/ServiceLockPaths.java:
##########
@@ -455,14 +468,34 @@ private Set<ServiceLockPath> get(final String serverType,
// Dead TServers don't have lock data
results.add(slp);
} else {
- final ZcStat stat = new ZcStat();
- Optional<ServiceLockData> sld = ServiceLock.getLockData(cache,
slp, stat);
- if (!sld.isEmpty()) {
- results.add(slp);
- }
+ // Execute reads to zookeeper to get lock info in parallel.
The zookeeper client has
+ // a single shared connection to a server so this will not
create lots of
+ // connections, it will place multiple outgoing request on
that single zookeeper
+ // connection at the same time though.
+ futures.add(executor.submit(() -> {
+ final ZcStat stat = new ZcStat();
+ Optional<ServiceLockData> sld =
ServiceLock.getLockData(cache, slp, stat);
+ if (sld.isPresent()) {
+ results.add(slp);
+ }
+ return null;
+ }));
}
}
}
+
+ // wait for futures to complete and check for errors
+ for (var future : futures) {
+ try {
+ future.get();
+ } catch (InterruptedException | ExecutionException e) {
+ throw new IllegalStateException(e);
+ }
+ }
Review Comment:
I don't think this will wait for all futures to return. Don't you need
something like:
```
while (futures.size() > 0) {
Iterator<Future<?>> iter = futures.iterator();
while (iter.hasNext()) {
Future<?> future = iter.next();
if (future.isDone()) {
iter.remove();
try {
future.get();
} catch (InterruptedException | ExecutionException e) {
throw new IllegalStateException(e);
}
}
}
if (futures.size() > 0) {
UtilWaitThread.sleep(3_000);
}
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
