GEODE-10032: Add Category to RedisCommandType#7354
GEODE-10032: Add Category to RedisCommandType#7354jdeppe-pivotal merged 4 commits intoapache:developfrom
Conversation
- This adds a single category (or type) to each command in order to identify the type of data structure it applies too. This is initially being done in order to allow Geode statistics to be finer grained in how the Radish-specific stats are grouped. - The category is derived from Redis' ACL Categories. In Redis these categories also include the command's flags (and possibly additional values). This implementation only specifies the category as it relates to the command's data type. For example, in Redis the `GET` command has categories `@read`, `@fast` and `@string`. In Geode, the category would only be `@string`. This may well change in the future if ACLs are implemented.
jdeppe-pivotal
requested review from
DonalEvans,
dschneider-pivotal,
nonbinaryprogrammer,
ringles and
upthewaterspout
as code owners
geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandType.java
Outdated
Show resolved
Hide resolved
geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandType.java
Outdated
Show resolved
Hide resolved
...in/java/org/apache/geode/redis/internal/commands/executor/server/CommandCommandExecutor.java
Outdated
Show resolved
Hide resolved
...in/java/org/apache/geode/redis/internal/commands/executor/server/CommandCommandExecutor.java
Outdated
Show resolved
Hide resolved
geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandType.java
Outdated
Show resolved
Hide resolved
geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandType.java
Outdated
Show resolved
Hide resolved
jdeppe-pivotal
changed the title
| SELECT(new SelectExecutor(), Category.KEYSPACE, UNSUPPORTED, | ||
| new Parameter().exact(2).firstKey(0).flags(LOADING, STALE, FAST)), |
There was a problem hiding this comment.
I think select still needs to be in the connection section. In the redis docs all the related commands are connection commands
There was a problem hiding this comment.
The category information is derived from the categories returned by the INFO command. So against Redis, INFO SELECT lists the categories as @keyspace and @fast. Using these values allows us to verify against a stricter source (any given redis version). The docs are a lot less specific and could easily re-categorize commands without us knowing.
There was a problem hiding this comment.
Or were you referring to just where in the file this entry appears?
There was a problem hiding this comment.
yes, I was just referring to its location. but if the info command has it in that section then that's fine. makes sense to use the info command to categorize our commands
onichols-pivotal
left a comment
onichols-pivotal
left a comment
There was a problem hiding this comment.
As per https://cwiki.apache.org/confluence/display/GEODE/Commit+Message+Format, first commit summary should be under 29 characters (not including bug# or pr#) (yours is 33 characters). If you amend your initial commit now and force-push, you can benefit from review feedback on your commit message, and you won't have to remember to fix it later when you merge.
- This adds a single category (or type) to each command in order to identify the type of data structure it applies too. This is initially being done in order to allow Geode statistics to be finer grained in how the Radish-specific stats are grouped. - The category is derived from Redis' ACL Categories. In Redis these categories also include the command's flags (and possibly additional values). This implementation only specifies the category as it relates to the command's data type. For example, in Redis the `GET` command has categories `@read`, `@fast` and `@string`. In Geode, the category would only be `@string`. This may well change in the future if ACLs are implemented.
5 participants
identify the type of data structure it applies too. This is initially
being done in order to allow Geode statistics to be finer grained in
how the Radish-specific stats are grouped.
categories also include the command's flags (and possibly additional
values). This implementation only specifies the category as it relates
to the command's data type. For example, in Redis the
GETcommandhas categories
@read,@fastand@string. In Geode, the categorywould only be
@string. This may well change in the future if ACLsare implemented.
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
Has your PR been rebased against the latest commit within the target branch (typically
develop)?Is your initial contribution a single, squashed commit?
Does
gradlew buildrun cleanly?Have you written or updated unit tests to verify your changes?
If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?