[GitHub] [geode] boglesby commented on pull request #6445: GEODE-9074: Added update of messageQueueSize at putting message to qu…

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

boglesby commented on pull request #6445:
URL: https://github.com/apache/geode/pull/6445#issuecomment-872444288


   There are a couple other stats along with this one would be useful (in progress and time).
   
   Its not really messages waiting to be queued. Its messages being queued. I think these names are better (but I'm no expert on naming things):
   
   - messagesBeingQueued
   - messagesBeingQueuedInProgress
   - messagesBeingQueuedTime
   
   A common pattern for updating several stats like this is:
   ```
   long startTime = _proxy._statistics.startPut();
   _messageQueue.put(message);
   _proxy._statistics.endPut(_messageQueue.size(), startTime);
   ```
   startPut would:
   
   - increment messages being queued
   - increment messages being queued in progress
   - return the start time (which would be non-0 if enable-time-statistics=true)
   
   endPut would:
   
   - decrement messages being queued
   - decrement messages being queued in progress
   - increment messages being queued
   - set queue size
   
   Here is an example in CachePerfStats for CacheWriter calls:
   ```
   public long startCacheWriterCall() {
     stats.incLong(cacheWriterCallsInProgressId, 1);
     return getTime();
   }
   
   public void endCacheWriterCall(long start) {
     if (clock.isEnabled()) {
       stats.incLong(cacheWriterCallTimeId, getTime() - start);
     }
     stats.incLong(cacheWriterCallsInProgressId, -1);
     stats.incLong(cacheWriterCallsCompletedId, 1);
   }
   ```
   Do we know what kind of performance impact this change will have?
   
   HARegionQueue.size gets a readLock:
   ```
   public int size() {
     acquireReadLock();
     try {
       return this.idsAvailable.size();
     } finally {
       releaseReadLock();
     }
   }
   ```
   Here are a few nit-picky comments. Feel free to ignore them.
   
   Need a period at the end of the description:
   ```
   Threads currently adding a message to the queue. Consistently high values indicate that the queue is full and adds are being delayed
   ```
   There are extra spaces at various places in the CacheClientProxyStats class.
   
   


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