[jira] [Commented] (CASSANDRA-15725) Add support for adding custom Verbs
[ https://issues.apache.org/jira/browse/CASSANDRA-15725?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17116548#comment-17116548 ] Benedict Elliott Smith commented on CASSANDRA-15725: +1 > Add support for adding custom Verbs > --- > > Key: CASSANDRA-15725 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15725 > Project: Cassandra > Issue Type: Improvement > Components: Messaging/Internode >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Normal > Fix For: 4.0-alpha > > Attachments: feedback_15725.patch > > > It should be possible to safely add custom/internal Verbs - without risking > conflicts when new ones are added. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15725) Add support for adding custom Verbs
[ https://issues.apache.org/jira/browse/CASSANDRA-15725?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17115906#comment-17115906 ] Marcus Eriksson commented on CASSANDRA-15725: - All good ideas, pushed a commit to the branch above, could you have a look [~benedict]? > Add support for adding custom Verbs > --- > > Key: CASSANDRA-15725 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15725 > Project: Cassandra > Issue Type: Improvement > Components: Messaging/Internode >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Normal > Fix For: 4.0-alpha > > Attachments: feedback_15725.patch > > > It should be possible to safely add custom/internal Verbs - without risking > conflicts when new ones are added. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15725) Add support for adding custom Verbs
[
https://issues.apache.org/jira/browse/CASSANDRA-15725?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17113030#comment-17113030
]
Benedict Elliott Smith commented on CASSANDRA-15725:
Sorry for the slow response. I really like the approach taken, of permitting
authors of custom verbs to specify integers in the normal way, just with their
own distinct number space.
I have just some minor cosmetic suggestions, none of them important and happy
to be overruled:
1. Perhaps we want to specify a maximum custom id, so that we leave plenty of
room for Cassandra growing upwards without accidentally infringing on custom
spaces that have begun to be used by others?
2. I think it _might_ help to introduce a {{Kind}} enum with e.g. {{NORMAL,
CUSTOM}}, and for the custom constructor accept {{Kind}} as the first argument,
so you would see {{UNUSED_CUSTOM_VERB (CUSTOM, 0...}} which might help prevent
fat finger errors, and also demarcate more visibly the compiler-enforced divide
between the verbs
3. In {{fromId}} it might be ever so slightly cleaner to simply choose the
array to look inside, and update {{id}}, e.g.:
{code}
public static Verb fromId(int id)
{
Verb[] verbs = idToVerbMap;
if (id >= minCustomId)
{
id = idForCustomVerb(id);
verbs = idToCustomVerbMap;
}
Verb verb = id >= 0 && id < verbs.length ? verbs[id] : null;
if (verb == null)
throw new IllegalArgumentException("Unknown verb id " + id);
return verb;
}
{code}
Either way, LGTM +1
> Add support for adding custom Verbs
> ---
>
> Key: CASSANDRA-15725
> URL: https://issues.apache.org/jira/browse/CASSANDRA-15725
> Project: Cassandra
> Issue Type: Improvement
> Components: Messaging/Internode
>Reporter: Marcus Eriksson
>Assignee: Marcus Eriksson
>Priority: Normal
> Fix For: 4.0-alpha
>
> Attachments: feedback_15725.patch
>
>
> It should be possible to safely add custom/internal Verbs - without risking
> conflicts when new ones are added.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15725) Add support for adding custom Verbs
[ https://issues.apache.org/jira/browse/CASSANDRA-15725?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17104143#comment-17104143 ] Marcus Eriksson commented on CASSANDRA-15725: - good point regarding the id conflicts, pushed a simpler commit that just makes sure that `minCustomId > max` > Add support for adding custom Verbs > --- > > Key: CASSANDRA-15725 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15725 > Project: Cassandra > Issue Type: Improvement > Components: Messaging/Internode >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Normal > Fix For: 4.0-alpha > > Attachments: feedback_15725.patch > > > It should be possible to safely add custom/internal Verbs - without risking > conflicts when new ones are added. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15725) Add support for adding custom Verbs
[ https://issues.apache.org/jira/browse/CASSANDRA-15725?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17102798#comment-17102798 ] David Capwell commented on CASSANDRA-15725: --- Overall LGTM. I attached a patch to make sure we detect all potential id conflicts (left 3 commented out verbs for testing). I also tested out a 3.x custom verb upgrade. if custom verbs exist which don't map to the new ID format, and backwards compatibility is needed, the verb can define a pre40 verb with the old id, and the custom one can override org.apache.cassandra.net.Verb#toPre40Verb to point to it; this allows migration. > Add support for adding custom Verbs > --- > > Key: CASSANDRA-15725 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15725 > Project: Cassandra > Issue Type: Improvement > Components: Messaging/Internode >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Normal > Fix For: 4.0-alpha > > Attachments: feedback_15725.patch > > > It should be possible to safely add custom/internal Verbs - without risking > conflicts when new ones are added. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
