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

Introduces a job that runs the AMPF sync on a per storage basis #15979

Merged
merged 1 commit into from
Jun 28, 2024

Conversation

mereghost
Copy link
Contributor

@mereghost mereghost commented Jun 26, 2024

⚠️ Related WP: OP#55916

Why?

We run a long job with that syncs all the Storages::Storage that have automatically managed folders enabled with its provider.

This job is triggered basically all the time and at a schedule. Project Created, run it. Member added somewhere, run it. Sneezed in a funny manner, run it.

This leads to a lot of wasted resources as we are doing work that probably isn't required.

What?

This PR aims to reduce the processing time and wasted resources by splitting the work on a per storage basis.

This would still avoid some race conditions while giving us slightly more flexibility when dealing with the synchronization process.

How?

First all the logic from Storages::ManageIntegrationJob was moved to Storages::AutomaticallyManagedStorageSyncJob.

This new job works as mentioned above: on a single Storages::Storage and has some concurrency controls over it allowing at most 2 jobs for each storage (1 running/1 on queue).

This new job was then hooked to the Members, Projects and ProjectStorages events, so that it queues a job only for the relevant Storages::Storage, if any.

The original job then will only need to queue this new one for each storage to keep its original functionality and benefit from the granularity.

@mereghost mereghost self-assigned this Jun 26, 2024
@mereghost mereghost force-pushed the maint/split-sync-per-storage branch from 4eb1816 to bfe812c Compare June 26, 2024 07:46
@mereghost mereghost force-pushed the maint/split-sync-per-storage branch from d76ab1b to c2d68d2 Compare June 26, 2024 17:09
@mereghost mereghost marked this pull request as ready for review June 27, 2024 08:22
@mereghost mereghost requested review from ba1ash and Kharonus June 27, 2024 08:22
Copy link
Member

@ba1ash ba1ash left a comment

Choose a reason for hiding this comment

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

I think you forgot to specify the queue for newly created job.
queue_with_priority :above_normal

Copy link
Member

@ba1ash ba1ash left a comment

Choose a reason for hiding this comment

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

Looks good to me. Well done!

Cleans up the big job
Update Event handling
Extracts the debounce logic to a module, so it can be reused by other jobs
@mereghost mereghost force-pushed the maint/split-sync-per-storage branch from 6801fad to 9808402 Compare June 28, 2024 11:28
@mereghost mereghost merged commit 467d80a into dev Jun 28, 2024
9 checks passed
@mereghost mereghost deleted the maint/split-sync-per-storage branch June 28, 2024 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants