feat: cache generation and sequence to reduce TSM filename parsing#26905
Merged
davidby-influx merged 8 commits intomaster-1.xfrom Oct 16, 2025
Merged
feat: cache generation and sequence to reduce TSM filename parsing#26905davidby-influx merged 8 commits intomaster-1.xfrom
davidby-influx merged 8 commits intomaster-1.xfrom
Conversation
Add `FileStore.SupportsCompactionPlanning()` which allows compaction planners to check if a `FileStore` supports compaction planning. Currently this means the `FileStore` must have a TSM filename parsing function available.
gwossum
reviewed
Oct 10, 2025
tsdb/engine/tsm1/reader.go
Outdated
| } | ||
| if t.parseFileNameFunc != nil { | ||
| if t.generation, t.sequence, err = t.parseFileNameFunc(path); err != nil { | ||
| return fmt.Errorf("failed parsing filename %q for generation and sequence numbers: %w", path, err) |
Member
There was a problem hiding this comment.
This error adds context that the same error in NewTSMReader does not. I think the context here is redundant because the error from DefaultParseFileName (the only one we use) includes the filename.
I would remove the name from this context, but add that the parsing error occurred during TSMReader.Rename.
gwossum
reviewed
Oct 10, 2025
tsdb/engine/tsm1/reader.go
Outdated
| // that does not involve compaction planning, and was not created by the FileStore | ||
| t.generation, t.sequence, err = t.parseFileNameFunc(t.Path()) | ||
| if err != nil { | ||
| return nil, err |
Member
There was a problem hiding this comment.
It might be a good idea to add context that the parse filename error occurred during the NewTSMReader call.
gwossum
reviewed
Oct 10, 2025
tsdb/engine/tsm1/reader.go
Outdated
Comment on lines
+447
to
+451
| if t.parseFileNameFunc != nil { | ||
| if t.generation, t.sequence, err = t.parseFileNameFunc(path); err != nil { | ||
| return fmt.Errorf("failed parsing filename %q for generation and sequence numbers: %w", path, err) | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
We may even want to refactor this chunk of code into
gwossum
reviewed
Oct 10, 2025
tsdb/engine/tsm1/reader.go
Outdated
Comment on lines
+276
to
+283
| if nil != t.parseFileNameFunc { | ||
| // If parseFileNameFunc is nil, we are in a test or another TSMReader use | ||
| // that does not involve compaction planning, and was not created by the FileStore | ||
| t.generation, t.sequence, err = t.parseFileNameFunc(t.Path()) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
We may want to refactor this chunk of code into it's own method, something like TSMReader.parseFileName. We repeat the same basic code twice and it might be nice to have a "safe" wrapper around parseFileNameFunc.
This was referenced Oct 14, 2025
devanbenz
added a commit
that referenced
this pull request
Feb 5, 2026
…26905) (#27189) Parse TSM file names only on creation or rename and cache the generation and level, instead of parsing them for those values repeatedly Fixes #26794 --------- (cherry picked from commit 8b1696a) Co-authored-by: davidby-influx <72418212+davidby-influx@users.noreply.github.com> Co-authored-by: Geoffrey Wossum <gwossum@influxdata.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Parsing TSM filenames for generation and level is a large CPU
cost in the compaction planner. Because these file names do
not change except through an explicit TSMReader.Rename()
call, we can cache generation and level on creation and
rename to avoid CPU load.
Fixes #26794