DonalEvans commented on a change in pull request #7358:
URL: https://github.com/apache/geode/pull/7358#discussion_r816332848
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/RedisStats.java
##########
@@ -194,43 +199,46 @@ public void changeUniquePatternSubscriptions(long delta) {
public void close() {
geodeRedisStats.close();
- stopPerSecondUpdater();
+ stopRollingAverageUpdater();
}
- private ScheduledExecutorService startPerSecondUpdater() {
- int INTERVAL = 1;
+ private ScheduledExecutorService startRollingAverageUpdater() {
+ long microsPerSecond = 1_000_000;
+ final long delayMicros = microsPerSecond /
ROLLING_AVERAGE_SAMPLES_PER_SECOND;
- ScheduledExecutorService perSecondExecutor =
- newSingleThreadScheduledExecutor("GemFireRedis-PerSecondUpdater-");
+ ScheduledExecutorService rollingAverageExecutor =
+
newSingleThreadScheduledExecutor("GemFireRedis-RollingAverageStatUpdater-");
- perSecondExecutor.scheduleWithFixedDelay(
- this::doPerSecondUpdates,
- INTERVAL,
- INTERVAL,
- SECONDS);
+
rollingAverageExecutor.scheduleWithFixedDelay(this::doRollingAverageUpdates,
delayMicros,
+ delayMicros, MICROSECONDS);
- return perSecondExecutor;
+ return rollingAverageExecutor;
}
- private void stopPerSecondUpdater() {
- perSecondExecutor.shutdownNow();
+ private void stopRollingAverageUpdater() {
+ rollingAverageExecutor.shutdownNow();
}
- private void doPerSecondUpdates() {
- updateNetworkKilobytesReadLastSecond();
- updateOpsPerformedOverLastSecond();
+ private void doRollingAverageUpdates() {
+ networkKiloBytesReadOverLastSecond = networkBytesReadRollingAverageStat
+ .calculate(getTotalNetworkBytesRead(), rollingAverageTick) / 1024.0;
+ opsPerformedOverLastSecond =
+ opsPerformedRollingAverageStat.calculate(getCommandsProcessed(),
rollingAverageTick);
+ rollingAverageTick++;
Review comment:
I like this idea. Since the tick count is inherently tied to the samples
per second, which is also used when determining the delay for the
`ScheduledExecutorService`, it's not possible to have different instances of
`RollingAverageStat` with different number of samples to average over without
also having a different instance of `ScheduledExecutorService` to execute them,
but it still cleans things up to move the tick logic into `RollingAverageStat`.
To make the `RollingAverageStat` instances more self-contained, I also moved
the call to get the current stat value inside the class, so all you need to do
is create an instance, then call a zero-argument `calculate()` method to get
the latest average.
--
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]
