xiangfu0 commented on code in PR #18185:
URL: https://github.com/apache/pinot/pull/18185#discussion_r3384206301
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerSegmentMetadataReader.java:
##########
@@ -144,6 +160,62 @@ public TableMetadataInfo
getAggregatedTableMetadataFromServer(String tableNameWi
}
return l;
}));
+ // Aggregate per-column compression stats from server responses
+ List<ColumnCompressionStatsInfo> serverColStats =
tableMetadataInfo.getColumnCompressionStats();
+ if (serverColStats != null) {
+ for (ColumnCompressionStatsInfo info : serverColStats) {
+ // Skip columns with no codec — these are old raw segments built
before compression stats
+ // tracking was enabled and carry no meaningful data.
+ if (info.getCodec() == null) {
+ continue;
+ }
+ String col = info.getColumn();
+ long[] accum = columnCompressionAccum.computeIfAbsent(col, k ->
new long[2]);
+ // Only accumulate uncompressed size when it is a real value (not
the -1 sentinel from dict columns)
+ if (info.getRawIngestSizeInBytes() >= 0) {
+ accum[0] += info.getRawIngestSizeInBytes();
+ }
+ accum[1] += info.getOnDiskSizeInBytes();
+ if (info.getCodec() != null) {
+ columnCodecMap.merge(col, info.getCodec(),
+ (existing, incoming) -> existing.equals(incoming) ? existing
: "MIXED");
Review Comment:
this `MIXED` doesn't provide much info, can you make it a list of codecs?
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImpl.java:
##########
@@ -268,6 +275,22 @@ private static long unpackIndexSize(long typeAndSize) {
return typeAndSize & SIZE_MASK;
}
+ @Override
+ public long getUncompressedForwardIndexSizeBytes() {
+ return _uncompressedForwardIndexSizeBytes;
+ }
+
+ @Override
+ @Nullable
+ public String getCompressionCodec() {
Review Comment:
this should be ChunkCompressionType not string
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/writer/impl/FixedByteChunkForwardIndexWriter.java:
##########
@@ -54,24 +54,36 @@ public FixedByteChunkForwardIndexWriter(File file,
ChunkCompressionType compress
}
public void putInt(int value) {
+ if (_trackUncompressedSize) {
Review Comment:
this make the tracking to be on hotspot for every put call.
is better to infer it from the _chunkDataOffset?
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/writer/impl/VarByteChunkForwardIndexWriterV4.java:
##########
@@ -92,6 +92,8 @@ public class VarByteChunkForwardIndexWriterV4 implements
VarByteChunkWriter {
private int _nextDocId = 0;
private int _metadataSize = 0;
private long _chunkOffset = 0;
+ private long _uncompressedSize = 0;
+ private boolean _trackUncompressedSize = true;
Review Comment:
the default should be false and set by external
--
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]
