suraj-goel commented on code in PR #17723:
URL: https://github.com/apache/druid/pull/17723#discussion_r1957025904
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskRunner.java:
##########
@@ -1887,10 +1887,12 @@ public Response pauseHTTP(
@VisibleForTesting
public Response pause() throws InterruptedException
{
- if (!(status == Status.PAUSED || status == Status.READING)) {
+ Status currentStatus = status;
+ if (!(currentStatus == Status.PAUSED || currentStatus == Status.READING)) {
Review Comment:
> SeekableStreamIndexTaskRunner seems to be in need of a serious overhaul as
there are far too many locks and conditions floating around
Agree @kfaraz !
IMO, This solution should work because in this particular case, the race
condition is between threads running `possiblyPause` and `pause` methods.
> Do you mean the change where you had copied over the value of status into
currentStatus?
Yes! No issues related to this bug from the past 2 days.
Assigning to local variable seems to be a safe and non breaking solution for
this bug. We can refactor this class separately to use the locks in the right
way.
--
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]
