bdeggleston commented on code in PR #1948:
URL: https://github.com/apache/cassandra/pull/1948#discussion_r1006286711
##########
src/java/org/apache/cassandra/service/accord/serializers/AcceptSerializers.java:
##########
@@ -104,22 +104,29 @@ public long serializedSize(Accept.Invalidate invalidate,
int version)
public void serialize(AcceptOk acceptOk, DataOutputPlus out, int
version) throws IOException
{
CommandSerializers.txnId.serialize(acceptOk.txnId, out, version);
- CommandSerializers.deps.serialize(acceptOk.deps, out, version);
+ boolean hasDeps = acceptOk.deps != null;
Review Comment:
can we use NullableSerializer here?
##########
src/java/org/apache/cassandra/service/accord/AccordKeyspace.java:
##########
@@ -115,12 +115,12 @@
private static final Logger logger =
LoggerFactory.getLogger(AccordKeyspace.class);
public static final String COMMANDS = "commands";
- public static final String COMMAND_SERIES = "command_series";
public static final String COMMANDS_FOR_KEY = "commands_for_key";
private static final String TIMESTAMP_TUPLE = "tuple<bigint, bigint, int,
bigint>";
private static final TupleType TIMESTAMP_TYPE = new
TupleType(Lists.newArrayList(LongType.instance, LongType.instance,
Int32Type.instance, LongType.instance));
private static final String KEY_TUPLE = "tuple<uuid, blob>";
+ private static final int CURRENT_VERSION = 1;
Review Comment:
this should be `MessagingService.current_version`
##########
src/java/org/apache/cassandra/service/accord/async/AsyncWriter.java:
##########
@@ -238,7 +240,7 @@ private void denormalize(AccordCommand command,
AsyncContext context, Object cal
}
// There won't be a txn to denormalize against until the command has
been preaccepted
- if (command.status().hasBeen(Status.PreAccepted) &&
AccordPartialCommand.WithDeps.serializer.needsUpdate(command))
+ if (command.status().hasBeen(Status.PreAccepted) &&
AccordPartialCommand.WithDeps.serializer.needsUpdate(command) &&
!(command.txn() == null && command.status().isInvalidated()))
Review Comment:
Not denormalizing an invalidated command may leave it in the uncommitted
set, which could lead to incorrect deps being reported. That said, cfk
denormalization may no longer be needed, and I'm going to try removing it in
another branch, so hang tight
##########
src/java/org/apache/cassandra/service/accord/AccordKeyspace.java:
##########
@@ -433,31 +424,27 @@ public static Mutation getCommandMutation(AccordCommand
command, long timestampM
Row.Builder builder = BTreeRow.unsortedBuilder();
builder.newRow(Clustering.EMPTY);
int nowInSeconds = (int)
TimeUnit.MICROSECONDS.toSeconds(timestampMicros);
- int version = MessagingService.current_version;
- ByteBuffer versionBytes = accessor.valueOf(version);
+
if (command.status.hasModifications())
builder.addCell(live(CommandsColumns.status, timestampMicros,
accessor.valueOf(command.status.get().ordinal())));
if (command.homeKey.hasModifications())
{
Review Comment:
we can drop the braces for the blocks that have become single lines
--
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]
