Measure the shared tier's connection reuse and operation latency - #14
Merged
Merged
Conversation
Two questions about the S3 backend have no answer in this repository. How
often does a request reuse a connection, and how long does an operation
take? Nothing here has ever measured either: there is no latency metric of
any kind, and the connection pool is whatever the AWS SDK resolved, which
retains a bounded number of idle connections per host. A process making
more concurrent requests than that bound closes connections it is about to
want again, and each replacement pays a TCP handshake and a TLS handshake
before its request is sent at all. Whether that costs anything in practice
is a guess until it is counted.
So count it, in the one place both paths pass through:
- net/http/httptrace's GotConn reports per request whether the transport
had a connection to hand over. A wrapper around the SDK's HTTP client
attaches one trace to every request and counts the two cases apart.
The wrapper wraps rather than replaces: by the time an option function
runs, the client the SDK resolved is already in Options, pool settings
and all, so nothing about the pool is touched by observing it. Every
request the client makes is counted, including retried attempts and
the session a directory bucket negotiates for itself, since those
compete for the same idle connections.
- Each of the four operations is timed into a histogram keyed by
operation and outcome. Twelve pairs at most, fixed at compile time.
Outcomes are ok, miss, and error, because a miss is the cheap path —
no body to transfer — and folding it into successes would make read
latency describe a mixture that moves with the hit rate.
The histogram is hand-rolled, because a dozen float comparisons and an
atomic add is what this needs and a metrics framework would be a direct
dependency for it. Buckets run 1ms to 10s with most of the resolution
below 100ms: that is the band where a request that established a
connection and one that reused an idle one are distinguishable, which is
the whole reason the two measurements belong together. Counts are per
bucket and made cumulative when a snapshot is taken, so an observation
touches one of them rather than all the ones above it, and the sum is
integer nanoseconds because an atomic float sum needs a CAS loop.
The cost of recording, since uploads reach hundreds a second per process:
BenchmarkObserve-12 3271532 367.1 ns/op 0 B/op 0 allocs/op
A get is measured to the open body rather than through it — the reader is
handed to the caller, so draining it is the caller's transfer and not this
tier's round trip — which makes reads first-byte latency. A put is the
whole transfer, because this package does stream that one itself.
Nothing is surfaced yet, and nothing about the pool changes. Tuning it in
the same breath as adding the instrument that would evaluate the tuning is
how a change nobody can read the result of gets made.
Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The measurements the previous commit records are of no use inside the
package that takes them. Carry them out to the two readers that already
exist for every other number here.
- Cache.RemoteStats passes on whatever the backend keeps, by type
assertion against an optional interface rather than a method on
Backend: Noop makes no requests and a test's fake makes them against
nothing, so obliging every implementation to return an empty snapshot
would be ceremony in four places to say so.
- StatusResponse carries it, which is what puts it on the socket's status
report, on the monitoring route, and in the exposition rendered from
the same report — the three cannot disagree because there is still one
place the numbers come from. It is a pointer, so a daemon with no
shared tier says nothing rather than reporting zeros: a local-only
cache has not failed to reuse a connection, it has not tried.
- plaid_cache_remote_requests_total{conn="reused"|"new"} and
plaid_cache_remote_operation_duration_seconds{operation,outcome}.
The family is named for requests rather than connections because
requests is what it counts; conn="new" is the subset that had to
establish one.
- `plaid-cache status` prints the ratio as its conns line, which is the
one figure worth having on a box with no scraper in front of it.
The histogram is the first one this endpoint emits, so renderMetrics
grows a second render path. A histogram declares one HELP and one TYPE
line for a family that then emits three derived series names per label
set — a repeated declaration is not a harmless duplicate, a scraper
rejects the document — so the header and the sample lines are now
separate helpers. The le="+Inf" bucket is written from the count, because
the format requires the two to be equal and storing that number twice is
an invitation for them not to be. Buckets are labelled with the edges the
recording package owns, handed over rather than restated, so the label
and the bound it describes cannot drift apart.
A pair with nothing in it is omitted rather than rendered as a bucket set
of zeros, which would be indistinguishable from a tier that answered
every request instantly.
The exposition test's parser needed the format's one exception to "a
sample's name is its family's" — the bucket, sum, and count series have no
TYPE line of their own — which it now applies only for a family actually
declared as a histogram.
Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two questions about the shared tier have no answer in this repository today.
How often does a request reuse a connection, and how long does an operation
take?
Neither is answerable from what is exposed now. There is no latency metric of
any kind — the
_secondsfamilies are all ages and uptime — and the S3client's connection pool is whatever the AWS SDK resolved, which retains a
bounded number of idle connections per host (10 by default) against a much
larger cap on total connections. A process making more concurrent requests
than that idle bound closes connections it is about to want again, and each
replacement pays a TCP handshake and then a TLS handshake before its request
is sent at all. One client serves both paths — gets on the read path, puts on
the upload pool whose size is configurable — so both can be affected.
Whether that costs anything in practice is a guess until it is counted. This
counts it and changes nothing else.
Connection reuse
net/http/httptrace'sGotConnreports, per request, whether the transporthad a connection to hand over. A wrapper around the SDK's HTTP client attaches
one trace to every request and counts the two cases apart:
The family is named for requests rather than connections because requests is
what it counts;
conn="new"is the subset that had to establish one.newdivided by the total is the number the whole change exists to produce, and
plaid-cache statusprints it as aconnsline for anyone without a scraper.The wrapper wraps rather than replaces. By the time a client option function
runs,
Options.HTTPClientalready holds the client the SDK resolved — poolsettings, dialer and TLS timeouts and all — so wrapping it cannot change any
of them. That is deliberate: the pool is untouched by construction, not by
promise. Every request the client makes is counted, including a retried
attempt and the session a directory bucket negotiates for itself, since those
compete for the same idle connections as the gets and puts.
Operation latency
A histogram per operation and outcome, which is what says whether reuse
matters — a handshake nobody waits on is a curiosity:
operationis one ofget_action,put_action,get_object,put_object,which is also the read/write split: a read blocks the build that asked for it
and a write is best-effort and off the critical path, so the two are not the
same measurement and a percentile over their union describes neither.
outcomeisok,miss, orerror— a miss is the cheap path, with no bodyto transfer, so folding it into successes would make read latency describe a
mixture that moves with the hit rate. Twelve pairs at most, fixed at compile
time, in a repository whose metrics endpoint takes label cardinality
seriously.
It is a histogram rather than a mean because the read path blocks callers and
the tail is the part that costs anything: one request in fifty waiting on a
handshake is invisible in an average and is exactly the thing being looked
for.
The histogram is hand-rolled. No metrics framework: a dozen float comparisons
and an atomic add is what this needs, and the exposition here is already
written by hand for the reasons
renderMetricsgives. Buckets run 1ms to 10swith most of the resolution below 100ms — that is the band where a request
that established a connection and one that reused an idle one are
distinguishable, which is why these two measurements belong in one change. The
edges and the reasoning behind each band are in a comment on
durationBuckets.Recording is meant to be cheaper than what it measures, since uploads can
reach a few hundred a second per process: counts are per bucket and made
cumulative only when a snapshot is taken, and the sum is integer nanoseconds
so it needs no compare-and-swap loop. Measured:
A get is measured to the open body rather than through it — the reader is
handed to the caller, so draining it is the caller's transfer and not this
tier's round trip — which makes reads first-byte latency. A put is the whole
transfer, because this package does stream that one itself. Both are noted in
comments at the measurement points and in the metric's help text.
Plumbing
Both measurements travel the route every other number here takes:
Cache.RemoteStats→StatusResponse→ the socket's status report, themonitoring route, and the exposition rendered from that same report. There is
still one place the numbers come from, so the three cannot disagree.
RemoteStatsreaches the backend through an optionalremote.Statserinterface rather than a new method on
Backend:Noopmakes no requests anda test's fake makes them against nothing, so obliging every implementation to
return an empty snapshot would be ceremony in several places to say so. The
field on
StatusResponseis a pointer, so a local-only daemon publishes notransport families at all — it has not failed to reuse a connection, it has
not tried — and an operation that has not happened publishes no distribution,
rather than a bucket set of zeros indistinguishable from a tier that answered
instantly.
This is the endpoint's first histogram, so
renderMetricsgrows a secondrender path: one HELP and one TYPE line for a family that then emits three
derived series names per label set,
le="+Inf"written from the count becausethe format requires the two to be equal, and buckets labelled with the edges
the recording package owns rather than a second copy of them.
What this deliberately does not do
It does not change the connection pool. That is the obvious follow-up and
it is left for one, on purpose: a change that adjusts behaviour and adds the
instrument that would evaluate the adjustment in the same commit makes the
before and after unreadable. Tune it once there is a reuse ratio to point at.
Defaults are otherwise unchanged, and no dependency was added.
Tests
with keep-alives the second request onward reuses the first's connection,
with them disabled every request opens its own.
server, including that the second of two gets reuses the first's connection,
and that a miss and a failure land under different outcome labels.
rather than an interval: a slow machine can always take longer, but no call
can return before the handler does.
that unobserved pairs are omitted.
declares HELP and TYPE exactly once, and that a local-only daemon emits
neither family.
connsline over the JSON the daemon actually sends, and its absencewhen the daemon sends no accounting.
go build ./...,go vet ./...,go test -race ./...andgolangci-lint run --max-issues-per-linter=0 --max-same-issues=0 ./...are allclean.