kfaraz commented on code in PR #17723:
URL: https://github.com/apache/druid/pull/17723#discussion_r1957024123
##########
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:
Hmm. @suraj-goel , I don't think the latest change will help either since
the `status` may still be changed by a thread that has not even acquired the
`pauseLock`. Apparently, changing the `status` requires the `statusLock` and I
am not sure if it would be wise to acquire both `statusLock` and `pauseLock`
together in the `pause()` method.
`SeekableStreamIndexTaskRunner` seems to be in need of a serious overhaul as
there are far too many locks and conditions floating around. I don't want to
make hasty changes that make things even more complicated.
##########
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:
> P.S. With the previous commit, there are not issues related to task
failures anymore.
Do you mean the change where you had copied over the value of `status` into
`currentStatus`?
--
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]
