jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.85k stars 1.91k forks source link

Improve ConnectionStatistics to report #5826

Open sbordet opened 3 years ago

sbordet commented 3 years ago

Jetty version 9.4.x

Description ConnectionStatistics gathers statistics from the connections only when they are closed.

This is not optimal for benchmarks that perform a run, but then keep connections open.

It would be great if ConnectionStatistics (or perhaps another class, say DynamicConnectionStatistics) had a mode where upon an API call (e.g. DynamicConnectionStatistics.collect()) would run through the open connections and gather the statistics from them.

rk1165 commented 3 years ago

Hi @sbordet , Please correct me if I am wrong. This task entails either adding a collect() method in ConnectionStatistics class or creating a new class DynamicConnectionStatistics which will have a collect() method that will gather statistics from the open connections.

sbordet commented 3 years ago

@rk1165 I think ConnectionStatistics works well for its use case and so I think we should not touch it.

This issue is another use case, somehow similar to what is covered by ConnectionStatistics, but different.

Let's say I start some benchmark that opens conn1 and conn2. Both connections are recorded by the new statistics class. Then the benchmark ends, but the connections remain open. I would like to collect the statistics about those connections so far (something that ConnectionStatistics only does when the connections are closed). Say for example that conn1 received 29 bytes and conn2 received 41 bytes. The new statistics class should report the total number of received bytes as 29+41=70 bytes. Then I want to "reset" this new class, and perform another benchmark run, using the existing conn1 and conn2. Then benchmark ends as above. At this point, conn1 will report 61 bytes and conn2 will report 83 bytes. The new statistics class, however, should report only the delta since it was last reset, i.e. (61-29)+(83-41)=74 bytes, or equivalently (61+83)-(29+41) -- it's easier to work on the totals than per-connection.

However, we should take into account the case where conn1 gets closed during a benchmark run. Its totals should be recorded (from the close event), and if a new connection is created, taken into account when the benchmark ends.

This class would be useful in benchmarks, but also via JMX to give more detailed information about the system also for currently open connections, not only for past connections that have been closed.

Fairly easy to implement for bytes sent/received, perhaps less for counts/rates.

gregw commented 3 years ago

@rk1165 regardless of if it is in ConnectionStatistics or a new class, you are correct, this requires a semantic that will gather statistics from open connections. More over it will have to remember the individual totals for each connection so that it doesn't "collect" the same bytes twice.

There is a cost associated with this of keeping a dynamic list of all open connections.... which unfortunately is somewhat contrary to this being used in benchmarking.

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has been a full year without activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has been a full year without activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] commented 10 months ago

This issue has been automatically marked as stale because it has been a full year without activity. It will be closed if no further activity occurs. Thank you for your contributions.