[GitHub] [geode] jchen21 commented on a change in pull request #6887: GEODE-9567: Geode for Redis rename

[GitBox <gi...@apache.org>]

jchen21 commented on a change in pull request #6887:
URL: https://github.com/apache/geode/pull/6887#discussion_r721592601



##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java
##########
@@ -294,6 +294,16 @@ private void setEventSeqNum() {
     }
   }
 
+  @Override

Review comment:
       The code here is not simply a rename of something, as the title of the pull request suggests. In the pull request, there are changes for the stats. I would suggest put the stat code change in a separate pull request, instead of including them in the 600+ files diff, which are mostly about renaming.
   
   I am not sure on what scenario this `startGet()` and `endGet()` on `BucketRegion` would be invoked. Why `startGet()` returns `0`, while `endGet()` does nothing. This doesn't seem right to me. And this overrides the method in `LocalRegion`. Maybe I am missing something. I can't find any description in the JIRA or pull request for stat related changes. Would you please explain a bit?




-- 
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: notifications-unsubscribe@geode.apache.org

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