Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed filters execution order and fix potential concurrency issue in filter chains #7023

Merged
merged 2 commits into from Jul 28, 2014

Conversation

javanna
Copy link
Member

@javanna javanna commented Jul 24, 2014

Fix filters ordering which seems to be the opposite to what it should be (#7019). Added missing tests for rest filters.

Solved concurrency issue in both rest filter chain and transport action filter chain (#7021).

Closes #7019
Closes #7021

@javanna javanna added the review label Jul 24, 2014
@javanna javanna self-assigned this Jul 24, 2014
@javanna javanna changed the title Internal: fixed filters execution order and fix concurrency issue in filter chains Internal: fixed filters execution order and fix potential concurrency issue in filter chains Jul 25, 2014
@@ -146,12 +148,12 @@ public void run() {

private class TransportActionFilterChain implements ActionFilterChain {

private volatile int index = 0;
private AtomicInteger index = new AtomicInteger();
Copy link
Member

Choose a reason for hiding this comment

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

can this be final?

Copy link
Member Author

Choose a reason for hiding this comment

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

sure, will change that!

@kimchy
Copy link
Member

kimchy commented Jul 25, 2014

LGTM with 2 minor comments

…ction filter position

Also improved filter chain tests to not rely on execution time, and made filter chain tests look more similar to what happens in reality by removing multiple threads creation in testTooManyContinueProcessing (something we don't support anyway, makes little sense to test it).

Closes elastic#7021
@javanna javanna merged commit 4e5ad56 into elastic:master Jul 28, 2014
@jpountz jpountz removed the review label Jul 29, 2014
@clintongormley clintongormley changed the title Internal: fixed filters execution order and fix potential concurrency issue in filter chains Internal: Fixed filters execution order and fix potential concurrency issue in filter chains Sep 10, 2014
@clintongormley clintongormley changed the title Internal: Fixed filters execution order and fix potential concurrency issue in filter chains Fixed filters execution order and fix potential concurrency issue in filter chains Jun 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants