ctubbsii commented on code in PR #3788:
URL: https://github.com/apache/accumulo/pull/3788#discussion_r1340743591
##########
server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java:
##########
@@ -428,10 +428,11 @@ private GCStatus fetchGcStatus() {
var path = ServiceLock.path(context.getZooKeeperRoot() +
Constants.ZGC_LOCK);
List<String> locks = ServiceLock.validateAndSort(path,
zk.getChildren(path.toString()));
if (locks != null && !locks.isEmpty()) {
- Optional<ServiceLockData> sld =
- ServiceLockData.parse(zk.getData(path + "/" + locks.get(0)));
- if (sld.isPresent()) {
- address = sld.orElseThrow().getAddress(ThriftService.GC);
+ address = ServiceLockData.parse(zk.getData(path + "/" + locks.get(0)))
+ .map(sld -> sld.getAddress(ThriftService.GC)).orElse(null);
+ if (address == null) {
+ log.warn("Unable to contact the garbage collector (no address)");
+ return null;
Review Comment:
Before this change, this would have resulted in attempting to make a
connection to a server using null for the address. That would have resulted in
some exception, though I don't know off the top of my head whether that would
be an NPE or an IOException, and caught further down, which would have resulted
in a similar message, but reporting that it failed to connect to the GC at the
"null" address. This just short-circuits that attempt to connect using a null
address and fail later with a non-sensical message. It's not strictly
necessary, but I thought it'd be a better behavior.
--
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]
