-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix ConcurrentModificationException in IncrementalPublishingKafkaIndexTaskRunner #5907
Conversation
@@ -1504,7 +1505,7 @@ public DateTime getStartTime(@Context final HttpServletRequest req) | |||
private final int sequenceId; | |||
private final String sequenceName; | |||
private final Map<Integer, Long> startOffsets; | |||
private final Map<Integer, Long> endOffsets; | |||
private final ConcurrentHashMap<Integer, Long> endOffsets; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please include a comment that explains two things:
- why this is a
ConcurrentHashMap
- who is allowed to modify it (do you need to be a specific thread or acquire a specific lock)? I bet that if multiple threads modify this concurrently then unexpected things will happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the problem is that setEndOffsets()
can be called by both the main thread and the http thread. Actually setEndOffsets()
also sets checkpointed
variable too, so I changed to acquire a lock before modifying endOffsets
and checkpointed
variables.
I bet that if multiple threads modify this concurrently then unexpected things will happen.
I have no idea about this.
…xTaskRunner (apache#5907) * Fix ConcurrentModificationException in IncrementalPublishingKafkaIndexTaskRunner * fix lock and add comments
…xTaskRunner (apache#5907) Ported without cherry-pick, since the original commit depends on the patch that splits KafkaIndexTask.
…xTaskRunner (apache#5907) Ported without cherry-pick, since the original commit depends on the patch that splits KafkaIndexTask.
Fixes #5745.
This change is![Reviewable](https://camo.githubusercontent.com/23b05f5fb48215c989e92cc44cf6512512d083132bd3daf689867c8d9d386888/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)