klsince commented on code in PR #12883:
URL: https://github.com/apache/pinot/pull/12883#discussion_r1572699997
##########
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/IngestionBasedConsumptionStatusChecker.java:
##########
@@ -37,62 +38,81 @@ public abstract class
IngestionBasedConsumptionStatusChecker {
// constructor parameters
protected final InstanceDataManager _instanceDataManager;
- protected final Set<String> _consumingSegments;
+ protected final Map<String, Set<String>> _consumingSegments;
+ protected final Function<String, Set<String>> _consumingSegmentsSupplier;
- // helper variable
- private final Set<String> _caughtUpSegments = new HashSet<>();
+ // helper variable, which is thread safe, as the method might be called from
multiple threads when the health check
+ // endpoint is called by many probes.
+ private final Set<String> _caughtUpSegments = ConcurrentHashMap.newKeySet();
Review Comment:
good point. `synchronized` should be good enough, as the method is supposed
to be called via the health check endpoint. It can be called concurrently, but
should rare.
--
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
