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.

Sign up for GitHub

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

Jump to bottom

ManagedSelector can lose selector thread under high concurrent load #1970

Closed
sbordet opened this issue Nov 14, 2017 · 4 comments
Closed

ManagedSelector can lose selector thread under high concurrent load #1970

sbordet opened this issue Nov 14, 2017 · 4 comments

Comments

@sbordet
Copy link
Contributor

sbordet commented Nov 14, 2017

Under high concurrent load, the following has been observed in a HttpClient dump:

 |       += org.eclipse.jetty.io.ManagedSelector@2227ebf5 id=0 keys=1864 selected=23 - STARTED
 |       |   += EatWhatYouKill@6b36f90e/SelectorProducer@6644bc44/IDLE/ReservedThreadExecutor@org.eclipse.jetty.client.AbstractConnectorHttpClientTransport$ClientSelectorManager@13eb4671{s=38/40,p=0} - STARTED
 |       |       +- SelectorProducer@6644bc44

The ManagedSelector is dumped, reports 1864 keys, but none of the keys is actually printed out.
sbordet added a commit that referenced this issue Nov 14, 2017
@sbordet
Issue #1970 - ManagedSelector loses selector thread.
d196db7 
Remove broken data structure ConcurrentStack (ABA problem).

Made ReservedThreadExecutor use a ConcurrentLinkedDeque
instead of ConcurrentStack.

Removed "waiting" ReservedThread count, since it was
reporting the same as the "size".

Removed instrumentation code from previous commit.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this issue Nov 14, 2017
@sbordet
Issue #1970 - ManagedSelector loses selector thread (#1971)
0f07c65 
* Issue #1970 - ManagedSelector loses selector thread.

Removed broken data structure ConcurrentStack (ABA problem).

Made ReservedThreadExecutor use a ConcurrentLinkedDeque
instead of ConcurrentStack.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet
Copy link
Contributor Author

sbordet commented Nov 14, 2017

Turned out that the problem was the ConcurrentStack data structure that suffered of the ABA problem.

@sbordet sbordet closed this as completed Nov 14, 2017
@sbordet sbordet reopened this Nov 20, 2017
@sbordet
Copy link
Contributor Author

sbordet commented Nov 20, 2017

The issue of losing a selector thread seems still present.

sbordet added a commit that referenced this issue Nov 20, 2017
@sbordet
Issue #1970 - ManagedSelector loses selector thread.
d7d92b5 
Instrumented EWYK to gather additional information
in case the selector thread is lost.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@joakime joakime changed the title ManagedSelector loses selector thread ManagedSelector can lose selector thread under high concurrent load Nov 21, 2017
@gregw gregw mentioned this issue Dec 6, 2017
Closed
gregw added a commit that referenced this issue Dec 14, 2017
@gregw
EWYK cleanups from #1970
d057514 
Signed-off-by: Greg Wilkins <gregw@webtide.com>
gregw added a commit that referenced this issue Dec 14, 2017
@gregw
ManagedSelector dump improvements from #1970
402d869 
ManagedSelector dump improvements from #1970:
 + DumpKeys is now prepended to actions list so it is less likely to be delayed by a stuck/busy selector
 + Timestamps are included for actions and keys which may be separated by time
 + Race removed race for updating dumpKey list while it is being added to.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This was referenced Dec 14, 2017
Merged
Merged
sbordet pushed a commit that referenced this issue Dec 19, 2017
@gregw @sbordet
EWYK cleanups from #1970
0c021f2 
Signed-off-by: Greg Wilkins <gregw@webtide.com>
sbordet added a commit that referenced this issue Dec 28, 2017
@sbordet
Issue #1970 - ManagedSelector dump improvements.
4febaf1 
Code cleanups.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
gregw added a commit that referenced this issue Jan 2, 2018
@gregw
Revert "Issue #1970 - ManagedSelector dump improvements."
f8bc8a6 
This reverts commit 4febaf1.
gregw added a commit that referenced this issue Jan 2, 2018
@gregw
ManagedSelector dump improvements from #1970 (#2062)
54c55b6 
* ManagedSelector dump improvements from #1970

ManagedSelector dump improvements from #1970:
 + DumpKeys is now prepended to actions list so it is less likely to be delayed by a stuck/busy selector
 + Timestamps are included for actions and keys which may be separated by time
 + Race removed race for updating dumpKey list while it is being added to.

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* moved utility timestamp format method to Log

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* updates after review

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* changes after review

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #1970 - ManagedSelector dump improvements.

Code cleanups.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

* Revert "Issue #1970 - ManagedSelector dump improvements."

This reverts commit 4febaf1.

* Fixed imports and other review feedback

Signed-off-by: Greg Wilkins <gregw@webtide.com>
@gregw
Copy link
Contributor

gregw commented Jan 11, 2018

@sbordet can we close this now?

@sbordet
Copy link
Contributor Author

sbordet commented Jan 11, 2018

@gregw I think so, I've never seen it again.

@gregw gregw closed this as completed Jan 11, 2018
@gregw gregw mentioned this issue Oct 8, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@gregw @sbordet