Skip to content

Merge druid-api, druid-common and java-util modules into druid-common; Replace EmittedBatchCounter and UpdateCounter with ConcurrentAwaitableCounter#5488

Closed
leventov wants to merge 2 commits intoapache:masterfrom
metamx:concurrent-awaitable-counter
Closed

Conversation

@leventov
Copy link
Copy Markdown
Member

EmittedBatchCounter and UpdateCounter are both not safe for concurrent increments/updates. ConcurrentAwaitableCounter is safe for concurrent increments.

Fixes #5485. I had to collapse druid-api, druid-common and java-util, to avoid cyclic dependencies. It was a part of #4312 plan.

…; Replace EmittedBatchCounter and UpdateCounter with (both not safe for concurrent increments/updates) with ConcurrentAwaitableCounter (safe for concurrent increments)
@gianm
Copy link
Copy Markdown
Contributor

gianm commented Mar 14, 2018

I know it is more of a pain, but I think it'd be nice to do this as two patches, one for the combine and one for the bug fix. It will make the git history more readable. Not to mention, the review of the bugfix will be more readable.

I think the combining of those modules makes total sense at this point.

@b-slim
Copy link
Copy Markdown
Contributor

b-slim commented Mar 14, 2018

@leventov can we do this in 2 different PRs? like that we can revert if needed, sorry for extra work

@leventov leventov closed this Mar 14, 2018
@leventov leventov deleted the concurrent-awaitable-counter branch March 14, 2018 22:04
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.

3 participants