droudnitsky commented on code in PR #6961:
URL: https://github.com/apache/hbase/pull/6961#discussion_r2094177695


##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRequestFutureImpl.java:
##########
@@ -783,8 +787,7 @@ private void receiveGlobalFailure(MultiAction rsActions, 
ServerName server, int
       // any of the regions in the MultiAction and do not update cache if 
exception is
       // from failing to submit action to thread pool
       if (clearServerCache) {
-        updateCachedLocations(server, regionName, row,
-          ClientExceptionsUtil.isMetaClearingException(t) ? null : t);
+        updateCachedLocations(server, regionName, row, t);

Review Comment:
   +1 would be good to preserve the exception for `updateCachedLocations`
   
   Since we currently pass null to `updateCachedLocations` if we have a meta 
cache clearing exception, does that means that we never [update the cache 
clearing exception metric properly 
](https://github.com/apache/hbase/blob/rel/2.5.11/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java#L2125)
 for cache clears coming from `receiveGlobalFailure`?



##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRequestFutureImpl.java:
##########
@@ -783,8 +787,7 @@ private void receiveGlobalFailure(MultiAction rsActions, 
ServerName server, int
       // any of the regions in the MultiAction and do not update cache if 
exception is
       // from failing to submit action to thread pool
       if (clearServerCache) {
-        updateCachedLocations(server, regionName, row,

Review Comment:
   hey @hgromer . From what I understand , the 
`ClientExceptionsUtil.isMetaClearingException(t) ? null : t` is what prevents 
the meta cache clear from happening deeper in `updateCachedLocations` for 
exceptions for which we should not clear the meta cache. If its a meta clearing 
exception we pass null and bypass [this 
check](https://github.com/apache/hbase/blob/rel/2.5.11/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java#L2105)
 and clear the meta cache , if its not a meta cache clearing exception we pass 
the exception and the check to bypass the meta cache clear if its not a meta 
cache clearing exception happens in `updateCachedLocations` 
[here](https://github.com/apache/hbase/blob/rel/2.5.11/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java#L2106-L2109)
   
   I may have misunderstand or have missed something, could you possibly add a 
test to show that on batch operation a non meta cache clearing exception is 
causing a meta cache clear through `receiveGlobalFailure`? There are some 
existing meta cache clear behavior tests you could reference. 



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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to