You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2022/03/11 18:19:55 UTC

[GitHub] [geode] jdeppe-pivotal commented on a change in pull request #7437: GEODE-10010: Refine per-second Redis stats

jdeppe-pivotal commented on a change in pull request #7437:
URL: https://github.com/apache/geode/pull/7437#discussion_r824963215



##########
File path: geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/RedisStats.java
##########
@@ -193,43 +198,57 @@ 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() / 1024.0;
+    opsPerformedOverLastSecond = opsPerformedRollingAverageStat.calculate();
   }
 
-  private void updateNetworkKilobytesReadLastSecond() {
-    final long totalNetworkBytesRead = getTotalNetworkBytesRead();
-    long deltaNetworkBytesRead = totalNetworkBytesRead - previousNetworkBytesRead;
-    networkKiloBytesReadOverLastSecond = deltaNetworkBytesRead / 1024.0;
-    previousNetworkBytesRead = totalNetworkBytesRead;
-  }
+  private static class RollingAverageStat {
+    private int tickNumber = 0;
+    private long valueReadLastTick;
+    private final long[] valuesReadOverLastNSamples;
+    private final Callable<Long> statCallable;
+
+    private RollingAverageStat(int samplesPerSecond, Callable<Long> getCurrentValue) {
+      valuesReadOverLastNSamples = new long[samplesPerSecond];
+      statCallable = getCurrentValue;
+    }
 
-  private void updateOpsPerformedOverLastSecond() {
-    final long totalOpsPerformed = getCommandsProcessed();
-    opsPerformedOverLastSecond = totalOpsPerformed - opsPerformedLastTick;
-    opsPerformedLastTick = totalOpsPerformed;
+    private long calculate() {
+      long currentValue;
+      try {
+        currentValue = statCallable.call();
+      } catch (Exception e) {
+        throw new RedisException("Error while calculating RollingAverage stats", e);
+      }
+      long delta = currentValue - valueReadLastTick;
+      valueReadLastTick = currentValue;
+      valuesReadOverLastNSamples[tickNumber] = delta;
+      tickNumber++;
+      if (tickNumber >= valuesReadOverLastNSamples.length) {
+        tickNumber = 0;
+      }
+      return Arrays.stream(valuesReadOverLastNSamples).sum();
+    }

Review comment:
       For fun I did some quick benchmarking and came up with this optimization:
   ```
       long currentTotal = 0;
       public long calculate2() {
         long currentValue;
         try {
           currentValue = statCallable.call();
         } catch (Exception e) {
           throw new RedisException("Error while calculating RollingAverage stats", e);
         }
         long delta = currentValue - valueReadLastTick;
         valueReadLastTick = currentValue;
   
         currentTotal = currentTotal + delta - valuesReadOverLastNSamples[tickNumber];
         valuesReadOverLastNSamples[tickNumber] = delta;
         // same as mod (%) but measurably faster...
         tickNumber = (tickNumber + 1) & (valuesReadOverLastNSamples.length - 1);
   
         return currentTotal;
       }
   ```
   This runs more than an order of magnitude faster. Granted, the benchmark was hammering this as fast as possible so at only 16 times / second the optimization would be negligible. Your call if you want to do it.

##########
File path: geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/RedisStats.java
##########
@@ -193,43 +198,57 @@ 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() / 1024.0;
+    opsPerformedOverLastSecond = opsPerformedRollingAverageStat.calculate();
   }
 
-  private void updateNetworkKilobytesReadLastSecond() {
-    final long totalNetworkBytesRead = getTotalNetworkBytesRead();
-    long deltaNetworkBytesRead = totalNetworkBytesRead - previousNetworkBytesRead;
-    networkKiloBytesReadOverLastSecond = deltaNetworkBytesRead / 1024.0;
-    previousNetworkBytesRead = totalNetworkBytesRead;
-  }
+  private static class RollingAverageStat {
+    private int tickNumber = 0;
+    private long valueReadLastTick;
+    private final long[] valuesReadOverLastNSamples;
+    private final Callable<Long> statCallable;
+
+    private RollingAverageStat(int samplesPerSecond, Callable<Long> getCurrentValue) {
+      valuesReadOverLastNSamples = new long[samplesPerSecond];
+      statCallable = getCurrentValue;
+    }
 
-  private void updateOpsPerformedOverLastSecond() {
-    final long totalOpsPerformed = getCommandsProcessed();
-    opsPerformedOverLastSecond = totalOpsPerformed - opsPerformedLastTick;
-    opsPerformedLastTick = totalOpsPerformed;
+    private long calculate() {
+      long currentValue;
+      try {
+        currentValue = statCallable.call();
+      } catch (Exception e) {
+        throw new RedisException("Error while calculating RollingAverage stats", e);
+      }
+      long delta = currentValue - valueReadLastTick;
+      valueReadLastTick = currentValue;
+      valuesReadOverLastNSamples[tickNumber] = delta;
+      tickNumber++;
+      if (tickNumber >= valuesReadOverLastNSamples.length) {
+        tickNumber = 0;
+      }
+      return Arrays.stream(valuesReadOverLastNSamples).sum();
+    }

Review comment:
       For fun I did some quick benchmarking and came up with this optimization:
   ```
       long currentTotal = 0;
       public long calculate2() {
         long currentValue;
         try {
           currentValue = statCallable.call();
         } catch (Exception e) {
           throw new RedisException("Error while calculating RollingAverage stats", e);
         }
         long delta = currentValue - valueReadLastTick;
         valueReadLastTick = currentValue;
   
         currentTotal = currentTotal + delta - valuesReadOverLastNSamples[tickNumber];
         valuesReadOverLastNSamples[tickNumber] = delta;
         // same as mod (%) but measurably faster...
         tickNumber = (tickNumber + 1) & (valuesReadOverLastNSamples.length - 1);
   
         return currentTotal;
       }
   ```
   This runs more than an order of magnitude faster. Granted, the benchmark was hammering this as fast as possible so at only 16 times / second the optimization would be negligible. Your call if you want to do it.




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