feat(blooms): Prefetch bloom blocks as soon as they are built #15050
Conversation
Signed-off-by: Christian Haudum <[email protected]>
salvacorts
changed the title
Signed-off-by: Christian Haudum <[email protected]>
Signed-off-by: Christian Haudum <[email protected]>
salvacorts
changed the title
github-actions
bot
added
the
type/docs
|
|
||
| bqs, err := g.bloomStore.FetchBlocks( | ||
| // We don't use the ctx passed to the handler since its canceled when the handler returns | ||
| context.Background(), |
There was a problem hiding this comment.
Not passing a timeout here since you always need to call the cancel function before returning, which would trigger the download to stop. This is not an issue since when the gateway stops, the queue is closed and any pending block download is stopped.
There was a problem hiding this comment.
LGTM. I'm interested in seeing how this works in practice.
Since prefetch is only ever triggered by the builder, I can think of a few cases where this wouldn't work:
- Gateways get restarted before they can finish prefetching (either from a rollout, an OOM, or something else)
- We scale up gateways; new replicas won't be told to prefetch
I'm not sure how much these matter in practice, but this PR is pretty small so I think it's fine for us to give it a shot.
| @@ -161,6 +161,40 @@ func (g *Gateway) stopping(_ error) error { | |||
| return services.StopManagerAndAwaitStopped(context.Background(), g.serviceMngr) | |||
| } | |||
|
|
|||
| func (g *Gateway) PrefetchBloomBlocks(_ context.Context, req *logproto.PrefetchBloomBlocksRequest) (*logproto.PrefetchBloomBlocksResponse, error) { | |||
There was a problem hiding this comment.
Do we need prefetch-specific metrics so we can measure the impact of this? (for example, how many blocks were downloaded via a prefetch)
What this PR does / why we need it:
This PR adds a new RPC endpoint to the bloom gateway service. Builders pass the blocks they built to the gateway and it download them async. That way blocks will likely be already present in the gateways at query-time.
This can be enabled setting the per-tenant
bloom_prefetch_blockslimit to true.We are also increasing the gateway download queue from 10K to 100K.
Checklist
CONTRIBUTING.mdguide (required)featPRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.mddeprecated-config.yamlanddeleted-config.yamlfiles respectively in thetools/deprecated-config-checkerdirectory. Example PR