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

Add config for the buffer #3308

Conversation

TheJokersThief
Copy link

@TheJokersThief TheJokersThief commented Mar 30, 2023

Commit Checklist

Thank you for creating a pull request! To help us review / merge this can you make sure that your PR adheres as much as possible to the following.

The Basics

  • Commit is a single logical unit of work, only use multiple commits if doing different tasks
  • Commit does not include commented out code or unneeded files
  • rebase of main branch

The Content

  • Must include testing for bug or feature
  • Must include appropriate documentation changes if it is introducing a new feature or changing existing functionality
  • Must pass existing test suites

The Commit Message

  • Short meaningful description (ex: remove deprecated steps)
  • Uses the imperative, present tense: "change", not "changed" or "changes"
  • Includes motivation for the change, and contrasts its implementation with the previous behavior

The Pull Request

What is the reason for this change

The cleanup script adds a hardcoded buffer time of 30 minutes to its decision making. This is fine if you're working on the scale of days (the default scale is 24hours) but we've found a want to kill pending jobs a lot sooner (on the scale of 20-30 mins) to avoid large backlogs of builds and a thundering herd problem as they start running.

This PR proposes to keep the default behaviour, but make the buffer configurable so that folks can tune it to their needs :)

@CLAassistant
Copy link

CLAassistant commented Mar 30, 2023

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@eoinmcafee00
Copy link

Hi @TheJokersThief

Thank you for submitting this PR. While the intention is to "keep the default behavior and make the buffer configurable to accommodate various needs," it appears that a default buffer of 30 minutes has been set. This change would impact everyone, which may not be ideal.

@TheJokersThief
Copy link
Author

@eoinmcafee00
Copy link

ah I missed the last line @TheJokersThief

@eoinmcafee00 eoinmcafee00 merged commit 8668d90 into harness:master Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants