Copilot commented on code in PR #19128:
URL: https://github.com/apache/pinot/pull/19128#discussion_r3681634337
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/H3IndexHandler.java:
##########
@@ -212,12 +213,20 @@ private void
handleDictionaryBasedColumn(SegmentDirectory.Writer segmentWriter,
.createIndexReader(segmentWriter, colIndexConf, columnMetadata);
GeoSpatialIndexCreator h3IndexCreator =
StandardIndexes.h3().createIndexCreator(context, config)) {
int numDocs = columnMetadata.getTotalDocs();
+ // Decode each distinct dictionary value at most once. Old segments
reloaded after a geo column was added are
+ // dominated by a single empty default value (cardinality 1), so caching
by dictId avoids re-reading and
+ // re-decoding that same value for every doc. Decoding through the
creator's toGeometry() fast-paths the empty
+ // default value to null instead of failing the whole reload with a
BufferUnderflowException, tolerating it the
+ // same way the segment-creation path does.
Review Comment:
Caching decoded geometries in a `Geometry[]` sized to `dictionary.length()`
can retain one `Geometry` per dictId for the whole reload. For high-cardinality
geo columns (dict size ~= numDocs), this can create very large retained heaps
and risk OOM during segment reload, while providing little/no benefit.
Consider limiting caching to the common empty-default case (e.g.
`dictionary.length() == 1`), and otherwise fall back to the existing per-doc
`add(bytes, dictId)` path (which now uses `toGeometry()` so it still avoids the
empty-byte exception).
--
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]
