-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
rocksdb: correctly handle FS that does not support hard links #40875
Conversation
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, though our docs on how to do RocksDB changes are out of date. Please open a PR on cockroachdb/rocksdb
with a target branch of crl-release-6.2.1
. We'll review the RocksDB change there, then get to rubber stamp this PR.
PS You'll need a Release justification
stanza as well.
@petermattis I talked to @darinpp and we couldn't think of a problem with following the procedure in the out-of-date README.md. My guess is the drawback is we can't comment on individual lines of code, but in cases where the change is small and we don't need to do that, can following either procedure be ok? |
Well, the big thing is that we want is for |
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.
Right that is step 7, after we LGTM here. OK then, I'll accept.
Ingesting a file from a file system that doesn't support hard links or from a network location, returns an error that wasn't handled correctly and the result was IO error instead of Not Supported status. Fixes cockroachdb#38789 Release justification: Category 2: Bug fixes and low-risk updates to new functionality. Release note (bug fix): Fixes an issue with creating table indexes when the server is running on Windows and the store is on a FS that does not support hard links (ex FAT32 or network share)
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 again, the commit matches head of crl-release-6.2.1 now.
bors r+ |
40875: rocksdb: correctly handle FS that does not support hard links r=darinpp a=darinpp Ingesting a file from a file system that doesn't support hard links or from a network location, returns an error that wasn't handled correctly and the result was IO error instead of Not Supported status. Fixes #38789 Release justification: Category 2: Bug fixes and low-risk updates to new functionality. Release note (bug fix): Fixes an issue with creating table indexes when the server is running on Windows and the store is on a FS that does not support hard links (ex FAT32 or network share) Co-authored-by: Darin <darinp@gmail.com>
thx for your help with this @darinpp |
Build succeeded |
Ingesting a file from a file system that doesn't
support hard links or from a network location, returns
an error that wasn't handled correctly and the result was
IO error instead of Not Supported status.
Fixes #38789
Release justification: Category 2: Bug fixes and low-risk updates to new functionality.
Release note (bug fix): Fixes an issue with creating table indexes
when the server is running on Windows and the store is on a FS
that does not support hard links (ex FAT32 or network share)