klsince commented on code in PR #13992:
URL: https://github.com/apache/pinot/pull/13992#discussion_r1762182421


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -1200,12 +1219,45 @@ public void trackSegmentForUpsertView(IndexSegment 
segment) {
     if (_upsertViewManager != null) {
       _upsertViewManager.trackSegment(segment);
     }
+    if (segment instanceof MutableSegment) {
+      trackNewlyAddedSegment(segment);
+    }
   }
 
   @Override
   public void untrackSegmentForUpsertView(IndexSegment segment) {
     if (_upsertViewManager != null) {
       _upsertViewManager.untrackSegment(segment);
     }
+    if (segment instanceof MutableSegment) {
+      untrackNewlyAddedSegment(segment);
+    }
+  }
+
+  @VisibleForTesting
+  void trackNewlyAddedSegment(IndexSegment segment) {
+    if (_newSegmentTrackingTimeMs > 0) {
+      _newlyAddedSegments.put(segment.getSegmentName(), -1L);
+    }
+  }
+
+  @VisibleForTesting
+  void untrackNewlyAddedSegment(IndexSegment segment) {

Review Comment:
   Yes, I intended to track the segment when it’s being added, and untrack it 
so we don’t always add such segments to query’s list of selected segments 
unnecessarily. The delay when to untrack a segment is the time to wait for 
broker to add it to routing table. I assumed broker adds the segment to query’s 
list of selected segments when it’s ONLINE in EV. (But you said IS in your 
comment. I may have missed some details here. Perhaps when uploading a segment, 
it’ll make all replica groups unavailable as checked by the strictReplicaGroup 
routing policy… )
   
   But anyway, as an example, if we upload a segment to upsert table and server 
takes X mins to add it. Then the segment takes at least X mins becomes ONLINE 
in EV, and then broker may take another few seconds to include it in routing 
table and start to add it to query’s list of selected segments. In this case, 
the server needs to track this segment for X mins + a few seconds in order to 
add the segment to query’s selected segment list speculatively. I assume X may 
vary a lot among segments, so I define the delay to be counted after the 
segment is added, to not worry about when X happen to be too long. 



-- 
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

Reply via email to