Skip to content

Replace EmittedBatchCounter and UpdateCounter with ConcurrentAwaitableCounter#5592

Merged
gianm merged 4 commits intoapache:masterfrom
metamx:concurrent-awaitable-counter-2
Apr 13, 2018
Merged

Replace EmittedBatchCounter and UpdateCounter with ConcurrentAwaitableCounter#5592
gianm merged 4 commits intoapache:masterfrom
metamx:concurrent-awaitable-counter-2

Conversation

@leventov
Copy link
Copy Markdown
Member

@leventov leventov commented Apr 7, 2018

Replace EmittedBatchCounter and UpdateCounter with (both not safe for concurrent increments/updates) with ConcurrentAwaitableCounter (safe for concurrent increments).

Fixes #5485.

@leventov leventov added the Bug label Apr 7, 2018
@leventov leventov added this to the 0.13.0 milestone Apr 7, 2018
… concurrent increments/updates) with ConcurrentAwaitableCounter (safe for concurrent increments)
@leventov leventov force-pushed the concurrent-awaitable-counter-2 branch from 15f572f to 37537b6 Compare April 7, 2018 16:26
@leventov
Copy link
Copy Markdown
Member Author

leventov commented Apr 9, 2018

This PR is ready for review.

Copy link
Copy Markdown
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

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

This LGTM 🤘

{
private static final long MAX_COUNT = Long.MAX_VALUE;

public static long nextCount(long prevCount)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

javadoc?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added

}
}

public void awaitCount(long totalCount, long timeout, TimeUnit unit) throws InterruptedException, TimeoutException
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

javadoc?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added

@leventov leventov modified the milestones: 0.13.0, 0.12.1 Apr 12, 2018
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.

LGTM

@gianm gianm merged commit 124c89e into apache:master Apr 13, 2018
@gianm
Copy link
Copy Markdown
Contributor

gianm commented Apr 13, 2018

@leventov will you do the backport or shall I?

@leventov
Copy link
Copy Markdown
Member Author

@gianm could you please backport?

gianm pushed a commit to gianm/druid that referenced this pull request Apr 13, 2018
…eCounter (apache#5592)

* Replace EmittedBatchCounter and UpdateCounter with (both not safe for concurrent increments/updates) with ConcurrentAwaitableCounter (safe for concurrent increments)

* Fixes

* Fix EmitterTest

* Added Javadoc and make awaitCount() to throw exceptions on wrong count instead of masking errors
@gianm
Copy link
Copy Markdown
Contributor

gianm commented Apr 13, 2018

Sure: #5642

jon-wei pushed a commit that referenced this pull request Apr 13, 2018
…eCounter (#5592) (#5642)

* Replace EmittedBatchCounter and UpdateCounter with (both not safe for concurrent increments/updates) with ConcurrentAwaitableCounter (safe for concurrent increments)

* Fixes

* Fix EmitterTest

* Added Javadoc and make awaitCount() to throw exceptions on wrong count instead of masking errors
@drcrallen drcrallen deleted the concurrent-awaitable-counter-2 branch April 14, 2018 00:07
gianm added a commit to implydata/druid-public that referenced this pull request Apr 16, 2018
…eCounter (apache#5592) (apache#5642)

* Replace EmittedBatchCounter and UpdateCounter with (both not safe for concurrent increments/updates) with ConcurrentAwaitableCounter (safe for concurrent increments)

* Fixes

* Fix EmitterTest

* Added Javadoc and make awaitCount() to throw exceptions on wrong count instead of masking errors
sathishsri88 pushed a commit to sathishs/druid that referenced this pull request May 8, 2018
…eCounter (apache#5592)

* Replace EmittedBatchCounter and UpdateCounter with (both not safe for concurrent increments/updates) with ConcurrentAwaitableCounter (safe for concurrent increments)

* Fixes

* Fix EmitterTest

* Added Javadoc and make awaitCount() to throw exceptions on wrong count instead of masking errors
@leventov leventov mentioned this pull request Jul 31, 2019
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants