Explicitly handle CommonPrefixes with nocompat_dir #2212
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@ggtakec this fixes a serious regression in the default behavior from 1.91. I think we can handle CommonPrefixes more elegantly/explicitly but this is a minimal fix. |
gaul
force-pushed
the
noncompat_dir/common-prefix
branch
from
4b5b98f
to
026633a
Compare
Closed
ggtakec
requested changes
Jul 13, 2023
There was a problem hiding this comment.
@gaul Thanks for your fix!
Bit, it seems to be missing a place to set the path to filled yet.
With this current PR code, the ls command ends up listing the same file multiple times.
I've confirmed that adding the path to filled at every call to filler fixes this problem.
I made the code in this PR plus the following fixes and confirmed that fixed code works fine.
Could you check the following and fix it?
- Added
filledto Multi-Head request callback parameter
3067: struct multi_head_callback_param
3067: {
3067: void* buf;
3067: fuse_fill_dir_t filler;
+ std::set<std::string>* pfilled;
3067: };
- In Multi-Head request callback function
3091: if(param){
3092: struct multi_head_callback_param* pcbparam = reinterpret_cast<struct multi_head_callback_param*>(param);
3093: struct stat st;
3094: if(StatCache::getStatCacheData()->GetStat(saved_path, &st)){
3095: pcbparam->filler(pcbparam->buf, bpath.c_str(), &st, 0);
3096: }else{
3097: S3FS_PRN_INFO2("Could not find %s file in stat cache.", saved_path.c_str());
3098: pcbparam->filler(pcbparam->buf, bpath.c_str(), 0, 0);
3099: }
+ pcbparam->pfilled->insert(bpath);
3100: }else{
3101: S3FS_PRN_WARN("param(multi_head_callback_param*) is NULL, then can not call filler.");
3102: }
- Set
filledpointer to Multi-Head callback parameter
3186: struct multi_head_callback_param success_param;
3187: success_param.buf = buf;
3188: success_param.filler = filler;
+ success_param.pfilled = &filled;
3189: curlmulti.SetSuccessCallbackParam(reinterpret_cast<void*>(&success_param));
- The
filledcalling is leaking
3291: if(StatCache::getStatCacheData()->AddStat(dirpath, dummy_header, true)){ // set forcedir=true
3292: // Get stats from stats cache(for converting from meta), and fill
3293: std::string base_path = mybasename(dirpath);
3294: if(use_wtf8){
3295: base_path = s3fs_wtf8_decode(base_path);
3296: }
3297:
3298: struct stat st;
3299: if(StatCache::getStatCacheData()->GetStat(dirpath, &st)){
3300: filler(buf, base_path.c_str(), &st, 0);
3301: }else{
3302: S3FS_PRN_INFO2("Could not find %s directory(no dir object) in stat cache.", dirpath.c_str());
3303: filler(buf, base_path.c_str(), 0, 0);
3304: }
+ filled.insert(base_path);
3305: }else{
3306: S3FS_PRN_ERR("failed adding stat cache [path=%s], but dontinue...", dirpath.c_str());
3307: }
Once this fix is done, I'll make a PR which is fixed possible the Thread data race soon.
Previously the test missed listing implicit directories and another test was incorrect. This fixes a regression from 1.91.
Done. |
gaul
force-pushed
the
noncompat_dir/common-prefix
branch
from
026633a
to
c405ad2
Compare
|
@gaul |
|
This symptom is gaul/s3proxy#303 which the next version will address. |
|
@gaul Thanks, wait for next version. |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously the test missed listing implicit directories and another test was incorrect. This fixes a regression from 1.91.