saurabhd336 commented on code in PR #8663:
URL: https://github.com/apache/pinot/pull/8663#discussion_r875482662
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -862,7 +863,9 @@ private Map<Integer, SegmentZKMetadata>
getLatestSegmentZKMetadataMap(String rea
* TODO: We need to find a place to detect and update a gauge for
nonConsumingPartitionsCount for a table, and
* reset it to 0 at the end of validateLLC
*/
- public void ensureAllPartitionsConsuming(TableConfig tableConfig,
PartitionLevelStreamConfig streamConfig) {
+ // TODO(saurabh) : We can reduce critical section by having tableName level
locks?
+ public synchronized void ensureAllPartitionsConsuming(TableConfig
tableConfig,
Review Comment:
> It should work without synchronization because it is performing a check
and write
Wasn't aware of this. Looks like helix itself guarantees concurrent updates
are handeled safely. Removed synchronization.
> Let's update the javadoc for this method to include the new scenario:
>
> ```
> If the consuming segment is deleted:
> Check whether there are segments in the PROPERTYSTORE with status DONE,
but no new segment in status IN_PROGRESS, and the state for the latest segment
in the IDEALSTATE is ONLINE
> ```
>
> (Note that this is very similar to the failure between step-1 and step-2,
the only difference is the state in the ideal state)
Ack. Added.
--
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
