Jackie-Jiang commented on code in PR #13668:
URL: https://github.com/apache/pinot/pull/13668#discussion_r1714463263
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/IngestionDelayTracker.java:
##########
@@ -214,7 +230,7 @@ private long getPartitionOffsetLag(IngestionOffsets offset)
{
*
* @param partitionGroupId partition ID which we should stop tracking.
*/
- private void removePartitionId(int partitionGroupId) {
+ private synchronized void removePartitionId(int partitionGroupId) {
Review Comment:
The above handling could leave the map and metrics in an inconsistent state
where the map contains the value but the metrics are not added. It could cause
other issues because metrics will never be added.
Good point on multiple partitions sharing the same ingestion delay tracker.
To reduce the contention, I can add a per partition lock so that different
partition won't intervene with each other. Since we only have one consuming
thread per partition, the overhead should be minimal
--
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]
