[GODRIVER-3104] Fix Bucket.Delete/DeleteContext returning ErrFileNotFound when chunks exist Created: 24/Jan/24 Updated: 07/Feb/24 |
|
| Status: | Waiting for Reporter |
| Project: | Go Driver |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Minor - P4 |
| Reporter: | Ed Pelc | Assignee: | Preston Vasquez |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Assigned Teams: |
Go Drivers
|
| Description |
ContextBucket.Delete and DeleteContext return an ErrFileNotFound error when the file document does not exist but chunks may have been deleted.
It should check the chunk error and only return ErrFileNotFound if no file or chunk documents were found.
Why do this? After a failed upload you can have chunks with no file document in mongodb using gridfs. The Delete methods should be able to clean this up without erroring. Also returning an error when Delete did in fact delete/fully cleanup a file doesn't make any sense. Definition of doneWhat must be done to consider the task complete?
A gridfs upload can fail and leave chunks in the chunks collection without a corresponding file document. If you call Delete or DeleteContext using the file id of these chunks then an error is returned that the file doesn't exist. It should return nil instead when these existing chunks are cleaned up. ErrFileNotFound should only be returned if both the file document and chunks do not exist. PitfallsExisting code could potentially be relying on this and require a version bump. However I believe it's unlikely as most code probably ignores the error from delete when used for cleanup after a failed file upload.
See https://github.com/mongodb/mongo-go-driver/blob/v1/mongo/gridfs/bucket.go#L277
The chunks error is ignored. This should be checked and if nil/deleted chunk documents and the file error is ErrFileNotFound then no error should be returned. |
| Comments |
| Comment by PM Bot [ 07/Feb/24 ] | ||||||
|
Hi epelc@greatcloak.com! GODRIVER-3104 is awaiting your response. If this is still an issue for you, please open Jira to review the latest status and provide your feedback. Thanks! | ||||||
| Comment by Preston Vasquez [ 31/Jan/24 ] | ||||||
|
Hey epelc@greatcloak.com, thanks for reaching out. ErrFileNotFound serves as a sentinel (io.EOF, for example) that signals to the user that a gridfs operation occurred without corresponding files. The occurrence of this SE indicates a potential inconsistency in the data storage, i.e. the absence of corresponding files might suggest a problem in the data structure or storage process (like losing a connection mid upsert). At the very least, this may be something a user needs to log. For this reason, removing the error in v1 would be backwards breaking, as you note.
| ||||||
| Comment by Ed Pelc [ 24/Jan/24 ] | ||||||
|
Sorry for the poor formatting. I submitted before finishing editing. "A gridfs upload can fail and leave chunks in the chunks collection without a corresponding file document. If you call Delete or DeleteContext using the file id of these chunks then an error is returned that the file doesn't exist. It should return nil instead when these existing chunks are cleaned up. ErrFileNotFound should only be returned if both the file document and chunks do not exist." should be at the top and is the main synopsis. It seems Jira doesn't let you edit the description. | ||||||
| Comment by PM Bot [ 24/Jan/24 ] | ||||||
|
Hi epelc@greatcloak.com, thank you for reporting this issue! The team will look into it and get back to you soon. |