-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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.
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
More details for 'tail prefetch size is calculated based on' #12667
base: main
Are you sure you want to change the base?
Conversation
@hx235 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a warning so we can track use cases that are at risk of regression when we finally remove TailPrefetchStats. The warnings and the risk can be mitigated by running compaction with a new enough RocksDB version (see #12664). If you still wanted to make a code change here, may I suggest a couple ideas:
- As you mentioned the documentation was inadequate to know what to do. Maybe the warning text or surrounding comments could be improved.
- Deduplicating the error could help with the spam problem. Maybe one warning per DB open saying N files are at risk.
@andlr has updated the pull request. You must reimport the pull request before landing. |
Thanks for the explanation. I think in such case, it's probably enough to add name of the SST file into the log message, and a comment in code, saying that this backward compatibility won't be preserved forever |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
@hx235 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@andlr has updated the pull request. You must reimport the pull request before landing. |
@hx235 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@andlr has updated the pull request. You must reimport the pull request before landing. |
@andlr has updated the pull request. You must reimport the pull request before landing. |
These messages indicate that SST file was created by a pre-9.0.0 RocksDB. Eventually,
TailPrefetchStats
might be removed, so it would be more informative if log message also included name of the affected SST file.Issue: #12664