ctubbsii commented on code in PR #3788:
URL: https://github.com/apache/accumulo/pull/3788#discussion_r1340747373
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -68,13 +64,9 @@ default Pair<String,C> getTabletServerConnection(Logger LOG,
ThriftClientTypes<C
for (String tserver : zc.getChildren(context.getZooKeeperRoot() +
Constants.ZTSERVERS)) {
var zLocPath =
ServiceLock.path(context.getZooKeeperRoot() + Constants.ZTSERVERS +
"/" + tserver);
- Optional<ServiceLockData> sld = zc.getLockData(zLocPath);
- if (sld.isPresent()) {
- HostAndPort address =
sld.orElseThrow().getAddress(ThriftService.TSERV);
- if (address != null) {
- servers.add(new ThriftTransportKey(address, rpcTimeout, context));
- }
- }
+ zc.getLockData(zLocPath).map(sld -> sld.getAddress(ThriftService.TSERV))
Review Comment:
It could be possible that lock data was serialized to that node, but did not
have the TSERV service registered (that would probably be a bigger bug if that
were to happen). If that were to happen before my fix, this method would have
blown up with an NPE in a similar way to the original reported issue. Now, the
address would be null, and it simply won't get added to the list of detected
servers... which was the original intent with the null check, as demonstrated
by the way it behaved in previous versions of Accumulo (I consulted 2.1
behavior when making these changes).
--
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]
