Skip to content

Thread-safety for ResponseContext.REGISTERED_KEYS#9667

Merged
gianm merged 1 commit intoapache:masterfrom
alex-plekhanov:issue-9106
Jan 8, 2021
Merged

Thread-safety for ResponseContext.REGISTERED_KEYS#9667
gianm merged 1 commit intoapache:masterfrom
alex-plekhanov:issue-9106

Conversation

@alex-plekhanov
Copy link
Copy Markdown
Contributor

@alex-plekhanov alex-plekhanov commented Apr 10, 2020 •
edited
Loading

Fixes #9106

Description

This PR fixes potential concurrency problems of ResponseContext.REGISTERED_KEYS.
TreeMap was replaced with ConcurrentSkipList to make ResponseContext.REGISTERED_KEYS thread-safe.

This PR has:

  • been self-reviewed.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
Key changed/added classes in this PR
  • ResponseContext

@stale
Copy link
Copy Markdown

stale Bot commented Jun 9, 2020

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@druid.apache.org list. Thank you for your contributions.

@stale stale Bot added the stale label Jun 9, 2020
@alex-plekhanov
Copy link
Copy Markdown
Contributor Author

alex-plekhanov commented Jun 9, 2020

Up

@stale
Copy link
Copy Markdown

stale Bot commented Jun 9, 2020

This issue is no longer marked as stale.

@stale stale Bot removed the stale label Jun 9, 2020
@stale
Copy link
Copy Markdown

stale Bot commented Aug 8, 2020

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@druid.apache.org list. Thank you for your contributions.

@stale stale Bot added the stale label Aug 8, 2020
@stale
Copy link
Copy Markdown

stale Bot commented Aug 11, 2020

This pull request/issue is no longer marked as stale.

@stale stale Bot removed the stale label Aug 11, 2020
@stale
Copy link
Copy Markdown

stale Bot commented Oct 11, 2020

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@druid.apache.org list. Thank you for your contributions.

@stale stale Bot added the stale label Oct 11, 2020
@alex-plekhanov
Copy link
Copy Markdown
Contributor Author

alex-plekhanov commented Oct 12, 2020

up

@stale
Copy link
Copy Markdown

stale Bot commented Oct 12, 2020

This issue is no longer marked as stale.

@stale stale Bot removed the stale label Oct 12, 2020
@stale
Copy link
Copy Markdown

stale Bot commented Dec 12, 2020

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@druid.apache.org list. Thank you for your contributions.

@stale stale Bot added the stale label Dec 12, 2020
@stale
Copy link
Copy Markdown

stale Bot commented Dec 14, 2020

This pull request/issue is no longer marked as stale.

@stale stale Bot removed the stale label Dec 14, 2020
gianm
gianm approved these changes Jan 8, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@gianm gianm 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 to me. I suppose another solution would be to make registerKey private, but perhaps it would be called in an extension.

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Jan 8, 2021

Thanks @alex-plekhanov!

@gianm gianm merged commit 26bcd47 into apache:master Jan 8, 2021
JulianJaffePinterest pushed a commit to JulianJaffePinterest/druid that referenced this pull request Jan 22, 2021
@alex-plekhanov @JulianJaffePinterest
Thread-safety for ResponseContext.REGISTERED_KEYS (apache#9667)
06be9a9
@jihoonson jihoonson added this to the 0.21.0 milestone Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gianm gianm gianm approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.21.0

Development

Successfully merging this pull request may close these issues.

Unbalanced synchronization of ResponseContext.REGISTERED_KEYS

3 participants

@alex-plekhanov @gianm @jihoonson