Skip to content

catfile: Refactor tracing to allow for batching

Patrick Steinhardt requested to merge pks-catfile-batched-tracing into master

When using the catfile interface, then each request to it will both create an opentracing span and increment a counter. In total, this results in quite a bunch of small short-lived allocations, where we have observed in production that across all RPCs, 12% of all of our allocations where done by opentracing. While useful, this cost is simply too big to warrant us keeping it.

We're about to refactor the catfile interface to allow for more efficient I/O patterns by using batched requests and buffering requests to the process by using a request queue. Like this we optimize how we write to the process so we have less syscalls and thus less context switches. In this world, we can also refactor tracing: instead of creating one span per requested object, we can instead create a single span across the whole lifetime of this batched request.

Naturally, we cannot log as much details as we do right now, where each span contains the revision we did request. Instead, we can tag the spans with how many objects of each type we have requested in total. While less detailed, this should be a good-enough tradeoff such that the spans don't loose all of their benefits.

Refactor the catfile tracing architecture to prepare for this change and to allow for batching of traces by storing per-request-type counters in a new trace structure. The following benchmarks without and with this change show a small reduction in allocations of about 2-3%, which probably stems from the fact that we're not sending the revision anymore:

BenchmarkListAllBlobs/ListAllBlobs-16       8   128331259 ns/op   155930660 B/op    28539 allocs/op
BenchmarkListAllCommits/ListAllCommits-16  39    37256301 ns/op     9203049 B/op    33162 allocs/op
BenchmarkFindAllTags/FindAllTags-16         6   172584078 ns/op    30899380 B/op   138956 allocs/op

BenchmarkListAllBlobs/ListAllBlobs-16       8   127551661 ns/op   155772826 B/op    27936 allocs/op
BenchmarkListAllCommits/ListAllCommits-16  38    38921473 ns/op     9223457 B/op    32429 allocs/op
BenchmarkFindAllTags/FindAllTags-16         6   195437102 ns/op    30978034 B/op   135009 allocs/op

So while this change is not really worth it on its own, it does prepare us to more readily refactor the catfile cache to batch requests while retaining useful tracing info.

Changelog: performance

Part of #3783 (closed)

Merge request reports