Fixed a bug in fdatasync(fsync)

This commit is contained in:
Takeshi Nakatani 2024-01-24 14:11:59 +00:00 committed by Andrew Gaul
parent 5e6f21a9ff
commit 517574c40c
4 changed files with 55 additions and 22 deletions

View File

@ -907,6 +907,13 @@ bool FdEntity::UpdateMtime(bool clear_holding_mtime)
if(!ClearHoldingMtime(AutoLock::ALREADY_LOCKED)){ if(!ClearHoldingMtime(AutoLock::ALREADY_LOCKED)){
return false; return false;
} }
// [NOTE]
// If come here after fdatasync has been processed, the file
// content update has already taken place. However, the metadata
// update is necessary and needs to be flagged in order to
// perform it with flush,
//
pending_status = pending_status_t::UPDATE_META_PENDING;
} }
}else{ }else{
struct stat st; struct stat st;
@ -1426,19 +1433,23 @@ int FdEntity::RowFlush(int fd, const char* tpath, AutoLock::Type type, bool forc
AutoLock auto_lock2(&fdent_data_lock); AutoLock auto_lock2(&fdent_data_lock);
if(!force_sync && !pagelist.IsModified()){ int result;
if(!force_sync && !pagelist.IsModified() && !IsDirtyMetadata()){
// nothing to update. // nothing to update.
return 0; return 0;
} }
if(S3fsLog::IsS3fsLogDbg()){ if(S3fsLog::IsS3fsLogDbg()){
pagelist.Dump(); pagelist.Dump();
} }
int result;
if(nomultipart){ if(nomultipart){
// No multipart upload // No multipart upload
if(!force_sync && !pagelist.IsModified()){
// for only push pending headers
result = UploadPending(-1, AutoLock::ALREADY_LOCKED);
}else{
result = RowFlushNoMultipart(pseudo_obj, tpath); result = RowFlushNoMultipart(pseudo_obj, tpath);
}
}else if(FdEntity::streamupload){ }else if(FdEntity::streamupload){
// Stream multipart upload // Stream multipart upload
result = RowFlushStreamMultipart(pseudo_obj, tpath); result = RowFlushStreamMultipart(pseudo_obj, tpath);
@ -1523,6 +1534,7 @@ int FdEntity::RowFlushNoMultipart(const PseudoFdInfo* pseudo_obj, const char* tp
if(0 == result){ if(0 == result){
pagelist.ClearAllModified(); pagelist.ClearAllModified();
} }
return result; return result;
} }
@ -2573,6 +2585,29 @@ bool FdEntity::IsDirtyNewFile() const
return (pending_status_t::CREATE_FILE_PENDING == pending_status); return (pending_status_t::CREATE_FILE_PENDING == pending_status);
} }
// [NOTE]
// The fdatasync call only uploads the content but does not update
// the meta data. In the flush call, if there is no update contents,
// need to upload only metadata, so use these functions.
//
void FdEntity::MarkDirtyMetadata()
{
AutoLock auto_lock(&fdent_lock);
AutoLock auto_lock2(&fdent_data_lock);
if(pending_status_t::NO_UPDATE_PENDING == pending_status){
pending_status = pending_status_t::UPDATE_META_PENDING;
}
}
bool FdEntity::IsDirtyMetadata() const
{
// [NOTE]
// fdent_lock must be previously locked.
//
return (pending_status_t::UPDATE_META_PENDING == pending_status);
}
bool FdEntity::AddUntreated(off_t start, off_t size) bool FdEntity::AddUntreated(off_t start, off_t size)
{ {
bool result = untreated_list.AddPart(start, size); bool result = untreated_list.AddPart(start, size);

View File

@ -97,6 +97,8 @@ class FdEntity
bool AddUntreated(off_t start, off_t size); bool AddUntreated(off_t start, off_t size);
bool IsDirtyMetadata() const;
public: public:
static bool GetNoMixMultipart() { return mixmultipart; } static bool GetNoMixMultipart() { return mixmultipart; }
static bool SetNoMixMultipart(); static bool SetNoMixMultipart();
@ -156,6 +158,7 @@ class FdEntity
void MarkDirtyNewFile(); void MarkDirtyNewFile();
bool IsDirtyNewFile() const; bool IsDirtyNewFile() const;
void MarkDirtyMetadata();
bool GetLastUpdateUntreatedPart(off_t& start, off_t& size) const; bool GetLastUpdateUntreatedPart(off_t& start, off_t& size) const;
bool ReplaceLastUpdateUntreatedPart(off_t front_start, off_t front_size, off_t behind_start, off_t behind_size); bool ReplaceLastUpdateUntreatedPart(off_t front_start, off_t front_size, off_t behind_start, off_t behind_size);

View File

@ -3047,7 +3047,7 @@ static int s3fs_fsync(const char* _path, int datasync, struct fuse_file_info* fi
WTF8_ENCODE(path) WTF8_ENCODE(path)
int result = 0; int result = 0;
FUSE_CTX_INFO("[path=%s][pseudo_fd=%llu]", path, (unsigned long long)(fi->fh)); FUSE_CTX_INFO("[path=%s][datasync=%d][pseudo_fd=%llu]", path, datasync, (unsigned long long)(fi->fh));
AutoFdEntity autoent; AutoFdEntity autoent;
FdEntity* ent; FdEntity* ent;
@ -3060,6 +3060,16 @@ static int s3fs_fsync(const char* _path, int datasync, struct fuse_file_info* fi
} }
result = ent->Flush(static_cast<int>(fi->fh), AutoLock::NONE, false); result = ent->Flush(static_cast<int>(fi->fh), AutoLock::NONE, false);
if(0 != datasync){
// [NOTE]
// The metadata are not updated when fdatasync is called.
// Instead of it, these metadata are pended and set the dirty flag here.
// Setting this flag allows metadata to be updated even if there is no
// content update between the fdatasync call and the flush call.
//
ent->MarkDirtyMetadata();
}
if(is_new_file){ if(is_new_file){
// update parent directory timestamp // update parent directory timestamp
int update_result; int update_result;

View File

@ -408,8 +408,6 @@ function test_external_modification {
local OBJECT_NAME; OBJECT_NAME=$(basename "${PWD}")/"${TEST_TEXT_FILE}" local OBJECT_NAME; OBJECT_NAME=$(basename "${PWD}")/"${TEST_TEXT_FILE}"
echo "new new" | aws_cli s3 cp - "s3://${TEST_BUCKET_1}/${OBJECT_NAME}" echo "new new" | aws_cli s3 cp - "s3://${TEST_BUCKET_1}/${OBJECT_NAME}"
wait_ostype 1 "Darwin"
cmp "${TEST_TEXT_FILE}" <(echo "new new") cmp "${TEST_TEXT_FILE}" <(echo "new new")
rm -f "${TEST_TEXT_FILE}" rm -f "${TEST_TEXT_FILE}"
} }
@ -884,7 +882,6 @@ function test_mtime_file {
#copy the test file with preserve mode #copy the test file with preserve mode
cp -p "${TEST_TEXT_FILE}" "${ALT_TEST_TEXT_FILE}" cp -p "${TEST_TEXT_FILE}" "${ALT_TEST_TEXT_FILE}"
wait_ostype 1 "Darwin"
local testmtime; testmtime=$(get_mtime "${TEST_TEXT_FILE}") local testmtime; testmtime=$(get_mtime "${TEST_TEXT_FILE}")
local testctime; testctime=$(get_ctime "${TEST_TEXT_FILE}") local testctime; testctime=$(get_ctime "${TEST_TEXT_FILE}")
@ -953,7 +950,6 @@ function test_update_time_chown() {
local base_atime; base_atime=$(get_atime "${TEST_TEXT_FILE}") local base_atime; base_atime=$(get_atime "${TEST_TEXT_FILE}")
local base_ctime; base_ctime=$(get_ctime "${TEST_TEXT_FILE}") local base_ctime; base_ctime=$(get_ctime "${TEST_TEXT_FILE}")
local base_mtime; base_mtime=$(get_mtime "${TEST_TEXT_FILE}") local base_mtime; base_mtime=$(get_mtime "${TEST_TEXT_FILE}")
wait_ostype 1 "Darwin"
# [NOTE] # [NOTE]
# In this test, chown is called with the same UID. # In this test, chown is called with the same UID.
@ -1050,7 +1046,6 @@ function test_update_time_touch_a() {
# "touch -a" -> update ctime/atime, not update mtime # "touch -a" -> update ctime/atime, not update mtime
# #
touch -a "${TEST_TEXT_FILE}" touch -a "${TEST_TEXT_FILE}"
wait_ostype 1 "Darwin"
local atime; atime=$(get_atime "${TEST_TEXT_FILE}") local atime; atime=$(get_atime "${TEST_TEXT_FILE}")
local ctime; ctime=$(get_ctime "${TEST_TEXT_FILE}") local ctime; ctime=$(get_ctime "${TEST_TEXT_FILE}")
@ -1119,6 +1114,8 @@ function test_update_time_cp_p() {
echo "cp with -p option expected updated ctime: $base_ctime != $ctime and same mtime: $base_mtime == $mtime, atime: $base_atime == $atime" echo "cp with -p option expected updated ctime: $base_ctime != $ctime and same mtime: $base_mtime == $mtime, atime: $base_atime == $atime"
return 1 return 1
fi fi
rm_test_file
rm_test_file "${TIME_TEST_TEXT_FILE}"
} }
function test_update_time_mv() { function test_update_time_mv() {
@ -1425,7 +1422,6 @@ function test_update_parent_directory_time_sub() {
local base_atime; base_atime=$(get_atime "${TEST_PARENTDIR_PARENT}") local base_atime; base_atime=$(get_atime "${TEST_PARENTDIR_PARENT}")
local base_ctime; base_ctime=$(get_ctime "${TEST_PARENTDIR_PARENT}") local base_ctime; base_ctime=$(get_ctime "${TEST_PARENTDIR_PARENT}")
local base_mtime; base_mtime=$(get_mtime "${TEST_PARENTDIR_PARENT}") local base_mtime; base_mtime=$(get_mtime "${TEST_PARENTDIR_PARENT}")
wait_ostype 1 "Darwin"
touch "${TEST_PARENTDIR_FILE}" touch "${TEST_PARENTDIR_FILE}"
@ -1444,7 +1440,6 @@ function test_update_parent_directory_time_sub() {
base_atime="${after_atime}" base_atime="${after_atime}"
base_ctime="${after_ctime}" base_ctime="${after_ctime}"
base_mtime="${after_mtime}" base_mtime="${after_mtime}"
wait_ostype 1 "Darwin"
touch "${TEST_PARENTDIR_FILE}" touch "${TEST_PARENTDIR_FILE}"
@ -1463,7 +1458,6 @@ function test_update_parent_directory_time_sub() {
base_atime="${after_atime}" base_atime="${after_atime}"
base_ctime="${after_ctime}" base_ctime="${after_ctime}"
base_mtime="${after_mtime}" base_mtime="${after_mtime}"
wait_ostype 1 "Darwin"
mv "${TEST_PARENTDIR_FILE}" "${TEST_PARENTDIR_FILE_MV}" mv "${TEST_PARENTDIR_FILE}" "${TEST_PARENTDIR_FILE_MV}"
@ -1482,7 +1476,6 @@ function test_update_parent_directory_time_sub() {
base_atime="${after_atime}" base_atime="${after_atime}"
base_ctime="${after_ctime}" base_ctime="${after_ctime}"
base_mtime="${after_mtime}" base_mtime="${after_mtime}"
wait_ostype 1 "Darwin"
ln -s "${TEST_PARENTDIR_SYMFILE_BASE}" "${TEST_PARENTDIR_SYMFILE}" ln -s "${TEST_PARENTDIR_SYMFILE_BASE}" "${TEST_PARENTDIR_SYMFILE}"
@ -1501,7 +1494,6 @@ function test_update_parent_directory_time_sub() {
base_atime="${after_atime}" base_atime="${after_atime}"
base_ctime="${after_ctime}" base_ctime="${after_ctime}"
base_mtime="${after_mtime}" base_mtime="${after_mtime}"
wait_ostype 1 "Darwin"
touch "${TEST_PARENTDIR_SYMFILE}" touch "${TEST_PARENTDIR_SYMFILE}"
@ -1520,7 +1512,6 @@ function test_update_parent_directory_time_sub() {
base_atime="${after_atime}" base_atime="${after_atime}"
base_ctime="${after_ctime}" base_ctime="${after_ctime}"
base_mtime="${after_mtime}" base_mtime="${after_mtime}"
wait_ostype 1 "Darwin"
mv "${TEST_PARENTDIR_SYMFILE}" "${TEST_PARENTDIR_SYMFILE_MV}" mv "${TEST_PARENTDIR_SYMFILE}" "${TEST_PARENTDIR_SYMFILE_MV}"
@ -1539,7 +1530,6 @@ function test_update_parent_directory_time_sub() {
base_atime="${after_atime}" base_atime="${after_atime}"
base_ctime="${after_ctime}" base_ctime="${after_ctime}"
base_mtime="${after_mtime}" base_mtime="${after_mtime}"
wait_ostype 1 "Darwin"
rm "${TEST_PARENTDIR_SYMFILE_MV}" rm "${TEST_PARENTDIR_SYMFILE_MV}"
@ -1558,7 +1548,6 @@ function test_update_parent_directory_time_sub() {
base_atime="${after_atime}" base_atime="${after_atime}"
base_ctime="${after_ctime}" base_ctime="${after_ctime}"
base_mtime="${after_mtime}" base_mtime="${after_mtime}"
wait_ostype 1 "Darwin"
rm "${TEST_PARENTDIR_FILE_MV}" rm "${TEST_PARENTDIR_FILE_MV}"
@ -1577,7 +1566,6 @@ function test_update_parent_directory_time_sub() {
local base_atime; base_atime=$(get_atime "${TEST_PARENTDIR_PARENT}") local base_atime; base_atime=$(get_atime "${TEST_PARENTDIR_PARENT}")
local base_ctime; base_ctime=$(get_ctime "${TEST_PARENTDIR_PARENT}") local base_ctime; base_ctime=$(get_ctime "${TEST_PARENTDIR_PARENT}")
local base_mtime; base_mtime=$(get_mtime "${TEST_PARENTDIR_PARENT}") local base_mtime; base_mtime=$(get_mtime "${TEST_PARENTDIR_PARENT}")
wait_ostype 1 "Darwin"
mkdir "${TEST_PARENTDIR_DIR}" mkdir "${TEST_PARENTDIR_DIR}"
@ -1596,7 +1584,6 @@ function test_update_parent_directory_time_sub() {
base_atime="${after_atime}" base_atime="${after_atime}"
base_ctime="${after_ctime}" base_ctime="${after_ctime}"
base_mtime="${after_mtime}" base_mtime="${after_mtime}"
wait_ostype 1 "Darwin"
touch "${TEST_PARENTDIR_DIR}" touch "${TEST_PARENTDIR_DIR}"
@ -1615,7 +1602,6 @@ function test_update_parent_directory_time_sub() {
base_atime="${after_atime}" base_atime="${after_atime}"
base_ctime="${after_ctime}" base_ctime="${after_ctime}"
base_mtime="${after_mtime}" base_mtime="${after_mtime}"
wait_ostype 1 "Darwin"
mv "${TEST_PARENTDIR_DIR}" "${TEST_PARENTDIR_DIR_MV}" mv "${TEST_PARENTDIR_DIR}" "${TEST_PARENTDIR_DIR_MV}"
@ -1634,7 +1620,6 @@ function test_update_parent_directory_time_sub() {
base_atime="${after_atime}" base_atime="${after_atime}"
base_ctime="${after_ctime}" base_ctime="${after_ctime}"
base_mtime="${after_mtime}" base_mtime="${after_mtime}"
wait_ostype 1 "Darwin"
rm -r "${TEST_PARENTDIR_DIR_MV}" rm -r "${TEST_PARENTDIR_DIR_MV}"