[PR #1919] [MERGED] Fixed a bug about truncation for shrinking file #2255

Closed
opened 2026-03-04 02:04:34 +03:00 by kerem · 0 comments
Owner

📋 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: masterHead: fix_truncate


📝 Commits (1)

  • 5f87dee Fixed 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::Open should only be bypassed if there are more than one file descriptor already open.
(FUSE always 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::Open

This 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::Open

It 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 FUSE2

truncate does not pass an open file descriptor(pseudo_fd for s3fs) to the truncate function in FUSE2.
Therefore, the following cannot be realized at present.(This will be a limitation.)

For example, suppose you have two files open.(Tentatively fd1 and fd2)
One of them(fd1) stretchs the file.
In fd2, it is does not change the file size.
If fd1 then 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 in fuse_file_info, so I think that it can be made into strict code.

Fix - 2

In addition to the above FdManager::Open and s3fs_truncate modifications, it was necessary to modify the PageList class.
This is a problem revealed by the fix of #1887(delayed execution of flush).

PageList has the information of the opened file inside s3fs.
PageList sets 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.

## 📋 Pull Request Information **Original PR:** https://github.com/s3fs-fuse/s3fs-fuse/pull/1919 **Author:** [@ggtakec](https://github.com/ggtakec) **Created:** 2/27/2022 **Status:** ✅ Merged **Merged:** 3/2/2022 **Merged by:** [@gaul](https://github.com/gaul) **Base:** `master` ← **Head:** `fix_truncate` --- ### 📝 Commits (1) - [`5f87dee`](https://github.com/s3fs-fuse/s3fs-fuse/commit/5f87deecb51ed6690d32ce22a709f9f4a0096449) Fixed a bug about truncation for shrinking file ### 📊 Changes **8 files changed** (+109 additions, -22 deletions) <details> <summary>View changed files</summary> 📝 `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) </details> ### 📄 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::Open` should only be bypassed if there are more than one file descriptor already open. _(`FUSE` always 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::Open` This 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::Open` It 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 `FUSE2` truncate does not pass an open file descriptor(`pseudo_fd` for s3fs) to the truncate function in `FUSE2`. Therefore, the following cannot be realized at present.(This will be a limitation.) For example, suppose you have two files open.(Tentatively `fd1` and `fd2`) One of them(`fd1`) stretchs the file. In `fd2`, it is does not change the file size. If `fd1` then 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 in `fuse_file_info`, so I think that it can be made into strict code. #### Fix - 2 In addition to the above `FdManager::Open` and `s3fs_truncate` modifications, it was necessary to modify the `PageList` class. This is a problem revealed by the fix of #1887(delayed execution of flush). `PageList` has the information of the opened file inside s3fs. `PageList` sets 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
kerem 2026-03-04 02:04:34 +03:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/s3fs-fuse#2255
No description provided.