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

a8n: Move "default branch" check to execution of CampaignJob #7725

Closed
mrnugget opened this issue Jan 14, 2020 · 0 comments · Fixed by #8008
Closed

a8n: Move "default branch" check to execution of CampaignJob #7725

mrnugget opened this issue Jan 14, 2020 · 0 comments · Fixed by #8008
Labels
batch-changes Issues related to Batch Changes estimate/2d planned/3.13 Issues that were planned for the given milestone. Used by cmd/tracking-issue. roadmap Issue tracking a product-eng roadmap item
Milestone

Comments

@mrnugget
Copy link
Contributor

mrnugget commented Jan 14, 2020

Current state when creating a new CampaignPlan ("Preview changes"):

  • We open a transaction
  • For each repo the scopeQuery yields we create a CampaignJob
  • For each CampaignJob we ask gitserver for its default branch
  • If gitserver cannot determine a default branch the transaction is rolled back
  • Otherwise: transaction is committed

Here is the code.

That means the transaction is open for the duration of time it takes to
ask gitserver for all default branches.

That's not a problem per se, but it is bad UX since it might take a
while until the user sees something after hitting the "Preview changes" button.

I propose that we move the determination of a CampaignJobs BaseRef
and Rev out of the transaction and set those when actually running the
job here.

That would require making rev and base_ref nullable fields in the
database and since we have a database unique constraint on
(campaign_plan_id, repo_id, rev) we have to find out how that works
with NULL fields.

The upsides:

  1. We don't hammer gitserver with possibly hundreds of requests in a tight loop
  2. One request failing won't fail the whole CampaignPlan but instead result in a single errored CampaignJob
  3. The user will get faster feedback when previewing changes because the transaction will be much faster since it's not bound to gitserver anymore
@mrnugget mrnugget added the roadmap Issue tracking a product-eng roadmap item label Jan 14, 2020
@mrnugget mrnugget added this to the 3.13 milestone Jan 15, 2020
@mrnugget mrnugget self-assigned this Jan 16, 2020
@mrnugget mrnugget assigned kzh and unassigned mrnugget Jan 17, 2020
@tsenart tsenart added the planned/3.13 Issues that were planned for the given milestone. Used by cmd/tracking-issue. label Jan 24, 2020
@sqs sqs closed this as completed in #8008 Jan 26, 2020
@chrispine chrispine added the batch-changes Issues related to Batch Changes label Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
batch-changes Issues related to Batch Changes estimate/2d planned/3.13 Issues that were planned for the given milestone. Used by cmd/tracking-issue. roadmap Issue tracking a product-eng roadmap item
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants