Currently the soonest a change is push mirrored is at least 5 minutes after the change is detected. This is done for performance reasons to decrease the frequency of the push. In reality it is typically much slower.
This will also cause serious in bidirectional mirroring scenarios where delays will create more opportunity for conflict.
@jramsay Nope, with push mirrors we actually force push and override any changes on the remote.
Creating a merge commit in the local repo that combines the local and remote branches, and then pushing that to the remote makes a lot of sense. Of course that is only an option if you can update both repos so that they are in agreement again, which we can do with push mirrors, but not pull mirrors. With bi-directional syncs, we have that option too, of course.
Note that there are some projects that require the master branch to be a straight history with only fast-forward merges (which is why we have that Merge Method option on EE), so those projects/people may not want this automatic "conflict resolution".
Note that this automatic "conflict resolution" may also be a problem when the local branch was intentionally force pushed into; we'd end up trying to merge the new local code with the old code that was synced, but should now be overridden.
If we only create a merge commit with protected branches, we can definitely use the merge commit, because we know there can be no force pushes locally, only remotely, which means we definitely want both the local and remote commits to survive.
@jramsay Instead of removing the limit, can we lower the 5-minute delay to 1 minute? In a project with a lot of activity on the master branch (like GitLab), we can't have each push result in an immediate push to the remote mirror. There's a reason we implemented the delay :)
Is creating a merge commit if fast-forward is not possible in scope for this issue? That's the only way to really do bi-directional sync (https://gitlab.com/gitlab-org/gitlab-ee/issues/3902), so if we want to prepare the backend for bi-directional sync, we'll need that too.
Without that, I'd say the weight is about 4. With that, the weight is about 6, so I'll put it at 5 for the moment.
Looking at the git logs for gitlab-ce only 1269 of 18596 merges to master (bi-directional is protect branches only) occurred within 60 seconds of each other. Also, the first quartile is 428 seconds (over 7 minutes) meaning 75% of merges are actually quite widely spaced.
The ideal experience would be for sync to be instant. Rather than delaying all sync operations by 1 minute which delays over 90% of sync operations for no gain, maybe we could implement a rate limit of no more than once per minute? This means most of the time sync is instant without creating undue load.
@jramsay So we'd do the first push instantly, and then if one or more pushes happen within the next 60 seconds, we wait the full 60 seconds to bundle them and push them up? I think that makes sense, good idea.
@jramsay Before we go making any changes though, do we know for sure that a 5 minute or 1 minute delay is not acceptable? Maybe that's perfectly fine to the customer; could we check? With https://gitlab.com/gitlab-org/gitlab-ee/issues/3943, we'll automatically resolve diverged branches anyway.
@DouweM it's important for mirroring because it decreases the window in which a conflict can occur. It's not really about a user needing to see the change for productivity reasons.
@tiagonbotelho the scope of this issue is about making it fast for protected branches, because we need it for bi-directional mirroring which will only support protected branches. However, I would imagine we'd prefer to keep the logic as similar as possible between the two scenarios?
Maybe we the logic proposed in https://gitlab.com/gitlab-org/gitlab-ee/issues/3832#note_46485537 could be used in all situations, but vary the rate limit? For example, use 1 minute for the rate limit for protected branches, and 5 minutes for the rate limit for all others. What do you think @DouweM?