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

Revisit Statistics classes #5786

Closed
sbordet opened this issue Dec 10, 2020 · 4 comments · Fixed by #5825
Closed

Revisit Statistics classes #5786

sbordet opened this issue Dec 10, 2020 · 4 comments · Fixed by #5825

Comments

@sbordet
Copy link
Contributor

sbordet commented Dec 10, 2020

Jetty version
9.4.x

Description
MonitoredQueuedThreadPool uses a number of statistics classes to record various properties.

MonitoredQueuedThreadPool.getTasks() is wrong, it should call taskLatencyStats.getCount() not taskLatencyStats.getTotal().

Also, replace Atomic* with LongAdder.

Stop using deprecated SampleStatistic.set() method.

@rk1165
Copy link
Contributor

rk1165 commented Dec 18, 2020

Hi @sbordet,
I didn't get this part - "Also, replace Atomic* with LongAdder." I tried searching for Atomic in MonitoredQueuedThreadPool class but couldn't find any reference.

@sbordet
Copy link
Contributor Author

sbordet commented Dec 18, 2020

@rk1165 sorry the replacement of Atomic* with LongAdder has already been done in #2288 -- ignore it.

However, please do fix getTasks() and replace usages of deprecated set() methods.

Work from branch jetty-9.4.x.

Thanks!

@rk1165
Copy link
Contributor

rk1165 commented Dec 18, 2020

@sbordet
Created the following PR #5825
Please review.

@sbordet sbordet added this to To do in Jetty 9.4.36 via automation Dec 18, 2020
@sbordet sbordet moved this from To do to Reviewer approved in Jetty 9.4.36 Dec 18, 2020
@sbordet sbordet linked a pull request Dec 18, 2020 that will close this issue
sbordet added a commit that referenced this issue Jan 12, 2021
Issue #5786 : Revisit Statistics classes
@sbordet
Copy link
Contributor Author

sbordet commented Jan 13, 2021

Thanks @rk1165!

@sbordet sbordet closed this as completed Jan 13, 2021
Jetty 9.4.36 automation moved this from Reviewer approved to Done Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants