Skip to content

Add API to respond with for running backfills #343

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

jrobotham-square
Copy link
Collaborator

@jrobotham-square jrobotham-square commented Nov 9, 2023

When an incident occurs it's often really valuable to be able to see what Backfills are currently running in order to:

  • Identify ones that may be causing or exacerbating issues.
  • Identify ones that could be stopped to reduce the risk.

This PR exposes a new endpoint to surface the currently running backfills.

As part of this change I've also included instructions on starting mysql locally for development and fixed some issues that occur locally when building / running tests on Apple silicon.

Subsequent PR will integrate this endpoint into the UI to create a screen something like:
Screenshot 2023-11-09 at 8 37 16 am

supports the ability to more easily see what's currently running when investigating possible
issues or looking to turn things off temporarily.

UI changes to come in separate PR
@adrw
Copy link
Collaborator

adrw commented Nov 9, 2023

Mind reposting the screenshots, they don't seem to be showing in the preview?

@mpawliszyn
Copy link
Collaborator

Make sure to rebase on top of the variant PR.

private val queryFactory: Query.Factory,
) : WebAction {

@Get("/backfills/running")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably needs to be paginated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah - was kinda assuming the number of backfills concurrently running wouldn't generally be enough to require pagination, but that's I guess that's not necessarily the case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, looks like we don't have pagination on any of the endpoints apart from the /services/{service}/variants/{variant}/backfill-runs one. (which makes sense because it's completely unbounded)

Feels like we might be able to avoid it for this endpoint given it's only returning currently running ones. WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You are listing runs so of course it needs to be paginated.

The other ones might need a rethink soon given how popular Backfila has become.

Why did you pick runs to show rather than services?

@jrobotham-square
Copy link
Collaborator Author

Mind reposting the screenshots, they don't seem to be showing in the preview?

👍 done. (not sure what happened there)

@jrobotham-square
Copy link
Collaborator Author

Make sure to rebase on top of the variant PR.

I've rebased. It's not immediately obvious to me how the variant changes work, I've just hardcoded some values to make the tests I added pass.

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.

3 participants