Jackie-Jiang commented on code in PR #18980:
URL: https://github.com/apache/pinot/pull/18980#discussion_r4067819244
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/UpsertUtils.java:
##########
@@ -116,6 +119,44 @@ public static void doRemoveDocId(IndexSegment segment, int
docId) {
}
}
+ /// Estimates the total size in bytes of a primary-key-to-record-location
map. Samples up to
+ /// [#PRIMARY_KEY_SAMPLE_SIZE] keys to compute the average key content size,
then extrapolates over all keys,
+ /// so the cost stays bounded regardless of map size. Handles both unhashed
([PrimaryKey]) and hashed
+ /// ([ByteArray]) keys.
+ ///
+ /// @param perEntryOverheadBytes estimated fixed per-entry overhead (e.g.
map node + record location object)
+ public static long estimatePrimaryKeyMapSizeInBytes(Map<Object, ?>
primaryKeyToRecordLocationMap,
+ long perEntryOverheadBytes) {
+ int numKeys = primaryKeyToRecordLocationMap.size();
+ if (numKeys == 0) {
+ return 0;
+ }
+ long sampledKeyBytes = 0;
+ int numSampled = 0;
+ for (Object key : primaryKeyToRecordLocationMap.keySet()) {
+ sampledKeyBytes += getPrimaryKeyContentSizeInBytes(key);
+ if (++numSampled >= PRIMARY_KEY_SAMPLE_SIZE) {
+ break;
+ }
Review Comment:
[P2] The sample limit does not bound map traversal
`ConcurrentHashMap` iteration scans empty buckets, and the backing table
does not shrink after deletions. With fewer than 1,000 remaining keys, this
scans the entire retained table. In an isolated JDK 25 benchmark, 64 keys took
approximately 4 ms with 2 million buckets and 65 ms with 32 million buckets.
This runs synchronously during segment transitions and adds another traversal
after TTL cleanup.
Please avoid arbitrary map traversal if bounded work is required, and
coalesce or rate-limit refreshes per partition. The current O(1) claim is not
valid.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/UpsertUtils.java:
##########
@@ -116,6 +119,44 @@ public static void doRemoveDocId(IndexSegment segment, int
docId) {
}
}
+ /// Estimates the total size in bytes of a primary-key-to-record-location
map. Samples up to
+ /// [#PRIMARY_KEY_SAMPLE_SIZE] keys to compute the average key content size,
then extrapolates over all keys,
+ /// so the cost stays bounded regardless of map size. Handles both unhashed
([PrimaryKey]) and hashed
+ /// ([ByteArray]) keys.
+ ///
+ /// @param perEntryOverheadBytes estimated fixed per-entry overhead (e.g.
map node + record location object)
+ public static long estimatePrimaryKeyMapSizeInBytes(Map<Object, ?>
primaryKeyToRecordLocationMap,
+ long perEntryOverheadBytes) {
+ int numKeys = primaryKeyToRecordLocationMap.size();
+ if (numKeys == 0) {
+ return 0;
+ }
+ long sampledKeyBytes = 0;
+ int numSampled = 0;
+ for (Object key : primaryKeyToRecordLocationMap.keySet()) {
+ sampledKeyBytes += getPrimaryKeyContentSizeInBytes(key);
+ if (++numSampled >= PRIMARY_KEY_SAMPLE_SIZE) {
+ break;
+ }
+ }
+ if (numSampled == 0) {
+ // ponytail: map drained concurrently between size() and keySet()
iteration; nothing sampled.
+ return 0;
+ }
+ double avgKeyBytes = (double) sampledKeyBytes / numSampled;
+ return (long) (numKeys * (avgKeyBytes + perEntryOverheadBytes));
+ }
+
+ private static int getPrimaryKeyContentSizeInBytes(Object key) {
+ if (key instanceof PrimaryKey) {
+ return ((PrimaryKey) key).asBytes().length;
Review Comment:
[P2] Estimate retained memory without serializing every sampled key
`PrimaryKey.asBytes()` allocates and encodes data just to read its length.
Sampling 1,000 four-kilobyte string keys allocated approximately 4.11 MB per
refresh; four 256-character string columns allocated approximately 2.18 MB.
Preloading repeats this work for each completed segment.
Serialized length also misses the `PrimaryKey` wrapper, its values array,
boxed values, and other retained objects. Please estimate directly from the
existing key objects, including object/array overhead. The overall map estimate
should also account for comparison values, which the shallow `RecordLocation`
size excludes.
--
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]
