statamic / ideas

💡Discussions on ideas and feature requests for Statamic
https://statamic.dev
31 stars 1 forks source link

Don't overlap CommitJob? #1024

Open strebl opened 1 year ago

strebl commented 1 year ago

Hi 👋🏻

We are using the automatic commit feature (queued). To prevent a flood of commits, we delay the commit using STATAMIC_GIT_DISPATCH_DELAY. That works great so far.

However, each save pushes a job to the queue. Is that really necessary? Are you open for a PR preventing this using the WithoutOverlapping middleware?

We have projects where multiple people save often and therefore adding a lot of jobs to the queue which therefore commit more often than we want and kinda make the STATAMIC_GIT_DISPATCH_DELAY option useless.

jasonvarga commented 1 year ago

That PR sounds fine.

Unless I'm mistaken though, if nothing is modified, nothing will get committed.

strebl commented 1 year ago

I made a small illustration of the current situation assuming STATAMIC_GIT_DISPATCH_DELAY=5.

Screenshot 2023-08-02 at 18 04 20

The first commit is what's expected, but the smaller commits (2-5) are not what we want and job (commit) 6-10 are unnecessary but won't do any harm.

With the PR only commit 1 will be queued and change 7 will trigger another job which will create the second commit. So down from 10 to 2 jobs and also only 2 commits.

jasonvarga commented 1 year ago

Love that illustration. The WithoutOverlapping middleware sounds like a good idea. It's possible when we initially wrote the displatch delay feature, that middleware didn't exist (or didn't exist on the lowest Laravel version we supported).

jesseleite commented 1 year ago

Yes this sounds like a good change to me as well ❤️

buffalom commented 1 year ago

We just realised that it isn't actually the WithoutOverlapping-middleware that we want to use, but rather the ShouldBeUnique-interface.

WithoutOverlapping makes sure there are never two jobs processed simultaneously, while ShouldBeUnique makes sure there's only ever one in the queue.

However, if I understand it correctly, the ideal setup for this specific use-case would be a combination of WithoutOverlapping and ShouldBeUniqueUntilProcessing. With that, we make sure all the latest changes are committed while still avoiding those unwanted, smaller commits.