Look for new branches more carefully
What does this MR do?
In certain cases, GitLab can miss a PostReceive invocation the first time a branch is pushed. When this happens, the "branch created" hooks are not run, which means various features don't work until the branch is deleted and pushed again.
This MR changes the Git::BranchPushService
so it checks the cache of
existing branches in addition to the oldrev
reported for the branch.
If the branch name isn't in the cache, chances are we haven't run the
service yet (it's what refreshes the cache), so we can go ahead and
run it, even through oldrev
is set.
If the cache has been cleared by some other means in the meantime, then we'll still fail to run the hooks when we should. Fixing that in the general case is a larger problem, and we'd need to devote significant engineering effort to it. If the cache is empty when we run, we'll fill it, and then immediately empty it again, but this should be a rare situation.
There's a chance that we'll run the relevant hooks multiple times
with this change, if there's a race between the branch being created,
and the PostReceive
worker being run multiple times, but this can
already happen, since Sidekiq is "at-least-once" execution of jobs. So,
this should be safe.
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation created/updated or follow-up review issue created -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Performance and testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Closes #59257 (closed)