Add in flight request metrics #3252
Conversation
This tracks CPU and DB usage while requests are in flight, rather than when we write the response.
erikjohnston
force-pushed
the
erikj/in_flight_requests
branch
from
7d04cd8 to
dfa70ad
Compare
synapse/http/request_metrics.py
Outdated
| ) | ||
|
|
||
|
|
||
| # The set of all in flight requests. |
There was a problem hiding this comment.
could you give this some type info: in particular that it contains _RequestMetricses?
synapse/http/request_metrics.py
Outdated
| counts[key] = counts.get(key, 0) + 1 | ||
|
|
||
| for (method, name), count in counts.iteritems(): | ||
| _in_flight_requests_count.set(count, method, name) |
There was a problem hiding this comment.
will this not end up with stale results, because we never properly clear it?
There was a problem hiding this comment.
This gets run before every call to /metrics, so should be fine. OTOH we may as well make it clearer by using a callback metrics
There was a problem hiding this comment.
yes, but we'll still end up with stale items in _in_flight_requests_count ?
+1 to fixing with a callbackmetrics
synapse/http/request_metrics.py
Outdated
|
|
||
| def __init__(self, context, ru_utime, ru_stime, db_txn_count, | ||
| db_txn_duration_ms, db_sched_duration_ms): | ||
| self.context = context |
There was a problem hiding this comment.
is this actually used anywhere? I can't see it
There was a problem hiding this comment.
It's mainly used in update to allow it to call from_context
There was a problem hiding this comment.
oh yes. Why not pass it through from RequestMetrics, since that already knows the context? It would distinguish more clearly between the data (_RequestStats) and the thing to manage it (RequestMetrics)
This tracks CPU and DB usage while requests are in flight, rather than
when we write the response.