jacek-lewandowski commented on code in PR #2464:
URL: https://github.com/apache/cassandra/pull/2464#discussion_r1263401561
##########
src/java/org/apache/cassandra/db/DeletionTime.java:
##########
@@ -195,42 +195,100 @@ public static Serializer getSerializer(Version version)
return legacySerializer;
}
- // Serializer for Usigned Integer ldt
+ /* Serializer for Usigned Integer ldt
+ *
+ * ldt is encoded as a uint in seconds since unix epoch, it can go up o
2106-02-07T06:28:13+00:00 only.
+ * Since mfda is micros since the Unix Epoch with 7 bytes we can encode
2^56 ~= 1142 years. That leaves
+ * 1 free byte we can use to store flags for sentinel values with some
space saving.
+ *
+ * Currently only a flag for the LIVE ldt is being used.
+ */
public static class Serializer implements ISerializer<DeletionTime>
{
+ private final static int IS_LIVE_DELETION = 0x01;
+ // Long.MAX_VALUE sentinel value equivalent in 7bytes
+ public final static long MAX_MFDA = (1L << (7 * 8)) - 1;
+
public void serialize(DeletionTime delTime, DataOutputPlus out) throws
IOException
{
- out.writeInt(delTime.localDeletionTimeUnsignedInteger);
- out.writeLong(delTime.markedForDeleteAt());
+ if (delTime.equals(LIVE))
+ out.writeByte(IS_LIVE_DELETION);
+ else
+ {
+ if (delTime.markedForDeleteAt() > MAX_MFDA &&
delTime.markedForDeleteAt() != Long.MAX_VALUE)
+ throw new IOException("Value too high for markForDeleteAt
to encode in 7 bytes: " + delTime.markedForDeleteAt());
+ // The flags byte is all zeros atm here, so we can write a
long directly
+ out.writeLong(delTime.markedForDeleteAt() == Long.MAX_VALUE ?
MAX_MFDA : delTime.markedForDeleteAt());
Review Comment:
why don't you store this information as a separate flag? say, if flag 0x2 is
set, don't have to store mfda at all
##########
src/java/org/apache/cassandra/db/DeletionTime.java:
##########
@@ -195,42 +195,100 @@ public static Serializer getSerializer(Version version)
return legacySerializer;
}
- // Serializer for Usigned Integer ldt
+ /* Serializer for Usigned Integer ldt
+ *
+ * ldt is encoded as a uint in seconds since unix epoch, it can go up o
2106-02-07T06:28:13+00:00 only.
+ * Since mfda is micros since the Unix Epoch with 7 bytes we can encode
2^56 ~= 1142 years. That leaves
+ * 1 free byte we can use to store flags for sentinel values with some
space saving.
+ *
+ * Currently only a flag for the LIVE ldt is being used.
+ */
public static class Serializer implements ISerializer<DeletionTime>
{
+ private final static int IS_LIVE_DELETION = 0x01;
+ // Long.MAX_VALUE sentinel value equivalent in 7bytes
+ public final static long MAX_MFDA = 0xffffffffffffffL;
+
public void serialize(DeletionTime delTime, DataOutputPlus out) throws
IOException
{
- out.writeInt(delTime.localDeletionTimeUnsignedInteger);
- out.writeLong(delTime.markedForDeleteAt());
+ if (delTime.equals(LIVE))
+ out.writeByte(IS_LIVE_DELETION);
+ else
+ {
+ if (delTime.markedForDeleteAt() > MAX_MFDA &&
delTime.markedForDeleteAt() != Long.MAX_VALUE)
+ throw new IOException("Value too high for markForDeleteAt
to encode in 7 bytes: " + delTime.markedForDeleteAt());
Review Comment:
I'm wondering if it should be verified earlier - that is, when the user
provides a mutation timestamp. There is also a guardrail for that:
`maximum_timestamp_fail_threshold` - maybe it deserves a default value?
##########
src/java/org/apache/cassandra/db/DeletionTime.java:
##########
@@ -195,42 +195,100 @@ public static Serializer getSerializer(Version version)
return legacySerializer;
}
- // Serializer for Usigned Integer ldt
+ /* Serializer for Usigned Integer ldt
+ *
+ * ldt is encoded as a uint in seconds since unix epoch, it can go up o
2106-02-07T06:28:13+00:00 only.
+ * Since mfda is micros since the Unix Epoch with 7 bytes we can encode
2^56 ~= 1142 years. That leaves
+ * 1 free byte we can use to store flags for sentinel values with some
space saving.
+ *
+ * Currently only a flag for the LIVE ldt is being used.
+ */
public static class Serializer implements ISerializer<DeletionTime>
{
+ private final static int IS_LIVE_DELETION = 0x01;
+ // Long.MAX_VALUE sentinel value equivalent in 7bytes
+ public final static long MAX_MFDA = 0xffffffffffffffL;
+
public void serialize(DeletionTime delTime, DataOutputPlus out) throws
IOException
{
- out.writeInt(delTime.localDeletionTimeUnsignedInteger);
- out.writeLong(delTime.markedForDeleteAt());
+ if (delTime.equals(LIVE))
+ out.writeByte(IS_LIVE_DELETION);
+ else
+ {
+ if (delTime.markedForDeleteAt() > MAX_MFDA &&
delTime.markedForDeleteAt() != Long.MAX_VALUE)
+ throw new IOException("Value too high for markForDeleteAt
to encode in 7 bytes: " + delTime.markedForDeleteAt());
+ // The flags byte is all zeros atm here, so we can write a
long directly
+ out.writeLong(delTime.markedForDeleteAt() == Long.MAX_VALUE ?
MAX_MFDA : delTime.markedForDeleteAt());
+ out.writeInt(delTime.localDeletionTimeUnsignedInteger);
+ }
}
public DeletionTime deserialize(DataInputPlus in) throws IOException
{
- int localDeletionTimeUnsignedInteger = in.readInt();
- long mfda = in.readLong();
- return mfda == Long.MIN_VALUE && localDeletionTimeUnsignedInteger
== Cell.NO_DELETION_TIME_UNSIGNED_INTEGER
- ? LIVE
- : new DeletionTime(mfda, localDeletionTimeUnsignedInteger);
+ int flags = in.readByte();
+ if ((flags & IS_LIVE_DELETION) != 0)
+ return LIVE;
+ else
+ {
+ // Read the 7 bytes
+ int bytes4 = in.readInt();
+ int bytes2 = in.readShort();
+ int bytes1 = in.readByte();
+
+ long mfda = sevenBytesToMFDA(bytes4, bytes2, bytes1);
+ int localDeletionTimeUnsignedInteger = in.readInt();
+
+ return new DeletionTime(mfda,
localDeletionTimeUnsignedInteger);
+ }
}
public DeletionTime deserialize(ByteBuffer buf, int offset)
{
- int localDeletionTimeUnsignedInteger = buf.getInt(offset);
- long mfda = buf.getLong(offset + 4);
- return mfda == Long.MIN_VALUE && localDeletionTimeUnsignedInteger
== Cell.NO_DELETION_TIME_UNSIGNED_INTEGER
- ? LIVE
- : new DeletionTime(mfda, localDeletionTimeUnsignedInteger);
+ int flags = buf.get(offset);
+ if ((flags & IS_LIVE_DELETION) != 0)
+ return LIVE;
+ else
+ {
+ // Read the 7 bytes
+ int bytes4 = buf.getInt(offset + 1);
+ int bytes2 = buf.getShort(offset + 5);
+ int bytes1 = buf.get(offset + 7);
+
+ long mfda = sevenBytesToMFDA(bytes4, bytes2, bytes1);
+ int localDeletionTimeUnsignedInteger = buf.getInt(offset + 8);
+
+ return new DeletionTime(mfda,
localDeletionTimeUnsignedInteger);
+ }
}
public void skip(DataInputPlus in) throws IOException
{
- in.skipBytesFully(4 + 8);
+ int flags = in.readByte();
+ if ((flags & IS_LIVE_DELETION) != 0)
+ // We read the flags already and there's nothing left to skip
over
+ return;
+ else
+ // We read the flags already but there is mfda and ldt to skip
over still
+ in.skipBytesFully(TypeSizes.LONG_SIZE - 1 +
TypeSizes.INT_SIZE);
}
public long serializedSize(DeletionTime delTime)
{
- return TypeSizes.sizeof(Integer.MAX_VALUE)
- + TypeSizes.sizeof(delTime.markedForDeleteAt());
+ if (delTime.equals(LIVE))
+ // Just the flags
+ return 1;
+ else
+ // 1 for the flags, 7 for mfda and 4 for ldt
+ return TypeSizes.INT_SIZE
+ + TypeSizes.LONG_SIZE;
+ }
+
+ private long sevenBytesToMFDA(int bytes4, int bytes2, int bytes1)
+ {
+ long mfda = bytes4 & 0xFFFFFFFFL;
Review Comment:
nit: either 0xff.. or 0xFF..
--
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]
