Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync bugs with unexpected deletes #1

Open
PhilZ-cwm6 opened this issue Jul 28, 2020 · 1 comment
Open

Sync bugs with unexpected deletes #1

PhilZ-cwm6 opened this issue Jul 28, 2020 · 1 comment

Comments

@PhilZ-cwm6
Copy link

PhilZ-cwm6 commented Jul 28, 2020

Hi,
Out of curiosity I looked quickly at the sync code in your current version. I know it is beta, so I never looked at it before, but I see that you largely edited the sync checks to delete files and they are again broken like before

https://github.com/Sentaroh/SMBSync3/blob/main/app/src/main/java/com/sentaroh/android/SMBSync3/SyncThreadSyncFile.java#L167

I did not look at the filters checks if code was changed for isFileSelected() and is DirectoryToBeProcessed() but the implementation was also optimized for overhead. The only project I know of and that has a non broken code is the one I linked at (wildcard-project). However, they use a char by char check instead of ever calling a pattern check. Speed is same but more prone to custom code bugs. Hope you deeply verify and check all possibilities if you modify it.

Note: I am taking a long break and do not plan to push commits except fr/en translations from time to time. I plan to migrate to Android R by end of year or begin 2021, so I am not migrating before that to SMBSync3 and won't really look at it before.

I will push the xda and Phone Android threads since the fixed filters v2 is now enabled.

Thank you for your free time on the project and Best regards

@PhilZ-cwm6
Copy link
Author

PhilZ-cwm6 commented Aug 20, 2020

I took a very quick look in one function SyncThreadSyncFile.java/LocalToLocal()
New code is much better and looks ok in regard of unexpected deletes/non deletes

There is still a room for a potential big improvement in sync speed when filter excludes many files or folders by wildcards for example (exclude *.jpg, *.tmp...). Your current code will check each excluded file/folder if it exists on master. The operation is very time consuming depending on the file system, and worst if master is SMB.

I suggest you this implementation, maybe more clear than the one in my original code in SMBSync2 (I only put the directory part, but file part is same logic).

Your current code for directories:

boolean mf_exists=mf.exists();
boolean isDirectoryToBeProcessed=SyncThread.isDirectoryToBeProcessed(stwa, relative_dir);
if (mf_exists && isDirectoryToBeProcessed) {
    //process subdirs
} else {
    if ((!mf_exists && isDirectoryToBeProcessed) ||
            (sti.isSyncOptionRemoveDirectoryFileThatExcludedByFilter() && !isDirectoryToBeProcessed)) {
        //delete target item
    }
}

Change it to this logic:

boolean isDirectoryToBeProcessed=SyncThread.isDirectoryToBeProcessed(stwa, relative_dir);

if (isDirectoryToBeProcessed) {
    if (mf.exists()) {
        //process subdirs
    } else {
        //delete target item
    }
} else { //excluded by filters, never check if file/dir exists on master
    if (sti.isSyncOptionRemoveDirectoryFileThatExcludedByFilter()) {
        //delete target item
    } else {
        //nothing to do, skip
    }
}

The code is more clear and if a file/dir is excluded by filters, you never check if it exists on the master.
The impact can be huge as explained above when there is a filter that excludes many files/dirs by generic wildcards (*.tmp, *.jpg, ...)

Again, I am not planning to compile/test it for now, but I am interested in all these future developments.

Best regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant