Skip to content

Remove concurrency limits from scheduled cron jobs in GoodJob#7197

Merged
mitchellhenke merged 2 commits intomainfrom
mitchellhenke/remove-good-job-concurrency
Oct 24, 2022
Merged

Remove concurrency limits from scheduled cron jobs in GoodJob#7197
mitchellhenke merged 2 commits intomainfrom
mitchellhenke/remove-good-job-concurrency

Conversation

@mitchellhenke
Copy link
Contributor

🛠 Summary of changes

Using the concurrency control limits has some performance impacts, but our usage of them should be unnecessary at this point (as mentioned in bensheldon/good_job#731)

changelog: Internal, GoodJob, Remove concurrency limits from scheduled cron jobs in GoodJob
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

Nice find! I'm so glad that the library handles this for us. We're past v2.5.0 right?

@mitchellhenke
Copy link
Contributor Author

Nice find! I'm so glad that the library handles this for us. We're past v2.5.0 right?

Yep, we're on v3.4.4 now and #7196 takes us to v3.6.0

@mitchellhenke mitchellhenke merged commit 65fe73f into main Oct 24, 2022
@mitchellhenke mitchellhenke deleted the mitchellhenke/remove-good-job-concurrency branch October 24, 2022 19:41
zachmargolis added a commit that referenced this pull request Nov 8, 2022
…#7197)"

This reverts commit 65fe73f.

**Why**: We discovered daily jobs like the GpoDailyJob were running twice
in the same day, indicating we still need these guards
zachmargolis added a commit that referenced this pull request Nov 8, 2022
#7317)

* Revert "Remove concurrency limits from scheduled cron jobs in GoodJob (#7197)"

This reverts commit 65fe73f.

**Why**: We discovered daily jobs like the GpoDailyJob were running twice
in the same day, indicating we still need these guards

changelog: Internal, GoodJob, Add back in concurrency limits from scheduled cron jobs
zachmargolis added a commit that referenced this pull request Nov 8, 2022
#7317)

* Revert "Remove concurrency limits from scheduled cron jobs in GoodJob (#7197)"

This reverts commit 65fe73f.

**Why**: We discovered daily jobs like the GpoDailyJob were running twice
in the same day, indicating we still need these guards

changelog: Internal, GoodJob, Add back in concurrency limits from scheduled cron jobs
(cherry picked from commit 3e8d252)
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.

2 participants