mirror of
https://github.com/s3fs-fuse/s3fs-fuse.git
synced 2026-04-25 05:16:00 +03:00
[PR #1919] [MERGED] Fixed a bug about truncation for shrinking file #2255
Labels
No labels
bug
bug
dataloss
duplicate
enhancement
feature request
help wanted
invalid
need info
performance
pull-request
question
question
testing
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/s3fs-fuse#2255
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
📋 Pull Request Information
Original PR: https://github.com/s3fs-fuse/s3fs-fuse/pull/1919
Author: @ggtakec
Created: 2/27/2022
Status: ✅ Merged
Merged: 3/2/2022
Merged by: @gaul
Base:
master← Head:fix_truncate📝 Commits (1)
5f87deeFixed a bug about truncation for shrinking file📊 Changes
8 files changed (+109 additions, -22 deletions)
View changed files
📝
src/fdcache.cpp(+41 -7)📝
src/fdcache.h(+5 -2)📝
src/fdcache_auto.cpp(+2 -2)📝
src/fdcache_auto.h(+1 -1)📝
src/fdcache_page.cpp(+12 -2)📝
src/fdcache_page.h(+2 -1)📝
src/s3fs.cpp(+25 -7)📝
test/integration-test-main.sh(+21 -0)📄 Description
Relevant Issue (if applicable)
#1631 #1875 #1887
Details
Overview
Due to the bug(#1875) fix(#1887), there was a bug in truncate that shrinks the file.
(A bug occurred by fixing #1887 after fixing #1631.)
#1631 bypasses the resize-process if it shrinks the file being edited.
This assumes the case where the file are multiple open(multiple fd).
And with the fix of #1887, flush to the file is delayed when truncate is called.
Before these fixes, it was always flushed after truncation, so this hidden potential bug was not materialized.
Fix - 1
The modification of #1631(in
FdManager::Open) should be bypassed only if there is an open fd other than the fd opened by the truncate process.Therefore,
FdManager::Openshould only be bypassed if there are more than one file descriptor already open.(
FUSEalways preopens a file when truncate it, if it hasn't already been opened, so one fd is open when you call truncate.)So s3fs has been changed to check if more than one fd is open when truncate is called.
There are two way shown below to fix, but the former is adopted.
Add an argument to
FdManager::OpenThis changes needs to modify many lines, but I adopted in consideration of the effect of other processing.
I wanted to limit the scope of influence to only truncate.
Modify only inside
FdManager::OpenIt can be modified more easily than the former by using
if(2 <= ent->GetOpenCount() && ent->IsModified()){.However, I didn't use this method because the scope of influence was not clear.
Also, I think the former is better when supporting
FUSE3.[NOTE] Limitations on
FUSE2truncate does not pass an open file descriptor(
pseudo_fdfor s3fs) to the truncate function inFUSE2.Therefore, the following cannot be realized at present.(This will be a limitation.)
For example, suppose you have two files open.(Tentatively
fd1andfd2)One of them(
fd1) stretchs the file.In
fd2, it is does not change the file size.If
fd1then shrinks the file size(for example, restores it to the original file size for clarity), s3fs will still increase the file size.This is because it doesn't keep track of which file descriptor is trying to increase and shrink the file, and it can't be determined.
If it corresponds to
FUSE3, fd is passed infuse_file_info, so I think that it can be made into strict code.Fix - 2
In addition to the above
FdManager::Openands3fs_truncatemodifications, it was necessary to modify thePageListclass.This is a problem revealed by the fix of #1887(delayed execution of flush).
PageListhas the information of the opened file inside s3fs.PageListsets a flag internally when the file is modified or increased.(modify flag)But when the file was shrunk, it didn't have the information being edited.(Not needed before #1887)
Therefore, I am also modifying the PageList in case the file size is reduced.
Add truncate test
There was no simple file shrink test case.
Added a simple truncate test for file sizes that require multipart uploads.
🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.
std::unique_ptr#2420