adelapena commented on code in PR #1891:
URL: https://github.com/apache/cassandra/pull/1891#discussion_r1171475059
##########
src/java/org/apache/cassandra/io/sstable/metadata/StatsMetadata.java:
##########
@@ -307,8 +310,14 @@ public int serializedSize(Version version, StatsMetadata
component) throws IOExc
size +=
EstimatedHistogram.serializer.serializedSize(component.estimatedPartitionSize);
size +=
EstimatedHistogram.serializer.serializedSize(component.estimatedCellPerPartitionCount);
size +=
CommitLogPosition.serializer.serializedSize(component.commitLogIntervals.upperBound().orElse(CommitLogPosition.NONE));
- size += 8 + 8 + 4 + 4 + 4 + 4 + 8 + 8; // mix/max timestamp(long),
min/maxLocalDeletionTime(int), min/max TTL, compressionRatio(double),
repairedAt (long)
- size +=
TombstoneHistogram.serializer.serializedSize(component.estimatedTombstoneDropTime);
+ size += 8 + 8; // mix/max timestamp(long)
+ size += 4 + 4; //min/maxLocalDeletionTime(either int or uint)
+ size += 4 + 4 + 8 + 8;// min/max TTL, compressionRatio(double),
repairedAt (long)
+ if (version.hasUIntDeletionTime())
+ size +=
TombstoneHistogram.serializer.serializedSize(component.estimatedTombstoneDropTime);
+ else
+ size +=
TombstoneHistogram.legacySerializer.serializedSize(component.estimatedTombstoneDropTime);
Review Comment:
There are three similar if-else block on this class. They select the proper
serializer according to the sstable version and then both if-else branches
perform the same operation for the selected serializer.
I think we could get rid of the repeated if-else blocks if we had a single
reusable method to get the serializer for the current sstable version. Also,
the serialization tests on `StreamingTombstoneHistogramBuilderTest` should test
both serializers. For example, [this
way](https://github.com/adelapena/cassandra/commit/13a7a802ebc31142a5221d3f87e39d23b1d651b3).
--
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]
