Make FsBlobContainer Listing Resilient to Concurrent Modifications#49142
Make FsBlobContainer Listing Resilient to Concurrent Modifications#49142original-brownbear merged 3 commits intoelastic:masterfrom original-brownbear:fix-concurrency-fs-container-list
Conversation
If we list out files in a folder via the lazily computed directory stream, we have to deal with concurrent deletes when reading the file attributes since we don't have a lock on the directory in any way. Closes #37581
|
Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore) |
|
Jenkins run elasticsearch-ci/1 |
| final BasicFileAttributes attrs; | ||
| try { | ||
| attrs = Files.readAttributes(file, BasicFileAttributes.class); | ||
| } catch (FileNotFoundException e) { |
There was a problem hiding this comment.
This might need to be NoSuchFileException | FileNotFoundException.
There was a problem hiding this comment.
Right! :)
| try { | ||
| attrs = Files.readAttributes(file, BasicFileAttributes.class); | ||
| } catch (FileNotFoundException e) { | ||
| continue; |
There was a problem hiding this comment.
can you add a comment here saying why it's ok to ignore?
| } catch (FileNotFoundException e) { | ||
| continue; | ||
| } | ||
| if (attrs.isRegularFile()) { |
There was a problem hiding this comment.
perhaps easier as the two suggestions above is to use Files.isRegularFile(file) here which takes care of all this.
There was a problem hiding this comment.
This won't help us I think? We still want the file size two lines below where we'd have to make another call to the FS that we'd have to guard against "file not found".
original-brownbear
left a comment
There was a problem hiding this comment.
All addressed I think :)
| } catch (FileNotFoundException e) { | ||
| continue; | ||
| } | ||
| if (attrs.isRegularFile()) { |
There was a problem hiding this comment.
This won't help us I think? We still want the file size two lines below where we'd have to make another call to the FS that we'd have to guard against "file not found".
| try { | ||
| attrs = Files.readAttributes(file, BasicFileAttributes.class); | ||
| } catch (FileNotFoundException e) { | ||
| continue; |
| final BasicFileAttributes attrs; | ||
| try { | ||
| attrs = Files.readAttributes(file, BasicFileAttributes.class); | ||
| } catch (FileNotFoundException e) { |
There was a problem hiding this comment.
Right! :)
|
Thanks Yannick! |
…49142) (#49176) * Make FsBlobContainer Listing Resilient to Concurrent Modifications If we list out files in a folder via the lazily computed directory stream, we have to deal with concurrent deletes when reading the file attributes since we don't have a lock on the directory in any way. Closes #37581
…49142) (#49177) * Make FsBlobContainer Listing Resilient to Concurrent Modifications If we list out files in a folder via the lazily computed directory stream, we have to deal with concurrent deletes when reading the file attributes since we don't have a lock on the directory in any way. Closes #37581
If we list out files in a folder via the lazily computed directory
stream, we have to deal with concurrent deletes when reading the file
attributes since we don't have a lock on the directory in any way.
Closes #37581