iamaleksey commented on code in PR #2073:
URL: https://github.com/apache/cassandra/pull/2073#discussion_r1068426939
##########
src/java/org/apache/cassandra/service/accord/AccordTopologyUtils.java:
##########
@@ -23,6 +23,7 @@
import java.util.HashSet;
Review Comment:
A bunch of unused imports (4)
##########
src/java/org/apache/cassandra/service/accord/AccordCommandStore.java:
##########
@@ -91,29 +342,25 @@ private static long getThreadId(ExecutorService executor)
private final Agent agent;
private final DataStore dataStore;
private final ProgressLog progressLog;
- private final RangesForEpoch rangesForEpoch;
+ private final RangesForEpochHolder rangesForEpochHolder;
public AccordCommandStore(int id,
- int generation,
- int index,
- int numShards,
NodeTimeService time,
Agent agent,
DataStore dataStore,
ProgressLog.Factory progressLogFactory,
- RangesForEpoch rangesForEpoch,
- ExecutorService executor)
+ RangesForEpochHolder rangesForEpoch)
{
- super(id, generation, index, numShards);
+ super(id);
this.time = time;
this.agent = agent;
this.dataStore = dataStore;
this.progressLog = progressLogFactory.create(this);
- this.rangesForEpoch = rangesForEpoch;
- this.loggingId = String.format("[%s:%s]", generation, index);
- this.executor = executor;
+ this.rangesForEpochHolder = rangesForEpoch;
+ this.loggingId = String.format("[%s]", id);
+ this.executor =
executorFactory().sequential(CommandStore.class.getSimpleName() + '[' + id +
']');;
Review Comment:
Redundant semicolon
##########
src/java/org/apache/cassandra/service/accord/AccordTopologyUtils.java:
##########
@@ -114,21 +112,15 @@ public static Topology createTopology(long epoch)
if
(SchemaConstants.REPLICATED_SYSTEM_KEYSPACE_NAMES.contains(ksname))
Review Comment:
Pre-existing, but `SchemaConstants#isSystemKeyspace()` is what you want
instead of two `contains()` checks.
##########
src/java/org/apache/cassandra/service/accord/api/AccordRoutableKey.java:
##########
@@ -22,30 +22,25 @@
import accord.primitives.RoutableKey;
import org.apache.cassandra.dht.Token;
-import org.apache.cassandra.schema.TableId;
+import org.apache.cassandra.service.accord.api.AccordRoutingKey.SentinelKey;
+import org.apache.cassandra.service.accord.api.AccordRoutingKey.TokenKey;
public abstract class AccordRoutableKey implements RoutableKey
{
- final TableId tableId;
+ final String keyspace; // TODO (desired): use an id (TrM)
- protected AccordRoutableKey(TableId tableId)
+ protected AccordRoutableKey(String keyspace)
{
- this.tableId = tableId;
+ this.keyspace = keyspace;
}
- public final TableId tableId() { return tableId; }
+ public final String keyspace() { return keyspace; }
public abstract Token token();
- @Override
- public final int routingHash()
- {
- return token().tokenHash();
- }
-
@Override
public int hashCode()
{
- return Objects.hash(tableId, routingHash());
+ return Objects.hash(keyspace, token().tokenHash());
}
public final int compareTo(RoutableKey that)
Review Comment:
`@Override`? (pre-existing)
##########
src/java/org/apache/cassandra/service/accord/AccordTopologyUtils.java:
##########
@@ -114,21 +112,15 @@ public static Topology createTopology(long epoch)
if
(SchemaConstants.REPLICATED_SYSTEM_KEYSPACE_NAMES.contains(ksname))
Review Comment:
Better yet, only iterate over `Schema.instance.distributedKeyspaces()`
(`.names()`) and filter out on `SchemaConstants#isReplicatedSystemKeyspace()`.
--
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]
