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

Pass optional readers to override and cache metadata objects #249

Merged
merged 5 commits into from
Jun 30, 2022
Merged

Pass optional readers to override and cache metadata objects #249

merged 5 commits into from
Jun 30, 2022

Conversation

annanay25
Copy link
Contributor

There are two ways to design metadata caching -

  • Override the column chunk reader: We pass in the cached reader as the “main” reader with an override to use a raw backend reader for column chunk data. (PR Add support for caching parquet metadata #180)
  • Override the metadata reader: We pass in the backend reader as the “main” reader with an override to use a cached reader for metadata. (This PR)

Option 2 struct me (a lot) later than it should have, and this design works much better for us.

Signed-off-by: Annanay Agarwal annanay.agarwal@grafana.com

achille-roussel
achille-roussel reviewed Jun 23, 2022
View reviewed changes
config.go Outdated Show resolved Hide resolved
config.go Outdated Show resolved Hide resolved
file.go Outdated Show resolved Hide resolved
@annanay25
Copy link
Contributor Author

Just checking with you if the overall approach taken in this PR make sense?

@annanay25
Use interface cast-check to set metadata offsets in reader
441016c 
Signed-off-by: Annanay Agarwal <annanay.agarwal@grafana.com>
achille-roussel
achille-roussel reviewed Jun 27, 2022
View reviewed changes
Copy link
Contributor

@achille-roussel achille-roussel left a comment

Choose a reason for hiding this comment

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

This is looking great, I left a few suggestions, let me know what you think!

file.go Outdated Show resolved Hide resolved
file.go Show resolved Hide resolved
achille-roussel
achille-roussel reviewed Jun 27, 2022
View reviewed changes
bloom/filter.go Outdated Show resolved Hide resolved
bloom/filter.go Outdated Show resolved Hide resolved
annanay25 and others added 2 commits June 29, 2022 20:27
@annanay25
Do not set magic header offset, move bloom to OpenFile
8f30b2f 
Signed-off-by: Annanay Agarwal <annanay.agarwal@grafana.com>
@annanay25
remove unintentional newline
a5c0229
annanay25
annanay25 commented Jun 29, 2022
View reviewed changes
file.go Show resolved Hide resolved
achille-roussel
achille-roussel approved these changes Jun 30, 2022
View reviewed changes
Copy link
Contributor

@achille-roussel achille-roussel left a comment

Choose a reason for hiding this comment

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

Let's ship this!

@achille-roussel achille-roussel merged commit 7691e3e into segmentio:main Jun 30, 2022
@annanay25 annanay25 deleted the cache-metadata branch June 30, 2022 17:54
@annanay25 annanay25 mentioned this pull request Jun 30, 2022
Closed
@achille-roussel achille-roussel added the enhancement Improve a feature that already exists label Jul 11, 2022
@fpetkovski
Copy link

Can this functionality be used to pre-fetch metadata in one shot instead of doing it with smaller reads?

I am trying to optimize opening a file from object storage, and now each page index leads to a single 4KB read.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@achille-roussel achille-roussel achille-roussel approved these changes

Assignees
No one assigned
Labels
enhancement Improve a feature that already exists
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@annanay25 @fpetkovski @achille-roussel