ifesdjeen commented on code in PR #3897:
URL: https://github.com/apache/cassandra/pull/3897#discussion_r1954282972
##########
src/java/org/apache/cassandra/service/accord/serializers/CommandSerializers.java:
##########
@@ -75,8 +75,7 @@ private CommandSerializers()
public static final IVersionedSerializer<TxnId> nullableTxnId =
NullableSerializer.wrap(txnId);
public static final TimestampSerializer<Timestamp> timestamp = new
TimestampSerializer<>(Timestamp::fromBits);
public static final IVersionedSerializer<Timestamp> nullableTimestamp =
NullableSerializer.wrap(timestamp);
- public static final TimestampSerializer<Ballot> ballot = new
TimestampSerializer<>(Ballot::fromBits);
- public static final IVersionedSerializer<Ballot> nullableBallot =
NullableSerializer.wrap(ballot);
+ public static final BallotSerializer ballot = new BallotSerializer(); //
permits null
Review Comment:
I am a bit confused why we have nullable ballot and ballot if this one
permits null?
##########
src/java/org/apache/cassandra/service/accord/serializers/AcceptSerializers.java:
##########
@@ -108,79 +109,47 @@ public long serializedSize(Accept.NotAccept invalidate,
int version)
@Override
public void serialize(AcceptReply reply, DataOutputPlus out, int
version) throws IOException
{
- switch (reply.outcome())
- {
- default: throw new AssertionError();
- case Retired:
- case Truncated:
- throw illegalState("AcceptReply with invalid
AcceptOutcome: " + reply.outcome);
- case Success:
- if (reply.deps != null)
- {
- out.writeByte(1);
- DepsSerializers.deps.serialize(reply.deps, out,
version);
- }
- else
- {
- Invariants.require(reply == AcceptReply.SUCCESS);
- out.writeByte(2);
- }
- break;
- case RejectedBallot:
- out.writeByte(3);
- CommandSerializers.ballot.serialize(reply.supersededBy,
out, version);
- break;
- case Redundant:
- int flags = 4 | (reply.supersededBy != null ? 0x8 : 0) |
(reply.committedExecuteAt != null ? 0x10 : 0);
- out.writeByte(flags);
- if (reply.supersededBy != null)
-
CommandSerializers.ballot.serialize(reply.supersededBy, out, version);
- if (reply.committedExecuteAt != null)
-
ExecuteAtSerializer.serialize(reply.committedExecuteAt, out);
- }
+ int flags = reply.outcome.ordinal()
+ | (reply.supersededBy != null ? 0x08 : 0)
Review Comment:
should we extract these magic numbers to constants to avoid slips in the
future? (that said i think new version is a strict improvement over previous)
--
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: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
