josh-mckenzie commented on a change in pull request #31:
URL:
https://github.com/apache/cassandra-in-jvm-dtest-api/pull/31#discussion_r797977881
##########
File path:
src/main/java/org/apache/cassandra/distributed/shared/AbstractBuilder.java
##########
@@ -64,6 +64,16 @@
private int datadirCount = 3;
private final List<Rack> racks = new ArrayList<>();
private boolean finalised;
+ private int tokenCount = getDefaultTokenCount();
+ private boolean allowVnodes = true;
Review comment:
Is the assumption here that most tests should Just Work with vnodes
enabled, so we allow it by default and if someone sets num_tokens they get that
run? Seems reasonable, just want to make sure I'm grokking the thinking.
##########
File path: src/main/java/org/apache/cassandra/distributed/api/TokenSupplier.java
##########
@@ -18,17 +18,41 @@
package org.apache.cassandra.distributed.api;
+import java.util.Arrays;
+import java.util.Collection;
+
public interface TokenSupplier
{
- long token(int nodeId);
+ Collection<String> tokens(int nodeId);
+
+ @Deprecated
+ default long token(int nodeId)
+ {
+ Collection<String> tokens = tokens(nodeId);
+ assert tokens.size() == 1: "tokens function returned multiple tokens,
only expected 1: " + tokens;
+ return Long.parseLong(tokens.stream().findFirst().get());
+ }
+ @Deprecated
static TokenSupplier evenlyDistributedTokens(int numNodes)
{
- long increment = (Long.MAX_VALUE / numNodes) * 2;
- return (int nodeId) -> {
- assert nodeId <= numNodes : String.format("Can not allocate a
token for a node %s, since only %s nodes are allowed by the token allocation
strategy",
- nodeId, numNodes);
- return Long.MIN_VALUE + 1 + nodeId * increment;
- };
+ return evenlyDistributedTokens(numNodes, 1);
+ }
+
+ static TokenSupplier evenlyDistributedTokens(int numNodes, int numTokens)
+ {
+ long increment = (Long.MAX_VALUE / (numNodes * numTokens)) * 2;
+ String[][] tokens = new String[numNodes][numTokens];
+
+ long value = Long.MIN_VALUE + 1;
+ for (int i = 0; i < numTokens; i++)
+ {
+ for (int nodeId = 1; nodeId <= numNodes; nodeId++)
+ {
+ value += nodeId + increment;
Review comment:
I think this should be nodeId * increment so it'll move by (nodeCount
(1, 2, 3, etc) * the size of the token range to assign to them). I believe this
as written will be assigning overlapping ranges.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
