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

SimpleNettyTransportTests#testTracerLog stalls #10187

Closed
javanna opened this issue Mar 20, 2015 · 2 comments
Closed

SimpleNettyTransportTests#testTracerLog stalls #10187

javanna opened this issue Mar 20, 2015 · 2 comments
Assignees
Labels
>test Issues or PRs that are addressing/adding tests

Comments

@javanna
Copy link
Member

javanna commented Mar 20, 2015

SimpleNettyTransportTests#testTracerLog stalls on my macbook, I saw this a couple of times today while running the whole core suite from command line. The suite timeout kicks in at some point and stops it.

@javanna javanna added the >test Issues or PRs that are addressing/adding tests label Mar 20, 2015
@javanna
Copy link
Member Author

javanna commented Mar 20, 2015

Assigning this to @bleskes as he has the logs and said he knows what is going on. Thanks @bleskes !

@bleskes
Copy link
Contributor

bleskes commented Mar 23, 2015

bleskes added a commit to bleskes/elasticsearch that referenced this issue Mar 23, 2015
If a request comes in at the same moment the timeout handler for it runs, we may leak a timeoutInfoHolder and erroneously log "Transport response handler not found of id" . The same issue could cause the request tracer to fire a traceUnresolvedResponse call instead of traceReceivedResponse , causing a failure of testTracerLog ( see elastic#10187 ) .

This commit synchronizes the two execution paths and also unifies the TransportService.Adapter#remove(requestId) with TransportService.Adapter#onResponseReceived(requestId), as they are always called together to indicate a response was received.

Closes elastic#10187
bleskes added a commit that referenced this issue Mar 23, 2015
If a request comes in at the same moment the timeout handler for it runs, we may leak a timeoutInfoHolder and erroneously log "Transport response handler not found of id" . The same issue could cause the request tracer to fire a traceUnresolvedResponse call instead of traceReceivedResponse , causing a failure of testTracerLog ( see #10187 ) .

This commit makes sure timeoutInfoHolder is visible before removing the corresponding RequestHolder. It also unifies the TransportService.Adapter#remove(requestId) with TransportService.Adapter#onResponseReceived(requestId), as they are always called together to indicate a response was received.

Closes #10187
Closes #10220
bleskes added a commit that referenced this issue Mar 23, 2015
If a request comes in at the same moment the timeout handler for it runs, we may leak a timeoutInfoHolder and erroneously log "Transport response handler not found of id" . The same issue could cause the request tracer to fire a traceUnresolvedResponse call instead of traceReceivedResponse , causing a failure of testTracerLog ( see #10187 ) .

This commit makes sure timeoutInfoHolder is visible before removing the corresponding RequestHolder. It also unifies the TransportService.Adapter#remove(requestId) with TransportService.Adapter#onResponseReceived(requestId), as they are always called together to indicate a response was received.

Closes #10187
Closes #10220
mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015
If a request comes in at the same moment the timeout handler for it runs, we may leak a timeoutInfoHolder and erroneously log "Transport response handler not found of id" . The same issue could cause the request tracer to fire a traceUnresolvedResponse call instead of traceReceivedResponse , causing a failure of testTracerLog ( see elastic#10187 ) .

This commit makes sure timeoutInfoHolder is visible before removing the corresponding RequestHolder. It also unifies the TransportService.Adapter#remove(requestId) with TransportService.Adapter#onResponseReceived(requestId), as they are always called together to indicate a response was received.

Closes elastic#10187
Closes elastic#10220
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>test Issues or PRs that are addressing/adding tests
Projects
None yet
Development

No branches or pull requests

2 participants