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



##########
File path: 
geode-core/src/main/java/org/apache/geode/cache/asyncqueue/internal/InternalAsyncEventQueue.java
##########
@@ -28,4 +28,6 @@
   void stop();
 
   void destroy();
+
+  boolean isPartitionedRegionClearUnsupported();

Review comment:
       Is it necessary to add the method signature here in the interface? I 
don't see much benefit here. It is only implemented in `AsyncEventQueueImpl`. 
The return value of `partitionedRegion.getCache()
             .getAsyncEventQueues(false)` has to be casted to  
`AsyncEventQueueImpl` or `InternalAsyncEventQueue`.

##########
File path: 
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
##########
@@ -8079,8 +8079,12 @@ public DistributionAdvisee getParentAdvisee() {
 
     private final long maxTimeInRetry;
 
-    public RetryTimeKeeper(int maxTime) {
-      this.maxTimeInRetry = maxTime;
+    public RetryTimeKeeper(int maxTimeInRetry) {

Review comment:
       If there is a `RetryTimeKeeper` that takes `long` parameter, is this 
method with `int` parameter needed?

##########
File path: 
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
##########
@@ -8129,6 +8133,10 @@ public boolean overMaximum() {
     public long getRetryTime() {
       return this.totalTimeInRetry;
     }
+
+    public void reset() {

Review comment:
       Is `reset()` needed? I see the usage of `RetryTimeKeeper` with `reset()` 
is different from other existing usages `RetryTimeKeeper`. The existing usages 
use `maxTimeInRetry` as an upper bound of accumulated time for all retries. You 
are using `maxTimeInRetry` as an upper bound for each retry attempt.




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