Skip to content
This repository has been archived by the owner on Jul 19, 2023. It is now read-only.

Improve objstore client for parquet Reading. #181

Merged
merged 9 commits into from
Sep 1, 2022
Merged

Conversation

cyriltovena
Copy link
Collaborator

This PR does fews things to improve the objstore client:

  • On filesystem avoid to Open/Close/Stat a file for each ReadRange
  • Instrument all clients
  • Introduce a benchmark to verify the performance boost.

There's still probably a lot that we can do to ( cache footer and metadata informations) but for now this seems like a good step forward.

❯ go test -benchmem -run=^$  -bench ^BenchmarkDBSelectProfile$ github.com/grafana/fire/pkg/firedb -v -count=5 -timeout=0s -memprofile=mem.out -cpuprofile=cpu.out > after.txt && benchstat before.txt after.txt
name                old time/op    new time/op    delta
DBSelectProfile-16     123ms ± 4%      62ms ± 2%  -49.64%  (p=0.029 n=4+4)

name                old alloc/op   new alloc/op   delta
DBSelectProfile-16     114MB ± 0%     100MB ± 0%  -11.82%  (p=0.008 n=5+5)

name                old allocs/op  new allocs/op  delta
DBSelectProfile-16      478k ± 0%      281k ± 0%  -41.17%  (p=0.008 n=5+5)

Copy link
Collaborator

@simonswine simonswine left a comment

Choose a reason for hiding this comment

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

LGTM, see a couple of comments, also in need of rebasing

pkg/firedb/head_test.go Outdated Show resolved Hide resolved
pkg/objstore/providers/filesystem/filesystem.go Outdated Show resolved Hide resolved
ReadererAt
}

type ReadererAt interface {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a big fan of this name. Maybe we PersistentReaderAt or ReaderAtCreator is better

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thinking about it.

@cyriltovena cyriltovena merged commit 981e857 into main Sep 1, 2022
@cyriltovena cyriltovena deleted the better-objstore branch September 1, 2022 12:40
simonswine pushed a commit to simonswine/pyroscope that referenced this pull request Jun 30, 2023
Improve objstore client for parquet Reading.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants