dschneider-pivotal commented on a change in pull request #7363:
URL: https://github.com/apache/geode/pull/7363#discussion_r807245028
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/GeodeRedisStats.java
##########
@@ -30,279 +32,298 @@
import org.apache.geode.redis.internal.commands.RedisCommandType;
public class GeodeRedisStats {
+
+ public static final String STATS_BASENAME = "GeodeForRedisStats";
+ private static final String GENERAL_CATEGORY = "General";
+
@Immutable
- private static final StatisticsType type;
+ private static final Map<String, StatisticsType> statisticTypes = new
HashMap<>();
@Immutable
private static final EnumMap<RedisCommandType, Integer>
completedCommandStatIds =
new EnumMap<>(RedisCommandType.class);
@Immutable
private static final EnumMap<RedisCommandType, Integer> timeCommandStatIds =
new EnumMap<>(RedisCommandType.class);
- private static final int currentlyConnectedClients;
+ private static final int currentlyConnectedClientsId;
private static final int passiveExpirationChecksId;
private static final int passiveExpirationCheckTimeId;
private static final int passiveExpirationsId;
private static final int expirationsId;
private static final int expirationTimeId;
- private static final int totalConnectionsReceived;
- private static final int commandsProcessed;
- private static final int keyspaceHits;
- private static final int keyspaceMisses;
- private static final int totalNetworkBytesRead;
+ private static final int totalConnectionsReceivedId;
+ private static final int commandsProcessedId;
+ private static final int keyspaceHitsId;
+ private static final int keyspaceMissesId;
+ private static final int totalNetworkBytesReadId;
private static final int publishRequestsCompletedId;
private static final int publishRequestsInProgressId;
private static final int publishRequestTimeId;
private static final int subscribersId;
private static final int uniqueChannelSubscriptionsId;
private static final int uniquePatternSubscriptionsId;
- private final Statistics stats;
+ private final Statistics generalStats;
+ private final Map<String, Statistics> statistics = new HashMap<>();
private final StatisticsClock clock;
- public GeodeRedisStats(StatisticsFactory factory, String name,
StatisticsClock clock) {
+ public GeodeRedisStats(StatisticsFactory factory, StatisticsClock clock) {
this.clock = clock;
- stats = factory == null ? null : factory.createAtomicStatistics(type,
name);
+ generalStats =
+ factory.createAtomicStatistics(statisticTypes.get(GENERAL_CATEGORY),
STATS_BASENAME);
+ statistics.put(GENERAL_CATEGORY, generalStats);
+
+ for (RedisCommandType.Category category :
RedisCommandType.Category.values()) {
+ String statName = STATS_BASENAME + ":" + category.name();
Review comment:
you don't need to recreate "statName" here. It was already added as the
name of the type. So when you call createAtomicStatistics just pass it the type
and the lower levels will use the type's name as the statistics name.
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/GeodeRedisStats.java
##########
@@ -30,279 +32,298 @@
import org.apache.geode.redis.internal.commands.RedisCommandType;
public class GeodeRedisStats {
+
+ public static final String STATS_BASENAME = "GeodeForRedisStats";
+ private static final String GENERAL_CATEGORY = "General";
+
@Immutable
- private static final StatisticsType type;
+ private static final Map<String, StatisticsType> statisticTypes = new
HashMap<>();
@Immutable
private static final EnumMap<RedisCommandType, Integer>
completedCommandStatIds =
new EnumMap<>(RedisCommandType.class);
@Immutable
private static final EnumMap<RedisCommandType, Integer> timeCommandStatIds =
new EnumMap<>(RedisCommandType.class);
- private static final int currentlyConnectedClients;
+ private static final int currentlyConnectedClientsId;
private static final int passiveExpirationChecksId;
private static final int passiveExpirationCheckTimeId;
private static final int passiveExpirationsId;
private static final int expirationsId;
private static final int expirationTimeId;
- private static final int totalConnectionsReceived;
- private static final int commandsProcessed;
- private static final int keyspaceHits;
- private static final int keyspaceMisses;
- private static final int totalNetworkBytesRead;
+ private static final int totalConnectionsReceivedId;
+ private static final int commandsProcessedId;
+ private static final int keyspaceHitsId;
+ private static final int keyspaceMissesId;
+ private static final int totalNetworkBytesReadId;
private static final int publishRequestsCompletedId;
private static final int publishRequestsInProgressId;
private static final int publishRequestTimeId;
private static final int subscribersId;
private static final int uniqueChannelSubscriptionsId;
private static final int uniquePatternSubscriptionsId;
- private final Statistics stats;
+ private final Statistics generalStats;
+ private final Map<String, Statistics> statistics = new HashMap<>();
private final StatisticsClock clock;
- public GeodeRedisStats(StatisticsFactory factory, String name,
StatisticsClock clock) {
+ public GeodeRedisStats(StatisticsFactory factory, StatisticsClock clock) {
this.clock = clock;
- stats = factory == null ? null : factory.createAtomicStatistics(type,
name);
+ generalStats =
+ factory.createAtomicStatistics(statisticTypes.get(GENERAL_CATEGORY),
STATS_BASENAME);
+ statistics.put(GENERAL_CATEGORY, generalStats);
+
+ for (RedisCommandType.Category category :
RedisCommandType.Category.values()) {
+ String statName = STATS_BASENAME + ":" + category.name();
+ Statistics stats =
+ factory.createAtomicStatistics(statisticTypes.get(category.name()),
statName);
+ statistics.put(category.name(), stats);
+ }
}
static {
StatisticsTypeFactory statisticsTypeFactory =
StatisticsTypeFactoryImpl.singleton();
- ArrayList<StatisticDescriptor> descriptorList = new ArrayList<>();
-
- fillListWithCompletedCommandDescriptors(statisticsTypeFactory,
descriptorList);
- fillListWithTimeCommandDescriptors(statisticsTypeFactory, descriptorList);
- fillListWithCommandDescriptors(statisticsTypeFactory, descriptorList);
-
- StatisticDescriptor[] descriptorArray =
- descriptorList.toArray(new StatisticDescriptor[0]);
- type = statisticsTypeFactory
- .createType("GeodeForRedisStats",
+ StatisticsType generalType = statisticsTypeFactory
+ .createType(STATS_BASENAME,
"Statistics for a geode-for-redis server",
- descriptorArray);
-
- fillCompletedIdMap();
- fillTimeIdMap();
-
- currentlyConnectedClients = type.nameToId("connectedClients");
- passiveExpirationChecksId = type.nameToId("passiveExpirationChecks");
- passiveExpirationCheckTimeId = type.nameToId("passiveExpirationCheckTime");
- passiveExpirationsId = type.nameToId("passiveExpirations");
- expirationsId = type.nameToId("expirations");
- expirationTimeId = type.nameToId("expirationTime");
- totalConnectionsReceived = type.nameToId("totalConnectionsReceived");
- commandsProcessed = type.nameToId("commandsProcessed");
- totalNetworkBytesRead = type.nameToId("totalNetworkBytesRead");
- keyspaceHits = type.nameToId("keyspaceHits");
- keyspaceMisses = type.nameToId("keyspaceMisses");
- publishRequestsCompletedId = type.nameToId("publishRequestsCompleted");
- publishRequestsInProgressId = type.nameToId("publishRequestsInProgress");
- publishRequestTimeId = type.nameToId("publishRequestTime");
- subscribersId = type.nameToId("subscribers");
- uniqueChannelSubscriptionsId = type.nameToId("uniqueChannelSubscriptions");
- uniquePatternSubscriptionsId = type.nameToId("uniquePatternSubscriptions");
+ createGeneralStatisticDescriptors(statisticsTypeFactory));
+
+ currentlyConnectedClientsId = generalType.nameToId("connectedClients");
+ passiveExpirationChecksId =
generalType.nameToId("passiveExpirationChecks");
+ passiveExpirationCheckTimeId =
generalType.nameToId("passiveExpirationCheckTime");
+ passiveExpirationsId = generalType.nameToId("passiveExpirations");
+ expirationsId = generalType.nameToId("expirations");
+ expirationTimeId = generalType.nameToId("expirationTime");
+ totalConnectionsReceivedId =
generalType.nameToId("totalConnectionsReceived");
+ commandsProcessedId = generalType.nameToId("commandsProcessed");
+ totalNetworkBytesReadId = generalType.nameToId("totalNetworkBytesRead");
+ keyspaceHitsId = generalType.nameToId("keyspaceHits");
+ keyspaceMissesId = generalType.nameToId("keyspaceMisses");
+ publishRequestsCompletedId =
generalType.nameToId("publishRequestsCompleted");
+ publishRequestsInProgressId =
generalType.nameToId("publishRequestsInProgress");
+ publishRequestTimeId = generalType.nameToId("publishRequestTime");
+ subscribersId = generalType.nameToId("subscribers");
+ uniqueChannelSubscriptionsId =
generalType.nameToId("uniqueChannelSubscriptions");
+ uniquePatternSubscriptionsId =
generalType.nameToId("uniquePatternSubscriptions");
+
+ statisticTypes.put(GENERAL_CATEGORY, generalType);
+
+ for (RedisCommandType.Category category :
RedisCommandType.Category.values()) {
+ StatisticsType type = statisticsTypeFactory
+ .createType(STATS_BASENAME + ":" + category.name(),
+ category.name() + " statistics for a geode-for-redis server",
+ createCategoryStatisticDescriptors(statisticsTypeFactory,
category));
+ statisticTypes.put(category.name(), type);
+
+ fillCompletedIdMap(category);
+ fillTimeIdMap(category);
+ }
}
private long getCurrentTimeNanos() {
return clock.getTime();
}
public void endCommand(RedisCommandType command, long start) {
+ Statistics stat = statistics.get(command.category().name());
if (clock.isEnabled()) {
- stats.incLong(timeCommandStatIds.get(command), getCurrentTimeNanos() -
start);
+ stat.incLong(timeCommandStatIds.get(command), getCurrentTimeNanos() -
start);
}
- stats.incLong(completedCommandStatIds.get(command), 1);
+ stat.incLong(completedCommandStatIds.get(command), 1);
}
public void addClient() {
- stats.incLong(currentlyConnectedClients, 1);
- stats.incLong(totalConnectionsReceived, 1);
+ generalStats.incLong(currentlyConnectedClientsId, 1);
+ generalStats.incLong(totalConnectionsReceivedId, 1);
}
public void removeClient() {
- stats.incLong(currentlyConnectedClients, -1);
+ generalStats.incLong(currentlyConnectedClientsId, -1);
}
public void endPassiveExpirationCheck(long start, long expireCount) {
if (expireCount > 0) {
incPassiveExpirations(expireCount);
}
if (clock.isEnabled()) {
- stats.incLong(passiveExpirationCheckTimeId, getCurrentTimeNanos() -
start);
+ generalStats.incLong(passiveExpirationCheckTimeId, getCurrentTimeNanos()
- start);
}
- stats.incLong(passiveExpirationChecksId, 1);
+ generalStats.incLong(passiveExpirationChecksId, 1);
}
private void incPassiveExpirations(long count) {
- stats.incLong(passiveExpirationsId, count);
+ generalStats.incLong(passiveExpirationsId, count);
}
public void endExpiration(long start) {
if (clock.isEnabled()) {
- stats.incLong(expirationTimeId, getCurrentTimeNanos() - start);
+ generalStats.incLong(expirationTimeId, getCurrentTimeNanos() - start);
}
- stats.incLong(expirationsId, 1);
+ generalStats.incLong(expirationsId, 1);
}
public void incrementCommandsProcessed() {
- stats.incLong(commandsProcessed, 1);
+ generalStats.incLong(commandsProcessedId, 1);
}
public void incrementTotalNetworkBytesRead(long bytes) {
- stats.incLong(totalNetworkBytesRead, bytes);
+ generalStats.incLong(totalNetworkBytesReadId, bytes);
}
public void incrementKeyspaceHits() {
- stats.incLong(keyspaceHits, 1);
+ generalStats.incLong(keyspaceHitsId, 1);
}
public void incrementKeyspaceMisses() {
- stats.incLong(keyspaceMisses, 1);
+ generalStats.incLong(keyspaceMissesId, 1);
}
public long startPublish() {
- stats.incLong(publishRequestsInProgressId, 1);
+ generalStats.incLong(publishRequestsInProgressId, 1);
return getCurrentTimeNanos();
}
public void endPublish(long publishCount, long time) {
- stats.incLong(publishRequestsInProgressId, -publishCount);
- stats.incLong(publishRequestsCompletedId, publishCount);
+ generalStats.incLong(publishRequestsInProgressId, -publishCount);
+ generalStats.incLong(publishRequestsCompletedId, publishCount);
if (clock.isEnabled()) {
- stats.incLong(publishRequestTimeId, time);
+ generalStats.incLong(publishRequestTimeId, time);
}
}
public void changeSubscribers(long delta) {
- stats.incLong(subscribersId, delta);
+ generalStats.incLong(subscribersId, delta);
}
public void changeUniqueChannelSubscriptions(long delta) {
- stats.incLong(uniqueChannelSubscriptionsId, delta);
+ generalStats.incLong(uniqueChannelSubscriptionsId, delta);
}
public void changeUniquePatternSubscriptions(long delta) {
- stats.incLong(uniquePatternSubscriptionsId, delta);
+ generalStats.incLong(uniquePatternSubscriptionsId, delta);
}
public void close() {
- if (stats != null) {
- stats.close();
+ if (generalStats != null) {
Review comment:
this null check is not needed
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/GeodeRedisStats.java
##########
@@ -30,170 +32,197 @@
import org.apache.geode.redis.internal.commands.RedisCommandType;
public class GeodeRedisStats {
+
+ public static final String STATS_BASENAME = "GeodeForRedisStats";
+ private static final String GENERAL_CATEGORY = "General";
+
@Immutable
- private static final StatisticsType type;
+ private static final Map<String, StatisticsType> statisticTypes = new
HashMap<>();
@Immutable
private static final EnumMap<RedisCommandType, Integer>
completedCommandStatIds =
new EnumMap<>(RedisCommandType.class);
@Immutable
private static final EnumMap<RedisCommandType, Integer> timeCommandStatIds =
new EnumMap<>(RedisCommandType.class);
- private static final int currentlyConnectedClients;
+ private static final int currentlyConnectedClientsId;
private static final int passiveExpirationChecksId;
private static final int passiveExpirationCheckTimeId;
private static final int passiveExpirationsId;
private static final int expirationsId;
private static final int expirationTimeId;
- private static final int totalConnectionsReceived;
- private static final int commandsProcessed;
- private static final int keyspaceHits;
- private static final int keyspaceMisses;
- private static final int totalNetworkBytesRead;
+ private static final int totalConnectionsReceivedId;
+ private static final int commandsProcessedId;
+ private static final int keyspaceHitsId;
+ private static final int keyspaceMissesId;
+ private static final int totalNetworkBytesReadId;
private static final int publishRequestsCompletedId;
private static final int publishRequestsInProgressId;
private static final int publishRequestTimeId;
private static final int subscribersId;
private static final int uniqueChannelSubscriptionsId;
private static final int uniquePatternSubscriptionsId;
- private final Statistics stats;
+ private final Statistics generalStats;
+ private final Map<String, Statistics> statistics = new HashMap<>();
private final StatisticsClock clock;
- public GeodeRedisStats(StatisticsFactory factory, String name,
StatisticsClock clock) {
+ public GeodeRedisStats(StatisticsFactory factory, StatisticsClock clock) {
this.clock = clock;
- stats = factory == null ? null : factory.createAtomicStatistics(type,
name);
+ generalStats = factory == null ? null
+ : factory.createAtomicStatistics(statisticTypes.get(GENERAL_CATEGORY),
STATS_BASENAME);
+ statistics.put(GENERAL_CATEGORY, generalStats);
+
+ for (RedisCommandType.Category category :
RedisCommandType.Category.values()) {
+ String statName = STATS_BASENAME + ":" + category.name();
+ Statistics stats = factory == null ? null
+ :
factory.createAtomicStatistics(statisticTypes.get(category.name()), statName);
+ statistics.put(category.name(), stats);
+ }
}
static {
StatisticsTypeFactory statisticsTypeFactory =
StatisticsTypeFactoryImpl.singleton();
ArrayList<StatisticDescriptor> descriptorList = new ArrayList<>();
- fillListWithCompletedCommandDescriptors(statisticsTypeFactory,
descriptorList);
- fillListWithTimeCommandDescriptors(statisticsTypeFactory, descriptorList);
fillListWithCommandDescriptors(statisticsTypeFactory, descriptorList);
+ StatisticDescriptor[] descriptorArray = descriptorList.toArray(new
StatisticDescriptor[0]);
- StatisticDescriptor[] descriptorArray =
- descriptorList.toArray(new StatisticDescriptor[0]);
-
- type = statisticsTypeFactory
- .createType("GeodeForRedisStats",
+ StatisticsType generalType = statisticsTypeFactory
+ .createType(STATS_BASENAME,
"Statistics for a geode-for-redis server",
descriptorArray);
- fillCompletedIdMap();
- fillTimeIdMap();
-
- currentlyConnectedClients = type.nameToId("connectedClients");
- passiveExpirationChecksId = type.nameToId("passiveExpirationChecks");
- passiveExpirationCheckTimeId = type.nameToId("passiveExpirationCheckTime");
- passiveExpirationsId = type.nameToId("passiveExpirations");
- expirationsId = type.nameToId("expirations");
- expirationTimeId = type.nameToId("expirationTime");
- totalConnectionsReceived = type.nameToId("totalConnectionsReceived");
- commandsProcessed = type.nameToId("commandsProcessed");
- totalNetworkBytesRead = type.nameToId("totalNetworkBytesRead");
- keyspaceHits = type.nameToId("keyspaceHits");
- keyspaceMisses = type.nameToId("keyspaceMisses");
- publishRequestsCompletedId = type.nameToId("publishRequestsCompleted");
- publishRequestsInProgressId = type.nameToId("publishRequestsInProgress");
- publishRequestTimeId = type.nameToId("publishRequestTime");
- subscribersId = type.nameToId("subscribers");
- uniqueChannelSubscriptionsId = type.nameToId("uniqueChannelSubscriptions");
- uniquePatternSubscriptionsId = type.nameToId("uniquePatternSubscriptions");
+ currentlyConnectedClientsId = generalType.nameToId("connectedClients");
+ passiveExpirationChecksId =
generalType.nameToId("passiveExpirationChecks");
+ passiveExpirationCheckTimeId =
generalType.nameToId("passiveExpirationCheckTime");
+ passiveExpirationsId = generalType.nameToId("passiveExpirations");
+ expirationsId = generalType.nameToId("expirations");
+ expirationTimeId = generalType.nameToId("expirationTime");
+ totalConnectionsReceivedId =
generalType.nameToId("totalConnectionsReceived");
+ commandsProcessedId = generalType.nameToId("commandsProcessed");
+ totalNetworkBytesReadId = generalType.nameToId("totalNetworkBytesRead");
+ keyspaceHitsId = generalType.nameToId("keyspaceHits");
+ keyspaceMissesId = generalType.nameToId("keyspaceMisses");
+ publishRequestsCompletedId =
generalType.nameToId("publishRequestsCompleted");
+ publishRequestsInProgressId =
generalType.nameToId("publishRequestsInProgress");
+ publishRequestTimeId = generalType.nameToId("publishRequestTime");
+ subscribersId = generalType.nameToId("subscribers");
+ uniqueChannelSubscriptionsId =
generalType.nameToId("uniqueChannelSubscriptions");
+ uniquePatternSubscriptionsId =
generalType.nameToId("uniquePatternSubscriptions");
+
+ statisticTypes.put(GENERAL_CATEGORY, generalType);
+
+ for (RedisCommandType.Category category :
RedisCommandType.Category.values()) {
+ ArrayList<StatisticDescriptor> descriptors = new ArrayList<>();
+ fillListWithCompletedCommandDescriptors(statisticsTypeFactory, category,
descriptors);
+ fillListWithTimeCommandDescriptors(statisticsTypeFactory, category,
descriptors);
+
+ StatisticDescriptor[] descriptorsArray = descriptors.toArray(new
StatisticDescriptor[0]);
+ StatisticsType type = statisticsTypeFactory
+ .createType(STATS_BASENAME + ":" + category.name(),
+ category.name() + " statistics for a geode-for-redis server",
+ descriptorsArray);
+ statisticTypes.put(category.name(), type);
+
+ fillCompletedIdMap(category);
Review comment:
type is associated with category. So instead of just passing category in
to fillCompletedIdMap and fillTimeIdMap also pass in type. Currently those fill
methods turn around and lookup the type inside the for loop
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/GeodeRedisStats.java
##########
@@ -30,279 +32,298 @@
import org.apache.geode.redis.internal.commands.RedisCommandType;
public class GeodeRedisStats {
+
+ public static final String STATS_BASENAME = "GeodeForRedisStats";
+ private static final String GENERAL_CATEGORY = "General";
+
@Immutable
- private static final StatisticsType type;
+ private static final Map<String, StatisticsType> statisticTypes = new
HashMap<>();
@Immutable
private static final EnumMap<RedisCommandType, Integer>
completedCommandStatIds =
new EnumMap<>(RedisCommandType.class);
@Immutable
private static final EnumMap<RedisCommandType, Integer> timeCommandStatIds =
new EnumMap<>(RedisCommandType.class);
- private static final int currentlyConnectedClients;
+ private static final int currentlyConnectedClientsId;
private static final int passiveExpirationChecksId;
private static final int passiveExpirationCheckTimeId;
private static final int passiveExpirationsId;
private static final int expirationsId;
private static final int expirationTimeId;
- private static final int totalConnectionsReceived;
- private static final int commandsProcessed;
- private static final int keyspaceHits;
- private static final int keyspaceMisses;
- private static final int totalNetworkBytesRead;
+ private static final int totalConnectionsReceivedId;
+ private static final int commandsProcessedId;
+ private static final int keyspaceHitsId;
+ private static final int keyspaceMissesId;
+ private static final int totalNetworkBytesReadId;
private static final int publishRequestsCompletedId;
private static final int publishRequestsInProgressId;
private static final int publishRequestTimeId;
private static final int subscribersId;
private static final int uniqueChannelSubscriptionsId;
private static final int uniquePatternSubscriptionsId;
- private final Statistics stats;
+ private final Statistics generalStats;
+ private final Map<String, Statistics> statistics = new HashMap<>();
private final StatisticsClock clock;
- public GeodeRedisStats(StatisticsFactory factory, String name,
StatisticsClock clock) {
+ public GeodeRedisStats(StatisticsFactory factory, StatisticsClock clock) {
this.clock = clock;
- stats = factory == null ? null : factory.createAtomicStatistics(type,
name);
+ generalStats =
+ factory.createAtomicStatistics(statisticTypes.get(GENERAL_CATEGORY),
STATS_BASENAME);
+ statistics.put(GENERAL_CATEGORY, generalStats);
Review comment:
given that most of the code directly references "generateStats" why do
we need to also put it in the "statistics" map? You could do the same with the
type "generalType" and then the map could be EnumMap
--
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]
