From 7d37ff3143e5fc8f32dda9e1abe2dde5db24248c Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Tue, 10 Jun 2014 21:47:04 +0200 Subject: ChangeMergeQueue: Fix race condition To the multiple triggers of submit action belong: * ChangeMergeQueue.merge() method called from the UI * ChangeMergeQueue.schedule() method called by background ReloadSubmitQueueOp active map in ChangeMergeQueue acts as a guard for submit job scheduling. However schedule() ignores the fact that the submit processing was already started for a specific branch and double submit action is scheduled. Specifically for cherry-pick submit strategy that cannot end good and fails with database constraints violation or similar, depending on the underlying database implementation. Guard the background scheduling similar to the UI scheduling. Defining unique index prevents the database corruption: [1], [2] but not the collision between manual and background merge jobs. [1] http://paste.openstack.org/show/83883 [2] http://paste.openstack.org/show/83888 Bug: issue 2034 Bug: issue 2383 Bug: issue 2702 Change-Id: I5b23f9d481351280e26412b82b525947338d9c00 (cherry picked from commit f26fdb499b26a2c02251546a7bb19703cc6c8bba) Reviewed-by: Oswald Buddenhagen Reviewed-by: Ismo Haataja --- .../main/java/com/google/gerrit/server/git/ChangeMergeQueue.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMergeQueue.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMergeQueue.java index 12219e23cf..e067d8ee8f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMergeQueue.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMergeQueue.java @@ -147,9 +147,11 @@ public class ChangeMergeQueue implements MergeQueue { if (e == null) { e = new MergeEntry(branch); active.put(branch, e); + e.needMerge = true; + scheduleJob(e); + } else { + e.needMerge = true; } - e.needMerge = true; - scheduleJob(e); } @Override -- cgit v1.2.3