Skip to content

HDDS-14860. Synchronize ThrottledAsyncChecker#schedule#9948

Merged
adoroszlai merged 1 commit intoapache:masterfrom
ptlrs:HDDS-14860-ThrottledAsyncChecker-does-not-schedule-checks-in-a-threadsafe-manner
Mar 19, 2026
Merged

HDDS-14860. Synchronize ThrottledAsyncChecker#schedule#9948
adoroszlai merged 1 commit intoapache:masterfrom
ptlrs:HDDS-14860-ThrottledAsyncChecker-does-not-schedule-checks-in-a-threadsafe-manner

Conversation

@ptlrs
Copy link
Contributor

@ptlrs ptlrs commented Mar 18, 2026

What changes were proposed in this pull request?

ThrottledAsyncChecker#schedule has two preconditions:

Is a check already in progress
Was the previous check completed a long time ago
If both of these conditions are satisfied then a new check is scheduled.

However, consider a case where a check was never run or it was run a long time ago. This situation will satisfy both the preconditions. Now if two threads call the same schedule method for the same target/volume, there will be a race condition and/or undefined behavior.

This is because the checksInProgress map is updated only towards the end of the method.

We cannot simply update the checksInProgress at the start of the method as we need the ListenableFuture available before we update that map.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-14860

How was this patch tested?

CI: https://github.com/ptlrs/ozone/actions/runs/23265982507

@Gargi-jais11
Copy link
Contributor

Thanks @ptlrs for the patch. I think you missed this PR raised by @ChenSammi which seems to be duplicated here .

Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Checked the whole class, no other methods require synchronizations.

@adoroszlai adoroszlai marked this pull request as ready for review March 19, 2026 12:09
@adoroszlai adoroszlai changed the title HDDS-14860. Synchronize the schedule method in ThrottledAsyncChecker HDDS-14860. Synchronize ThrottledAsyncChecker#schedule Mar 19, 2026
@adoroszlai adoroszlai merged commit b49226d into apache:master Mar 19, 2026
59 checks passed
@adoroszlai
Copy link
Contributor

Thanks @ptlrs for the patch, @Gargi-jais11, @jojochuang for the review, @ChenSammi for also submitting a patch for the same problem.

@ptlrs
Copy link
Contributor Author

ptlrs commented Mar 19, 2026

Thanks for the patch @ChenSammi. I didn't see that you had also created a PR until @Gargi-jais11 pointed it out.

Thanks for the reviews @Gargi-jais11 @jojochuang @adoroszlai

@ptlrs ptlrs deleted the HDDS-14860-ThrottledAsyncChecker-does-not-schedule-checks-in-a-threadsafe-manner branch March 19, 2026 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants