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

Create individual workers for every Autoupload Folder #13015

Merged
merged 5 commits into from
Jun 3, 2024

Conversation

JonasMayerDev
Copy link
Collaborator

@JonasMayerDev JonasMayerDev commented May 16, 2024

This PR finally gets rid of the foreground worker and the corresponding notification by lowering the chance the worker gets killed.

Internally, the app now starts a new worker for every auto upload folder instead of one worker for all folders.
Advantages are:

  • Worker finish quicker -> Lower chance of being killed
  • Scanning of Folders can happen in parallel
  • Better handling in the future

The best way to test is using logcat and filtering for "File sync" since all important log messages start with that term. Like this, you should be able to observe the behavior of the background worker without messing with it. If you want to check something specific, use the debugger, but be aware that that changes behavior (like execution speed).

  • Tests written, or not not needed

@JonasMayerDev JonasMayerDev changed the title Remove ForegroundWorker from Auto Upload Create individual workers for every Autoupload Folder May 16, 2024
@JonasMayerDev JonasMayerDev force-pushed the remove-foreground-worker-for-auto-upload branch 3 times, most recently from d011ec9 to 49103a7 Compare May 16, 2024 17:55
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
@JonasMayerDev JonasMayerDev force-pushed the remove-foreground-worker-for-auto-upload branch from bbc8743 to d91b652 Compare May 28, 2024 18:06
@JonasMayerDev JonasMayerDev marked this pull request as ready for review May 28, 2024 18:07
Copy link

Codacy

Lint

TypemasterPR
Warnings7171
Errors33

SpotBugs

CategoryBaseNew
Bad practice6464
Correctness6969
Dodgy code346346
Experimental11
Internationalization77
Malicious code vulnerability22
Multithreaded correctness66
Performance5757
Security1919
Total571571

Copy link

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/13015.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

Copy link

Copy link
Collaborator

@alperozturk96 alperozturk96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested. I created 3 folder set auto upload and only 3 worker created for each auto upload and all uploads successfully completed.

@alperozturk96 alperozturk96 merged commit 0459e97 into master Jun 3, 2024
21 checks passed
@delete-merged-branch delete-merged-branch bot deleted the remove-foreground-worker-for-auto-upload branch June 3, 2024 09:49
@AndyScherzinger AndyScherzinger added this to the Nextcloud App 3.30.0 milestone Jun 3, 2024
@JonasMayerDev JonasMayerDev mentioned this pull request Jun 3, 2024
4 tasks
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

Successfully merging this pull request may close these issues.

None yet

3 participants