frostruan commented on code in PR #4335:
URL: https://github.com/apache/hbase/pull/4335#discussion_r848582891


##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTableRegionLocatorImpl.java:
##########
@@ -62,7 +62,10 @@ public CompletableFuture<List<HRegionLocation>> 
getAllRegionLocations() {
           .thenApply(locs -> Arrays.asList(locs.getRegionLocations()));
       }
       return ClientMetaTableAccessor
-        .getTableHRegionLocations(conn.getTable(TableName.META_TABLE_NAME), 
tableName);
+        .getTableHRegionLocations(conn.getTable(TableName.META_TABLE_NAME), 
tableName)
+        .whenComplete((locs, error) -> {

Review Comment:
   Thanks for your great suggestions.
   
   From my understanding, if the error is not null, the get method on this 
future will get an ExecutionException, the region location caching code will 
not run, therefore there is no NPR problem here (Correct me if I am wrong). 
Based on this consideration I didn't handle the case error != null. But I agree 
that it's better to use FutureUtils.addListener. Thanks for reminding me about 
this. I'll address it as soon as possible.
   
   Also, Congratulations on being HBase committer. @bbeaudreault 



-- 
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]

Reply via email to