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
Improve safety when deleting files from the store #9801
Conversation
38c0538
to
0811537
Compare
I updated this to fix other places as well where we need the write lock and added more safety. @mikemccand @rmuir can you take a look please |
+1 |
} | ||
try { | ||
dir.deleteFile(reason, existingFile); | ||
} catch (FileNotFoundException | NoSuchFileException ex) { |
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.
Hmm if we have the write lock on this dir, why would we hit FNFE/NSFE?
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.
good question - I will remove
Left a couple comments, otherwise LGTM! |
0811537
to
451c9d8
Compare
Today if we delete files from the index directory we never acquire the write lock. Yet, for safety reasons we should get the write lock before we modify / delete any files. Where we can we should leave the deletion to the index writer and only delete that are necessary to delete ourself.
451c9d8
to
c54bd2f
Compare
Today if we delete files from the index directory we never acquire the
write lock. Yet, for safety reasons we should get the write lock before
we modify / delete any files. Where we can we should leave the deletion
to the index writer and only delete that are necessary to delete ourself.