From f9d3941d9d24871825c3eed03bc34e35bdfd5302 Mon Sep 17 00:00:00 2001 From: Takeshi Nakatani Date: Sun, 15 Oct 2023 04:01:14 +0000 Subject: [PATCH] Fixed a bug in the re-upload part of Streamupload --- src/fdcache_entity.cpp | 25 +++++++++++++++++++++++-- src/fdcache_fdinfo.cpp | 11 +++++++++-- src/fdcache_fdinfo.h | 4 ++-- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/fdcache_entity.cpp b/src/fdcache_entity.cpp index 9dcf4fd15d..1de87262b8 100644 --- a/src/fdcache_entity.cpp +++ b/src/fdcache_entity.cpp @@ -1737,7 +1737,7 @@ int FdEntity::RowFlushMixMultipart(PseudoFdInfo* pseudo_obj, const char* tpath) return result; } untreated_list.ClearParts(untreated_start, untreated_size); - } + } // complete multipart uploading. if(0 != (result = NoCacheCompleteMultipartPost(pseudo_obj))){ S3FS_PRN_ERR("failed to complete(finish) multipart post for file(physical_fd=%d).", physical_fd); @@ -1812,7 +1812,8 @@ int FdEntity::RowFlushStreamMultipart(PseudoFdInfo* pseudo_obj, const char* tpat mp_part_list_t to_copy_list; mp_part_list_t to_download_list; filepart_list_t cancel_uploaded_list; - if(!pseudo_obj->ExtractUploadPartsFromAllArea(untreated_list, to_upload_list, to_copy_list, to_download_list, cancel_uploaded_list, S3fsCurl::GetMultipartSize(), pagelist.Size(), FdEntity::mixmultipart)){ + bool wait_upload_complete = false; + if(!pseudo_obj->ExtractUploadPartsFromAllArea(untreated_list, to_upload_list, to_copy_list, to_download_list, cancel_uploaded_list, wait_upload_complete, S3fsCurl::GetMultipartSize(), pagelist.Size(), FdEntity::mixmultipart)){ S3FS_PRN_ERR("Failed to extract various upload parts list from all area: errno(EIO)"); return -EIO; } @@ -1888,6 +1889,26 @@ int FdEntity::RowFlushStreamMultipart(PseudoFdInfo* pseudo_obj, const char* tpat } } + // [NOTE] + // If there is a part where has already been uploading, that part + // is re-updated after finishing uploading, so the part of the last + // uploded must be canceled. + // (These are cancel_uploaded_list, cancellation processing means + // re-uploading the same area.) + // + // In rare cases, the completion of the previous upload and the + // re-upload may be reversed, causing the ETag to be reversed, + // in which case the upload will fail. + // To prevent this, if the upload of the same area as the re-upload + // is incomplete, we must wait for it to complete here. + // + if(wait_upload_complete){ + if(0 != (result = pseudo_obj->WaitAllThreadsExit())){ + S3FS_PRN_ERR("Some cancel area uploads that were waiting to complete failed with %d.", result); + return result; + } + } + // // Upload multipart and copy parts and wait exiting them // diff --git a/src/fdcache_fdinfo.cpp b/src/fdcache_fdinfo.cpp index a1715b4093..9886241d1a 100644 --- a/src/fdcache_fdinfo.cpp +++ b/src/fdcache_fdinfo.cpp @@ -763,7 +763,8 @@ bool PseudoFdInfo::ExtractUploadPartsFromUntreatedArea(off_t& untreated_start, o // to_upload_list : A list of areas to upload in multipart upload. // to_copy_list : A list of areas for copy upload in multipart upload. // to_download_list : A list of areas that must be downloaded before multipart upload. -// cancel_upload_list : A list of areas that have already been uploaded and will be canceled(overwritten). +// cancel_upload_list : A list of areas that have already been uploaded and will be canceled(overwritten). +// wait_upload_complete : If cancellation areas exist, this flag is set to true when it is necessary to wait until the upload of those cancellation areas is complete. // file_size : The size of the upload file. // use_copy : Specify true if copy multipart upload is available. // @@ -771,7 +772,7 @@ bool PseudoFdInfo::ExtractUploadPartsFromUntreatedArea(off_t& untreated_start, o // The untreated_list in fdentity does not change, but upload_list is changed. // (If you want to restore it, you can use cancel_upload_list.) // -bool PseudoFdInfo::ExtractUploadPartsFromAllArea(UntreatedParts& untreated_list, mp_part_list_t& to_upload_list, mp_part_list_t& to_copy_list, mp_part_list_t& to_download_list, filepart_list_t& cancel_upload_list, off_t max_mp_size, off_t file_size, bool use_copy) +bool PseudoFdInfo::ExtractUploadPartsFromAllArea(UntreatedParts& untreated_list, mp_part_list_t& to_upload_list, mp_part_list_t& to_copy_list, mp_part_list_t& to_download_list, filepart_list_t& cancel_upload_list, bool& wait_upload_complete, off_t max_mp_size, off_t file_size, bool use_copy) { AutoLock auto_lock(&upload_list_lock); @@ -780,6 +781,7 @@ bool PseudoFdInfo::ExtractUploadPartsFromAllArea(UntreatedParts& untreated_list, to_copy_list.clear(); to_download_list.clear(); cancel_upload_list.clear(); + wait_upload_complete = false; // Duplicate untreated list untreated_list_t dup_untreated_list; @@ -939,6 +941,11 @@ bool PseudoFdInfo::ExtractUploadPartsFromAllArea(UntreatedParts& untreated_list, // So this current area only needs to be uploaded again. // S3FS_PRN_DBG("Cancel upload: start=%lld, size=%lld", static_cast(overlap_uploaded_iter->startpos), static_cast(overlap_uploaded_iter->size)); + + if(!overlap_uploaded_iter->uploaded){ + S3FS_PRN_DBG("This cancel upload area is still uploading, so you must wait for it to complete before starting any Stream uploads."); + wait_upload_complete = true; + } cancel_upload_list.push_back(*overlap_uploaded_iter); // add this uploaded area to cancel_upload_list uploaded_iter = upload_list.erase(overlap_uploaded_iter); // remove it from upload_list diff --git a/src/fdcache_fdinfo.h b/src/fdcache_fdinfo.h index 19e15b527c..de23d1797f 100644 --- a/src/fdcache_fdinfo.h +++ b/src/fdcache_fdinfo.h @@ -85,7 +85,6 @@ class PseudoFdInfo bool CompleteInstruction(int result, AutoLock::Type type = AutoLock::NONE); bool ParallelMultipartUpload(const char* path, const mp_part_list_t& mplist, bool is_copy, AutoLock::Type type = AutoLock::NONE); bool InsertUploadPart(off_t start, off_t size, int part_num, bool is_copy, etagpair** ppetag, AutoLock::Type type = AutoLock::NONE); - int WaitAllThreadsExit(); bool CancelAllThreads(); bool ExtractUploadPartsFromUntreatedArea(off_t& untreated_start, off_t& untreated_size, mp_part_list_t& to_upload_list, filepart_list_t& cancel_upload_list, off_t max_mp_size); @@ -115,8 +114,9 @@ class PseudoFdInfo bool ParallelMultipartUploadAll(const char* path, const mp_part_list_t& to_upload_list, const mp_part_list_t& copy_list, int& result); + int WaitAllThreadsExit(); ssize_t UploadBoundaryLastUntreatedArea(const char* path, headers_t& meta, FdEntity* pfdent); - bool ExtractUploadPartsFromAllArea(UntreatedParts& untreated_list, mp_part_list_t& to_upload_list, mp_part_list_t& to_copy_list, mp_part_list_t& to_download_list, filepart_list_t& cancel_upload_list, off_t max_mp_size, off_t file_size, bool use_copy); + bool ExtractUploadPartsFromAllArea(UntreatedParts& untreated_list, mp_part_list_t& to_upload_list, mp_part_list_t& to_copy_list, mp_part_list_t& to_download_list, filepart_list_t& cancel_upload_list, bool& wait_upload_complete, off_t max_mp_size, off_t file_size, bool use_copy); }; typedef std::map> fdinfo_map_t;