Jackie-Jiang commented on code in PR #10463:
URL: https://github.com/apache/pinot/pull/10463#discussion_r1237936450
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java:
##########
@@ -489,14 +489,16 @@ static void setDefaultTimeValueIfInvalid(TableConfig
tableConfig, Schema schema,
@Override
public void addSegment(ImmutableSegment immutableSegment) {
- if (isUpsertEnabled()) {
- handleUpsert(immutableSegment);
- return;
- }
+ if (immutableSegment instanceof ImmutableSegmentImpl) {
Review Comment:
Interesting point. If we are replacing a segment that was a consuming
segment, the partition id can be derived from the segment name, which is fine.
If the segment was uploaded, then we need to keep the partition in the segment
metadata the same as the replaced segment. But we cannot skip the empty segment
for upsert handling.
On the other hand, I think we might be able to avoid uploading empty
segment. When we find one segment does not contain any valid doc, we can
directly delete the segment, instead of uploading an empty segment to replace
it. This logic can be applied in the segment generator. In the executor, if we
find a segment does not contain any valid doc, we can skip processing it (note
that the valid doc id read from executor might not be the same as the one read
from the generator).
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
