HassanAITBRIK opened a new issue, #20196: URL: https://github.com/apache/pinot/issues/20196
## Summary Since #17789, `upsertConfig.metadataTTL` is accepted on OFFLINE tables, but the primary keys are never expired at runtime. The TTL sweep and the validDocIds snapshot are only triggered when a REALTIME consuming segment commits, which never happens for an OFFLINE table. The heap used by the upsert metadata of an OFFLINE table therefore keeps growing with the number of primary keys ever loaded, until the server restarts. On master, the TTL has no effect at all on an OFFLINE table, even at restart. The docs ([Upsert > Metadata time-to-live](https://docs.pinot.apache.org/build-with-pinot/ingestion/upsert-and-dedup/upsert#metadata-time-to-live-ttl)) present the TTL as a general upsert feature, and the [offline upsert page](https://docs.pinot.apache.org/build-with-pinot/ingestion/upsert-and-dedup/offline-table-upsert) refers to them for TTL without mentioning this limitation. ## Where it happens (master @ 0007e8b9e4) - `removeExpiredPrimaryKeys()` and `takeSnapshot(...)` are only called from `RealtimeSegmentDataManager`, after a consuming segment is committed ([RealtimeSegmentDataManager.java#L911-L924](https://github.com/apache/pinot/blob/0007e8b9e4a144eb8f102ca1c52f0af3af848963/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java#L911-L924)). No OFFLINE code path calls either of them. - `removeSegment()` skips the metadata of a segment out of TTL, relying on that sweep ([BasePartitionUpsertMetadataManager.java#L847-L851](https://github.com/apache/pinot/blob/0007e8b9e4a144eb8f102ca1c52f0af3af848963/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java#L847-L851)): "The expired metadata is removed while creating new consuming segment in batches". On an OFFLINE table, deleting a segment out of TTL logs `Skip removing segment: ... because it's out of TTL` and its primary keys stay in the map until the server restarts. Without `metadataTTL`, deleting the same segment frees its primary keys immediately. - At load, a segment is only skipped as out of TTL if it is also out of TTL against the persisted watermark (`canSkipSegmentOutOfTTL`, [L371-L373](https://github.com/apache/pinot/blob/0007e8b9e4a144eb8f102ca1c52f0af3af848963/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java#L371-L373)). The watermark is only persisted after a snapshot ([L1206-L1212](https://github.com/apache/pinot/blob/0007e8b9e4a144eb8f102ca1c52f0af3af848963/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java#L1206-L1212)), which an OFFLINE table never takes. The persisted watermark stays `TTL_WATERMARK_NOT_SET`, so no segment is ever skipped at load either. ## Observed on 1.5.1 Local cluster, OFFLINE upsert table, `mode: FULL`, `snapshot: ENABLE`, `metadataTTL` set, `hashFunction: MURMUR3`, one comparison column (numeric, date based): 1. Upload segments for 300 days (6M primary keys), with `metadataTTL` covering 180 days. 2. While the server runs, the upsert map holds all 6M primary keys: nothing is expired. 3. Delete the segment of a date out of TTL: the server logs `Skip removing segment ... out of TTL` and the primary key count does not change. 4. Restart the server: about 4M primary keys are kept. In 1.5.1, the TTL only trims at load, against the watermark rebuilt from the loaded segments. With master, step 4 no longer trims, because of the persisted watermark check above. ## Why it matters Offline upsert is a natural fit for batch pipelines that push daily deltas instead of rebuilding segments. These pipelines often have a large number of primary keys over the whole history (over a billion in our case, about 145 B per primary key with MURMUR3), while updates only target recent dates. The metadata TTL is the documented way to keep the heap bounded in that case, and today it does not work for OFFLINE tables. ## Possible directions - Trigger the snapshot then the TTL sweep for OFFLINE tables, in that order as the REALTIME flow does: after a segment is added or replaced, and/or from a periodic task on the server (e.g. per partition, debounced). - Persist the watermark after that snapshot, so that segments out of TTL can be skipped at load, as for REALTIME. - Until then, document that `metadataTTL` does not expire primary keys at runtime on OFFLINE tables (or reject it in `TableConfigUtils` for OFFLINE tables). Related: #17789 (offline upsert support), #19271 (offline upsert preload). -- 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]
