Skip to content

catfile: Stop creating decorrelated spans

Patrick Steinhardt requested to merge pks-catfile-drop-decorrelated-traces into master

Processes spawned by the catfile cache are potentially decorrelated from the main context such that these processes can be kept around after the RPC context has been cancelled for later reuse. In order to make these cached processes observable, we create all tracing spans for requests to the catfile process both in the current per-RPC context and in the decorrelated context hosted by the cache.

We have since observed that creating traces is comparatively expensive for us given that it creates many allocations and also has quite some runtime overhead. Doing this work twice per request only makes this problem worse for us. While one way to fix this is the ongoing work to allow batching requests, another angle to tackle this problem is to only start creating one context.

Naturally, we want to keep the per-RPC spans such that one can easily see what's going on in a given RPC. The information about how the catfile process is used globally across different RPC calls isn't really all that useful in the first place: while it gives a global picture, we wouldn't ever see any activity outside of the context of an RPC anyway.

Drop support for creating decorrelated spans in the context of the cache. This not only simplifies the code, but also optimizes away some allocations and gives us a slight speedup. Before this change:

BenchmarkListAllBlobs/ListAllBlobs-16        8   133269398 ns/op   155884456 B/op    27954 allocs/op
BenchmarkListAllCommits/ListAllCommits-16   40    43940502 ns/op     9231545 B/op    32434 allocs/op
BenchmarkFindAllTags/FindAllTags-16          6   185349615 ns/op    31240846 B/op   134990 allocs/op

After this change:

BenchmarkListAllBlobs/ListAllBlobs-16        8   130457695 ns/op   155643219 B/op    25795 allocs/op
BenchmarkListAllCommits/ListAllCommits-16   39    40942865 ns/op     9171181 B/op    30244 allocs/op
BenchmarkFindAllTags/FindAllTags-16          6   172061555 ns/op    30800354 B/op   122930 allocs/op

We have roughly 10% less memory allocations and a slight speedup.

Changelog: performance

Part of #3783 (closed)

Merge request reports