dschneider-pivotal commented on a change in pull request #6445:
URL: https://github.com/apache/geode/pull/6445#discussion_r645143885
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientProxyStats.java
##########
@@ -128,7 +133,10 @@
"operations"),
f.createLongCounter(CQ_COUNT, "Number of CQs on the client.",
"operations"),
- f.createLongCounter("sentBytes", "Total number of bytes sent to
client.", "bytes"),});
+ f.createLongCounter("sentBytes", "Total number of bytes sent to
client.", "bytes"),
+ f.createIntCounter(NUMBER_THREADS_PUT_IN_QUEUE,
+ "Number of threads currently adding message in queue",
"operations"),
Review comment:
I would change this description slightly to "Threads currently adding a
message to the queue. Consistently high values indicate that the queue is full
and adds are being delayed". You might want to change the units to be "threads".
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientProxyStats.java
##########
@@ -60,6 +60,8 @@
private static final String DELTA_FULL_MESSAGES_SENT =
"deltaFullMessagesSent";
/** Name of the CQ count statistic */
private static final String CQ_COUNT = "cqCount";
+ /** Name of the number of threads currently adding message in queue
statistic */
+ private static final String NUMBER_THREADS_PUT_IN_QUEUE =
"numberThreadsPutInQueue";
Review comment:
"number" is not needed in the name since all stats are numeric. Given
that we have a stat named "messagesQueued", I think this one should be named
"messagesWaitingToQueue".
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientProxyStats.java
##########
@@ -128,7 +133,10 @@
"operations"),
f.createLongCounter(CQ_COUNT, "Number of CQs on the client.",
"operations"),
- f.createLongCounter("sentBytes", "Total number of bytes sent to
client.", "bytes"),});
+ f.createLongCounter("sentBytes", "Total number of bytes sent to
client.", "bytes"),
+ f.createIntCounter(NUMBER_THREADS_PUT_IN_QUEUE,
Review comment:
This should be a gauge not a counter. Counters always increase. Also all
the "int" ones are deprecated so just make it "createLongGauge"
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientProxyStats.java
##########
@@ -313,19 +332,38 @@ public void incCqCount() {
this._stats.incInt(_cqCountId, 1);
}
+ /**
+ * Increments the "numberThreadsPutInQueue" stat.
+ */
+ public void incNumberThreadsPutInQueue() {
+ synchronized (this) {
Review comment:
CacheClientProxyStats are atomic so you should not need synchronization
(I saw it in three places: inc, dec, and set).
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
