Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(blooms): bloom integration in query planning #12208

Merged
merged 72 commits into from
Mar 26, 2024
Merged

feat(blooms): bloom integration in query planning #12208

merged 72 commits into from
Mar 26, 2024

Conversation

owen-d
Copy link
Member

@owen-d owen-d commented Mar 14, 2024 •
edited
Loading

TODO:

  • proto extensions
  • wiring through storage
  • GetShards implementation on indexshipper
  • GetShards implementation on idx-gw
  • Graceful fallback when not implemented
  • integration into queryrange (codec, etc)
  • http routes
  • Integration into sharding middleware
  • Integration into logql engine
  • storage matcher compat
  • ingester inverted index integration

owen-d added 3 commits March 13, 2024 17:08
@owen-d
streaming GetShards on idx-gw + some refactoring
b831d07 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
moves FingerprintBounds to logproto pkg to avoid circular imports
91ee79e 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
proto alignment, wiring GetShards() through storage
2db2852 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d owen-d requested a review from a team as a code owner March 14, 2024 02:15
@owen-d
tsdb index ForSeries() can signal a stop to iteration
6801fbb 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
chaudum
chaudum reviewed Mar 14, 2024
View reviewed changes
pkg/storage/stores/composite_store.go Outdated
Comment on lines 229 to 230
case err != nil:
return nil, err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move the error handling into a regular if err != nil statement

pkg/storage/stores/shipper/indexshipper/gatewayclient/gateway_client.go Outdated
if err != nil {
return errors.WithStack(err)
}
res.Shards = append(res.Shards, resp.Shards...)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The append is error prone here. poolDo() retries the job with the next instance in the replication set of a tenant in case of an error.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch, tyvm.

owen-d added 7 commits March 14, 2024 10:37
@owen-d
ForSeries in tsdb.Index ifc
c2200a0 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
pr feedback
db1c830 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
removes pointers in protos
693c48b 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
gateway GetShards impl
808d88b 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
[wip] integrating bound ranges into GetShards()
391093f 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
generic result accumulator
42dccdd 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
indexclient getshards impl
cc105e0 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
owen-d added 6 commits March 14, 2024 18:58
@owen-d
comment
b355b9f 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
thread plan+predicate
14e4815 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
[wip] HasForSeries wiring
c42344e 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
finish HasForSeries impl, move ifc to index pkg
3052e2f 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
moves HasForSeries to StatsReader
548fe30 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
new sharding pkg
272e06f 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
owen-d added 6 commits March 15, 2024 15:30
@owen-d
removes bounds from shardsrequest
cfb38a5 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
HasForSeries accepts timerange + composite store impl
8988232 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
resultAccumulator doesnt run merge for single list
300d710 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
sorts MultiIndex.GetChunkRefs during merge
1ea82b1 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
naive implementation of bloom-accelerated sharding calculation in gat…
6bad531 
…eway

Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
extracts & tests accumulating chunks to shards
09f1618 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
owen-d added 3 commits March 21, 2024 12:53
@owen-d
Merge remote-tracking branch 'upstream/main' into blooms/query-planni…
37edc8b 
…ng-i

Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
index statistics
b0350ae 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
Merge remote-tracking branch 'upstream/main' into blooms/query-planni…
5b70e96 
…ng-i

Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d owen-d force-pushed the blooms/query-planning-i branch from 08ed82e to 5b70e96 Compare March 22, 2024 00:20
@owen-d
stats cleanup and proper propagation
db506cb 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d owen-d force-pushed the blooms/query-planning-i branch from 6437c38 to db506cb Compare March 22, 2024 01:04
owen-d added 2 commits March 21, 2024 18:39
@owen-d
routing stats through store
5ca8d2f 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
gatewayclient propagates stats
d43b3f7 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d owen-d force-pushed the blooms/query-planning-i branch from 89fd586 to d43b3f7 Compare March 22, 2024 04:42
owen-d added 8 commits March 22, 2024 09:22
@owen-d
logging fixes
52df554 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
Merge remote-tracking branch 'upstream/main' into blooms/query-planni…
ce33787 
…ng-i

Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
Merge remote-tracking branch 'upstream/main' into blooms/query-planni…
3e96e75 
…ng-i

Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
skip (unhealthy?) bloom-gws with no token range
1e77cee 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
correctly merges stats in collector middleware for bloom data, some a…
fed5262 
…ddtl logging

Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
v1 metrics wired up through read path
2805339 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
optional pool support for bloom pages
11d8b42 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d
Merge remote-tracking branch 'upstream/main' into blooms/query-planni…
ade14a9 
…ng-i

Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d owen-d force-pushed the blooms/query-planning-i branch from c9ce04a to 2041eed Compare March 26, 2024 03:33
@owen-d
test signature
463dafd 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d owen-d force-pushed the blooms/query-planning-i branch from 2041eed to 463dafd Compare March 26, 2024 04:03
salvacorts
salvacorts approved these changes Mar 26, 2024
View reviewed changes
Copy link
Contributor

@salvacorts salvacorts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed, since:

  • @chaudum and I already reviewed the PR at a high level and we haven't noticed anything obviously wrong.
  • The CI is happy
  • This should be safe to rollout: QFs fallback to linear sharding if index-gws don't implement GetShards.
  • We have been running this in dev for some days already.

We are merging this PR.

@salvacorts salvacorts merged commit a36483b into grafana:main Mar 26, 2024
11 checks passed
grafanabot pushed a commit that referenced this pull request Mar 26, 2024
@owen-d @grafana-delivery-bot
feat(blooms): bloom integration in query planning (#12208)
4eec989 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
(cherry picked from commit a36483b)
@grafanabot grafanabot mentioned this pull request Mar 26, 2024
Merged
11 tasks
@loki-gh-app loki-gh-app bot mentioned this pull request Mar 27, 2024
Closed
trevorwhitney added a commit that referenced this pull request Mar 27, 2024
@trevorwhitney
chore: merge with origin/main
99d57d2 
Squashed commit of the following:

commit 15dc2ba
Author: Christian Haudum <christian.haudum@gmail.com>
Date:   Wed Mar 27 19:55:13 2024 +0100

    feat(bloomstore): Support for sharding blocks across multiple different directories (#12375)

    Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

commit c1edb82
Author: J Stickler <julie.stickler@grafana.com>
Date:   Wed Mar 27 14:46:00 2024 -0400

    docs: fix formatting in structured metadata topic (#12382)

commit c9e5c7f
Author: Owen Diehl <ow.diehl@gmail.com>
Date:   Wed Mar 27 10:27:44 2024 -0700

    chore(blooms): removes bloom-gw & bloom-compactor from all non-microservice targets (#12381)

commit 2b8db8b
Author: Travis Patterson <travis.patterson@grafana.com>
Date:   Wed Mar 27 08:48:04 2024 -0600

    fix: attempt non-string label filtering when errors are present (#12378)

commit f99e6ac
Author: Dylan Guedes <djmgguedes@gmail.com>
Date:   Wed Mar 27 10:51:21 2024 -0300

    docs: Add missing changelog entry for mTLS memcached addition (#12377)

commit 016daa5
Author: Karsten Jeschkies <karsten.jeschkies@grafana.com>
Date:   Wed Mar 27 14:28:32 2024 +0100

    fix: Track all OTLP metadata bytes in usage tracker. (#12376)

    Co-authored-by: Vladyslav Diachenko <82767850+vlad-diachenko@users.noreply.github.com>

commit aca2a38
Author: Paul Rogers <129207811+paul1r@users.noreply.github.com>
Date:   Wed Mar 27 07:50:25 2024 -0400

    test: gcs object client test data race (#12324)

commit bdad1d3
Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Date:   Wed Mar 27 10:52:30 2024 +0000

    chore(deps): update module google.golang.org/protobuf to v1.33.0 [security] (main) (#12137)

    Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
    Co-authored-by: Michel Hollands <42814411+MichelHollands@users.noreply.github.com>

commit ca667aa
Author: Christian Haudum <christian.haudum@gmail.com>
Date:   Wed Mar 27 09:53:33 2024 +0100

    chore(bloom-gw): Cleanup tracing implementation (#12368)

    The huge amount of `bloomgateway.ProcessTask` spans caused problem with Tempo's span limit, resulting in dropped/dangling spans.

    Since the processing time is now recorded also in a metrics.go-like stats log line, we can remove the spans and only add a log event with the summary to the main span of the request handler.

    This PR also unifies the event logging in the index gateway, and adds the real processing time (not aggregated processing time) to the request stats.

    ---
    Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

commit 120b76d
Author: Owen Diehl <ow.diehl@gmail.com>
Date:   Wed Mar 27 00:16:07 2024 -0700

    chore(blooms): misc fixes (#12369)

    unbounds bloom-gw-client concurrency

    removes unused log_gateway_requests param

    adds note wrt bloom-gw results cache keygen issue

    add msg field to stats log

    sets bloomshipper download workers count to 16 default

commit 19c046f
Author: Christian Haudum <christian.haudum@gmail.com>
Date:   Tue Mar 26 23:07:02 2024 +0100

    chore(blooms): Make max bloom page size for querying configurable (#12337)

    Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

commit 9810e8e
Author: Owen Diehl <ow.diehl@gmail.com>
Date:   Tue Mar 26 11:57:27 2024 -0700

    chore(blooms): computed fields for bloomgw stats logging (#12367)

commit 6b3bbb3
Author: J Stickler <julie.stickler@grafana.com>
Date:   Tue Mar 26 14:22:48 2024 -0400

    docs: Update release notes for 2.9.6 (#12365)

commit 30ac88b
Author: Owen Diehl <ow.diehl@gmail.com>
Date:   Tue Mar 26 11:00:47 2024 -0700

    chore(blooms): increase blockpool by factor-of-2 (#12363)

commit d3266a1
Author: Pangidoan Butar <38452094+doanbutar@users.noreply.github.com>
Date:   Wed Mar 27 01:53:05 2024 +0800

    docs: Update structured_metadata.md (#12355)

    Co-authored-by: J Stickler <julie.stickler@grafana.com>

commit c0c7a19
Author: J Stickler <julie.stickler@grafana.com>
Date:   Tue Mar 26 11:30:00 2024 -0400

    docs: Update release notes for recent 2.9 releases (#12349)

commit 9af191f
Author: Christian Haudum <christian.haudum@gmail.com>
Date:   Tue Mar 26 15:09:40 2024 +0100

    feat(bloom-gw): Add `metrics.go` style log line to bloom gateway `FilterChunks` call (#12354)

    This PR adds a "metrics.go" style log line to the bloom gateway that contains latencies for queueing, processing, post-processing, as well as number of chunks/series requested/filtered.

    This log line should give operators a better understanding where time is spent for individual requests.

    Example log line:

    ```
    ts=2024-03-26T13:16:11.869619964Z caller=spanlogger.go:109 component=bloom-gateway tenant=XXX method=bloomgateway.FilterChunkRefs user=XXX traceID=6239d49e1e88f88645bd7f1f6f1b85c8 level=info status=success tasks=1 series_requested=35 series_filtered=31 chunks_requested=103 chunks_filtered=93 queue_time=4.108128936s metas_fetch_time=22.040408ms blocks_fetch_time=1.284575ms processing_time=30.215863002s post_processing_time=16.084µs
    ```

    Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

commit 5c6a031
Author: Salva Corts <salva.corts@grafana.com>
Date:   Tue Mar 26 14:59:26 2024 +0100

    fix(blooms): Not filters should always match (#12358)

commit 64c7812
Author: Ashwanth <iamashwanth@gmail.com>
Date:   Tue Mar 26 18:55:12 2024 +0530

    fix(codec): inject disable wrappers in ctx (#12352)

commit a36483b
Author: Owen Diehl <ow.diehl@gmail.com>
Date:   Tue Mar 26 03:36:49 2024 -0700

    feat(blooms): bloom integration in query planning (#12208)

    Signed-off-by: Owen Diehl <ow.diehl@gmail.com>

commit d5ecf9a
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Tue Mar 26 10:21:48 2024 +0000

    chore(deps): bump google.golang.org/protobuf from 1.32.0 to 1.33.0 in /operator (#12207)

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
    Co-authored-by: Periklis Tsirakidis <periklis@redhat.com>

commit 774b01d
Author: Ashwanth <iamashwanth@gmail.com>
Date:   Tue Mar 26 13:15:54 2024 +0530

    fix(ruler): pass noop analyseRules to rules manager (#12353)

commit 03982c2
Author: arukiidou <arukiidou@yahoo.co.jp>
Date:   Tue Mar 26 16:10:31 2024 +0900

    docs: fix otlp guide - collector config example (#12328)

    Signed-off-by: junya koyama <arukiidou@yahoo.co.jp>
    Co-authored-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>

commit 664e569
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Mon Mar 25 16:12:58 2024 -0600

    feat: prepare 3.0.0 release candidate (#12348)

    Release-As: 3.0.0-alpha.1

commit 40f695e
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Mon Mar 25 15:29:12 2024 -0600

    ci: prepare 3.0 release workflow (#12344)

    Release-As: 3.0.0-alpha.1

commit 04398a1
Author: Travis Patterson <travis.patterson@grafana.com>
Date:   Mon Mar 25 14:52:24 2024 -0600

    fix: log state of 'disable pipeline wrappers' (#12345)
@owen-d owen-d mentioned this pull request Mar 28, 2024
Merged
rhnasc pushed a commit to inloco/loki that referenced this pull request Apr 12, 2024
@owen-d @rhnasc
feat(blooms): bloom integration in query planning (grafana#12208)
b104c1d 
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@loki-gh-app loki-gh-app bot mentioned this pull request Oct 18, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chaudum chaudum chaudum left review comments

@salvacorts salvacorts salvacorts approved these changes

Assignees
No one assigned
Labels
backport k195 size/XXL type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@owen-d @chaudum @salvacorts