Replace all transactions with single row updates

We now try to perform writes in a reasonably sane order, but only
update one record at a time (or do a batch against the same table
but not in a transaction).

This matches with eventually consistent databases like Cassandra
where we can't rely upon a transaction being available for any
sort of update we make.

Change-Id: Ib8cdff05f08467dfe3258f03b72d5cabb5b2641f
Signed-off-by: Shawn O. Pearce <sop@google.com>

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/ChangeListServiceImpl.java

@@ -41,7 +41,6 @@ import com.google.gwt.user.client.rpc.AsyncCallback;
 import com.google.gwtjsonrpc.client.VoidResult;
 import com.google.gwtorm.client.OrmException;
 import com.google.gwtorm.client.ResultSet;
-import com.google.gwtorm.client.Transaction;
 import com.google.gwtorm.client.impl.ListResultSet;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -402,8 +401,8 @@ public class ChangeListServiceImpl extends BaseServiceImplementation implements
       public VoidResult run(final ReviewDb db) throws OrmException {
         final Account.Id me = getAccountId();
         final Set<Change.Id> existing = currentUser.get().getStarredChanges();
-        final ArrayList<StarredChange> add = new ArrayList<StarredChange>();
+        List<StarredChange> add = new ArrayList<StarredChange>();
-        final ArrayList<StarredChange> remove = new ArrayList<StarredChange>();
+        List<StarredChange.Key> remove = new ArrayList<StarredChange.Key>();
 
         if (req.getAddSet() != null) {
           for (final Change.Id id : req.getAddSet()) {
@@ -415,18 +414,12 @@ public class ChangeListServiceImpl extends BaseServiceImplementation implements
 
         if (req.getRemoveSet() != null) {
           for (final Change.Id id : req.getRemoveSet()) {
-            if (existing.contains(id)) {
+            remove.add(new StarredChange.Key(me, id));
-              remove.add(new StarredChange(new StarredChange.Key(me, id)));
-            }
           }
         }
 
-        if (!add.isEmpty() || !remove.isEmpty()) {
+        db.starredChanges().insert(add);
-          final Transaction txn = db.beginTransaction();
+        db.starredChanges().deleteKeys(remove);
-          db.starredChanges().insert(add);
-          db.starredChanges().delete(remove);
-          txn.commit();
-        }
         return VoidResult.INSTANCE;
       }
     });

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountSecurityImpl.java

@@ -48,7 +48,6 @@ import com.google.gwtjsonrpc.client.VoidResult;
 import com.google.gwtjsonrpc.server.ValidToken;
 import com.google.gwtjsonrpc.server.XsrfException;
 import com.google.gwtorm.client.OrmException;
-import com.google.gwtorm.client.Transaction;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
@@ -152,13 +151,8 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
             throw new Failure(new NoSuchEntityException());
         }
 
-        final List<AccountSshKey> k = db.accountSshKeys().get(ids).toList();
+        db.accountSshKeys().deleteKeys(ids);
-        if (!k.isEmpty()) {
+        uncacheSshKeys();
-          final Transaction txn = db.beginTransaction();
-          db.accountSshKeys().delete(k, txn);
-          txn.commit();
-          uncacheSshKeys();
-        }
 
         return VoidResult.INSTANCE;
       }

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountServiceImpl.java

@@ -31,7 +31,6 @@ import com.google.gerrit.server.project.ProjectControl;
 import com.google.gwt.user.client.rpc.AsyncCallback;
 import com.google.gwtjsonrpc.client.VoidResult;
 import com.google.gwtorm.client.OrmException;
-import com.google.gwtorm.client.Transaction;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
@@ -153,14 +152,7 @@ class AccountServiceImpl extends BaseServiceImplementation implements
             throw new Failure(new NoSuchEntityException());
         }
 
-        final List<AccountProjectWatch> k =
+        db.accountProjectWatches().deleteKeys(keys);
-            db.accountProjectWatches().get(keys).toList();
-        if (!k.isEmpty()) {
-          final Transaction txn = db.beginTransaction();
-          db.accountProjectWatches().delete(k, txn);
-          txn.commit();
-        }
-
         return VoidResult.INSTANCE;
       }
     });

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/DeleteExternalIds.java

@@ -21,7 +21,6 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountByEmailCache;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gwtorm.client.OrmException;
-import com.google.gwtorm.client.Transaction;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 
@@ -73,9 +72,7 @@ class DeleteExternalIds extends Handler<Set<AccountExternalId.Key>> {
     }
 
     if (!toDelete.isEmpty()) {
-      final Transaction txn = db.beginTransaction();
+      db.accountExternalIds().delete(toDelete);
-      db.accountExternalIds().delete(toDelete, txn);
-      txn.commit();
       accountCache.evict(user.getAccountId());
       for (AccountExternalId e : toDelete) {
         byEmailCache.evict(e.getEmailAddress());

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java

@@ -35,7 +35,6 @@ import com.google.gerrit.server.account.Realm;
 import com.google.gwt.user.client.rpc.AsyncCallback;
 import com.google.gwtjsonrpc.client.VoidResult;
 import com.google.gwtorm.client.OrmException;
-import com.google.gwtorm.client.Transaction;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
@@ -231,12 +230,10 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
         AccountGroupMember m = db.accountGroupMembers().get(key);
         if (m == null) {
           m = new AccountGroupMember(key);
-          final Transaction txn = db.beginTransaction();
-          db.accountGroupMembers().insert(Collections.singleton(m), txn);
           db.accountGroupMembersAudit().insert(
               Collections.singleton(new AccountGroupMemberAudit(m,
-                  getAccountId())), txn);
+                  getAccountId())));
-          txn.commit();
+          db.accountGroupMembers().insert(Collections.singleton(m));
           accountCache.evict(m.getAccountId());
         }
 
@@ -279,19 +276,18 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
               }
             }
 
-            final Transaction txn = db.beginTransaction();
-            db.accountGroupMembers().delete(Collections.singleton(m), txn);
             if (audit != null) {
               audit.removed(me);
-              db.accountGroupMembersAudit().update(
+              db.accountGroupMembersAudit()
-                  Collections.singleton(audit), txn);
+                  .update(Collections.singleton(audit));
             } else {
               audit = new AccountGroupMemberAudit(m, me);
               audit.removedLegacy();
-              db.accountGroupMembersAudit().insert(
+              db.accountGroupMembersAudit()
-                  Collections.singleton(audit), txn);
+                  .insert(Collections.singleton(audit));
             }
-            txn.commit();
+
+            db.accountGroupMembers().delete(Collections.singleton(m));
             accountCache.evict(m.getAccountId());
           }
         }

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/AbandonChange.java

@@ -30,6 +30,7 @@ import com.google.gerrit.server.mail.EmailException;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gwtorm.client.AtomicUpdate;
 import com.google.gwtorm.client.OrmException;
 import com.google.gwtorm.client.OrmRunnable;
 import com.google.gwtorm.client.Transaction;
@@ -79,7 +80,7 @@ class AbandonChange extends Handler<ChangeDetail> {
     if (!control.canAbandon()) {
       throw new NoSuchChangeException(changeId);
     }
-    final Change change = control.getChange();
+    Change change = control.getChange();
     final PatchSet patch = db.patchSets().get(patchSetId);
     if (patch == null) {
       throw new NoSuchChangeException(changeId);
@@ -89,22 +90,37 @@ class AbandonChange extends Handler<ChangeDetail> {
         new ChangeMessage(new ChangeMessage.Key(changeId, ChangeUtil
             .messageUUID(db)), currentUser.getAccountId());
     final StringBuilder msgBuf =
-        new StringBuilder("Patch Set " + change.currentPatchSetId().get()
+        new StringBuilder("Patch Set " + patchSetId.get() + ": Abandoned");
-            + ": Abandoned");
     if (message != null && message.length() > 0) {
       msgBuf.append("\n\n");
       msgBuf.append(message);
     }
     cmsg.setMessage(msgBuf.toString());
 
-    Boolean dbSuccess = db.run(new OrmRunnable<Boolean, ReviewDb>() {
+    change = db.changes().atomicUpdate(changeId, new AtomicUpdate<Change>() {
-      public Boolean run(ReviewDb db, Transaction txn, boolean retry)
+      @Override
-          throws OrmException {
+      public Change update(Change change) {
-        return doAbandonChange(message, change, patchSetId, cmsg, db, txn);
+        if (change.getStatus().isOpen()
+            && change.currentPatchSetId().equals(patchSetId)) {
+          change.setStatus(Change.Status.ABANDONED);
+          ChangeUtil.updated(change);
+          return change;
+        } else {
+          return null;
+        }
       }
     });
 
-    if (dbSuccess) {
+    if (change != null) {
+      db.changeMessages().insert(Collections.singleton(cmsg));
+
+      final List<PatchSetApproval> approvals =
+          db.patchSetApprovals().byChange(changeId).toList();
+      for (PatchSetApproval a : approvals) {
+        a.cache(change);
+      }
+      db.patchSetApprovals().update(approvals);
+
       // Email the reviewers
       final AbandonedSender cm = abandonedSenderFactory.create(change);
       cm.setFrom(currentUser.getAccountId());
@@ -115,29 +131,4 @@ class AbandonChange extends Handler<ChangeDetail> {
 
     return changeDetailFactory.create(changeId).call();
   }
-
-  private Boolean doAbandonChange(final String message, final Change change,
-      final PatchSet.Id psid, final ChangeMessage cm, final ReviewDb db,
-      final Transaction txn) throws OrmException {
-
-    // Check to make sure the change status and current patchset ID haven't
-    // changed while the user was typing an abandon message
-    if (change.getStatus().isOpen() && change.currentPatchSetId().equals(psid)) {
-      change.setStatus(Change.Status.ABANDONED);
-      ChangeUtil.updated(change);
-
-      final List<PatchSetApproval> approvals =
-          db.patchSetApprovals().byChange(change.getId()).toList();
-      for (PatchSetApproval a : approvals) {
-        a.cache(change);
-      }
-      db.patchSetApprovals().update(approvals, txn);
-
-      db.changeMessages().insert(Collections.singleton(cm), txn);
-      db.changes().update(Collections.singleton(change), txn);
-      return Boolean.TRUE;
-    }
-
-    return Boolean.FALSE;
-  }
 }

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/SubmitAction.java

@@ -32,8 +32,8 @@ import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.workflow.CategoryFunction;
 import com.google.gerrit.server.workflow.FunctionState;
+import com.google.gwtorm.client.AtomicUpdate;
 import com.google.gwtorm.client.OrmException;
-import com.google.gwtorm.client.Transaction;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 
@@ -74,7 +74,8 @@ class SubmitAction extends Handler<ChangeDetail> {
   public ChangeDetail call() throws OrmException, NoSuchEntityException,
       IllegalStateException, PatchSetInfoNotAvailableException,
       NoSuchChangeException {
-    final Change change = db.changes().get(patchSetId.getParentKey());
+    final Change.Id changeId = patchSetId.getParentKey();
+    Change change = db.changes().get(changeId);
     if (change == null) {
       throw new NoSuchEntityException();
     }
@@ -84,7 +85,7 @@ class SubmitAction extends Handler<ChangeDetail> {
           + " not current");
     }
     if (change.getStatus().isClosed()) {
-      throw new IllegalStateException("Change" + change.getId() + " is closed");
+      throw new IllegalStateException("Change" + changeId + " is closed");
     }
 
     final List<PatchSetApproval> allApprovals =
@@ -94,10 +95,8 @@ class SubmitAction extends Handler<ChangeDetail> {
     final PatchSetApproval.Key ak =
         new PatchSetApproval.Key(patchSetId, user.getAccountId(), SUBMIT);
     PatchSetApproval myAction = null;
-    boolean isnew = true;
     for (final PatchSetApproval ca : allApprovals) {
       if (ak.equals(ca.getKey())) {
-        isnew = false;
         myAction = ca;
         myAction.setValue((short) 1);
         myAction.setGranted();
@@ -132,27 +131,23 @@ class SubmitAction extends Handler<ChangeDetail> {
           + " not permitted");
     }
 
-    if (change.getStatus() == Change.Status.NEW) {
+    db.patchSetApprovals().upsert(Collections.singleton(myAction));
-      change.setStatus(Change.Status.SUBMITTED);
-      ChangeUtil.updated(change);
-    }
 
-    final Transaction txn = db.beginTransaction();
+    change = db.changes().atomicUpdate(changeId, new AtomicUpdate<Change>() {
-    db.changes().update(Collections.singleton(change), txn);
+      @Override
-    if (change.getStatus().isClosed()) {
+      public Change update(Change change) {
-      db.patchSetApprovals().update(fs.getDirtyChangeApprovals(), txn);
+        if (change.getStatus() == Change.Status.NEW) {
-    }
+          change.setStatus(Change.Status.SUBMITTED);
-    if (isnew) {
+          ChangeUtil.updated(change);
-      db.patchSetApprovals().insert(Collections.singleton(myAction), txn);
+        }
-    } else {
+        return change;
-      db.patchSetApprovals().update(Collections.singleton(myAction), txn);
+      }
-    }
+    });
-    txn.commit();
 
     if (change.getStatus() == Change.Status.SUBMITTED) {
       merger.merge(change.getDest());
     }
 
-    return changeDetailFactory.create(change.getId()).call();
+    return changeDetailFactory.create(changeId).call();
   }
 }

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/AddReviewer.java

@@ -30,13 +30,11 @@ import com.google.gerrit.server.account.AccountResolver;
 import com.google.gerrit.server.mail.AddReviewerSender;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gwtorm.client.OrmException;
-import com.google.gwtorm.client.OrmRunnable;
-import com.google.gwtorm.client.Transaction;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 
+import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -114,22 +112,18 @@ class AddReviewer extends Handler<AddReviewerResult> {
     // Add the reviewers to the database
     //
     final Set<Account.Id> added = new HashSet<Account.Id>();
-    db.run(new OrmRunnable<Object, ReviewDb>() {
+    final List<PatchSetApproval> toInsert = new ArrayList<PatchSetApproval>();
-      public Object run(ReviewDb db, Transaction txn, boolean retry)
+    final PatchSet.Id psid = control.getChange().currentPatchSetId();
-          throws OrmException {
+    for (final Account.Id reviewer : reviewerIds) {
-        final PatchSet.Id psid = control.getChange().currentPatchSetId();
+      if (!exists(psid, reviewer)) {
-        for (final Account.Id reviewer : reviewerIds) {
+        // This reviewer has not entered an approval for this change yet.
-          if (!exists(psid, reviewer)) {
+        //
-            // This reviewer has not entered an approval for this change yet.
+        final PatchSetApproval myca = dummyApproval(psid, reviewer);
-            //
+        toInsert.add(myca);
-            final PatchSetApproval myca = dummyApproval(psid, reviewer);
+        added.add(reviewer);
-            db.patchSetApprovals().insert(Collections.singleton(myca), txn);
-            added.add(reviewer);
-          }
-        }
-        return null;
       }
-    });
+    }
+    db.patchSetApprovals().insert(toInsert);
 
     // Email the reviewers
     //

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java

@@ -17,7 +17,6 @@ package com.google.gerrit.httpd.rpc.patch;
 import com.google.gerrit.common.data.AddReviewerResult;
 import com.google.gerrit.common.data.ApprovalSummary;
 import com.google.gerrit.common.data.ApprovalSummarySet;
-import com.google.gerrit.common.data.ApprovalType;
 import com.google.gerrit.common.data.ApprovalTypes;
 import com.google.gerrit.common.data.CommentDetail;
 import com.google.gerrit.common.data.PatchDetailService;
@@ -25,39 +24,30 @@ import com.google.gerrit.common.data.PatchScript;
 import com.google.gerrit.common.data.PatchScriptSettings;
 import com.google.gerrit.common.errors.NoSuchEntityException;
 import com.google.gerrit.httpd.rpc.BaseServiceImplementation;
+import com.google.gerrit.httpd.rpc.Handler;
 import com.google.gerrit.reviewdb.Account;
 import com.google.gerrit.reviewdb.AccountPatchReview;
 import com.google.gerrit.reviewdb.ApprovalCategory;
 import com.google.gerrit.reviewdb.ApprovalCategoryValue;
 import com.google.gerrit.reviewdb.Change;
-import com.google.gerrit.reviewdb.ChangeMessage;
 import com.google.gerrit.reviewdb.Patch;
 import com.google.gerrit.reviewdb.PatchLineComment;
 import com.google.gerrit.reviewdb.PatchSet;
 import com.google.gerrit.reviewdb.PatchSetApproval;
 import com.google.gerrit.reviewdb.ReviewDb;
 import com.google.gerrit.reviewdb.Patch.Key;
-import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.account.AccountInfoCacheFactory;
-import com.google.gerrit.server.mail.CommentSender;
+import com.google.gerrit.server.patch.PublishComments;
-import com.google.gerrit.server.mail.EmailException;
-import com.google.gerrit.server.patch.PatchSetInfoFactory;
-import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.workflow.FunctionState;
 import com.google.gwt.user.client.rpc.AsyncCallback;
 import com.google.gwtjsonrpc.client.VoidResult;
 import com.google.gwtorm.client.OrmException;
-import com.google.gwtorm.client.OrmRunnable;
-import com.google.gwtorm.client.Transaction;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -66,9 +56,6 @@ import java.util.Set;
 
 class PatchDetailServiceImpl extends BaseServiceImplementation implements
     PatchDetailService {
-  private final Logger log = LoggerFactory.getLogger(getClass());
-  private final CommentSender.Factory commentSenderFactory;
-  private final PatchSetInfoFactory patchSetInfoFactory;
   private final ApprovalTypes approvalTypes;
 
   private final AccountInfoCacheFactory.Factory accountInfoCacheFactory;
@@ -76,14 +63,13 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
   private final ChangeControl.Factory changeControlFactory;
   private final CommentDetailFactory.Factory commentDetailFactory;
   private final FunctionState.Factory functionStateFactory;
+  private final PublishComments.Factory publishCommentsFactory;
   private final PatchScriptFactory.Factory patchScriptFactoryFactory;
   private final SaveDraft.Factory saveDraftFactory;
 
   @Inject
   PatchDetailServiceImpl(final Provider<ReviewDb> schema,
       final Provider<CurrentUser> currentUser,
-      final CommentSender.Factory commentSenderFactory,
-      final PatchSetInfoFactory patchSetInfoFactory,
       final ApprovalTypes approvalTypes,
       final AccountInfoCacheFactory.Factory accountInfoCacheFactory,
       final AddReviewer.Factory addReviewerFactory,
@@ -91,10 +77,9 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
       final CommentDetailFactory.Factory commentDetailFactory,
       final FunctionState.Factory functionStateFactory,
       final PatchScriptFactory.Factory patchScriptFactoryFactory,
+      final PublishComments.Factory publishCommentsFactory,
       final SaveDraft.Factory saveDraftFactory) {
     super(schema, currentUser);
-    this.patchSetInfoFactory = patchSetInfoFactory;
-    this.commentSenderFactory = commentSenderFactory;
     this.approvalTypes = approvalTypes;
 
     this.accountInfoCacheFactory = accountInfoCacheFactory;
@@ -103,6 +88,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
     this.commentDetailFactory = commentDetailFactory;
     this.functionStateFactory = functionStateFactory;
     this.patchScriptFactoryFactory = patchScriptFactoryFactory;
+    this.publishCommentsFactory = publishCommentsFactory;
     this.saveDraftFactory = saveDraftFactory;
   }
 
@@ -150,39 +136,10 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
     });
   }
 
-  public void publishComments(final PatchSet.Id psid, final String message,
+  public void publishComments(final PatchSet.Id psid, final String msg,
-      final Set<ApprovalCategoryValue.Id> approvals,
+      final Set<ApprovalCategoryValue.Id> tags,
-      final AsyncCallback<VoidResult> callback) {
+      final AsyncCallback<VoidResult> cb) {
-    run(callback, new Action<VoidResult>() {
+    Handler.wrap(publishCommentsFactory.create(psid, msg, tags)).to(cb);
-      public VoidResult run(ReviewDb db) throws OrmException, Failure {
-        final PublishResult r;
-
-        r = db.run(new OrmRunnable<PublishResult, ReviewDb>() {
-          public PublishResult run(ReviewDb db, Transaction txn, boolean retry)
-              throws OrmException {
-            return doPublishComments(psid, message, approvals, db, txn);
-          }
-        });
-
-        try {
-          final CommentSender cm;
-          cm = commentSenderFactory.create(r.change);
-          cm.setFrom(getAccountId());
-          cm.setPatchSet(r.patchSet, patchSetInfoFactory.get(psid));
-          cm.setChangeMessage(r.message);
-          cm.setPatchLineComments(r.comments);
-          cm.setReviewDb(db);
-          cm.send();
-        } catch (EmailException e) {
-          log.error("Cannot send comments by email for patch set " + psid, e);
-          throw new Failure(e);
-        } catch (PatchSetInfoNotAvailableException e) {
-          log.error("Failed to obtain PatchSetInfo for patch set " + psid, e);
-          throw new Failure(e);
-        }
-        return VoidResult.INSTANCE;
-      }
-    });
   }
 
   /**
@@ -207,108 +164,6 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
     });
   }
 
-
-  private static class PublishResult {
-    Change change;
-    PatchSet patchSet;
-    ChangeMessage message;
-    List<PatchLineComment> comments;
-  }
-
-  private PublishResult doPublishComments(final PatchSet.Id psid,
-      final String messageText, final Set<ApprovalCategoryValue.Id> approvals,
-      final ReviewDb db, final Transaction txn) throws OrmException {
-    final PublishResult r = new PublishResult();
-    final Account.Id me = getAccountId();
-    r.change = db.changes().get(psid.getParentKey());
-    r.patchSet = db.patchSets().get(psid);
-    if (r.change == null || r.patchSet == null) {
-      throw new OrmException(new NoSuchEntityException());
-    }
-
-    final boolean iscurrent = psid.equals(r.change.currentPatchSetId());
-    r.comments = db.patchComments().draft(psid, me).toList();
-    for (final PatchLineComment c : r.comments) {
-      c.setStatus(PatchLineComment.Status.PUBLISHED);
-      c.updated();
-    }
-    db.patchComments().update(r.comments, txn);
-
-    final StringBuilder msgbuf = new StringBuilder();
-    final Map<ApprovalCategory.Id, ApprovalCategoryValue.Id> values =
-        new HashMap<ApprovalCategory.Id, ApprovalCategoryValue.Id>();
-    for (final ApprovalCategoryValue.Id v : approvals) {
-      values.put(v.getParentKey(), v);
-    }
-
-    final boolean applyApprovals = iscurrent && r.change.getStatus().isOpen();
-    final Map<ApprovalCategory.Id, PatchSetApproval> have =
-        new HashMap<ApprovalCategory.Id, PatchSetApproval>();
-    for (PatchSetApproval a : db.patchSetApprovals().byPatchSetUser(psid, me)) {
-      have.put(a.getCategoryId(), a);
-    }
-    for (final ApprovalType at : approvalTypes.getApprovalTypes()) {
-      final ApprovalCategoryValue.Id v = values.get(at.getCategory().getId());
-      if (v == null) {
-        continue;
-      }
-
-      final ApprovalCategoryValue val = at.getValue(v.get());
-      if (val == null) {
-        continue;
-      }
-
-      PatchSetApproval mycatpp = have.remove(v.getParentKey());
-      if (mycatpp == null) {
-        if (msgbuf.length() > 0) {
-          msgbuf.append("; ");
-        }
-        msgbuf.append(val.getName());
-        if (applyApprovals) {
-          mycatpp =
-              new PatchSetApproval(new PatchSetApproval.Key(psid, me, v
-                  .getParentKey()), v.get());
-          db.patchSetApprovals().insert(Collections.singleton(mycatpp), txn);
-        }
-
-      } else if (mycatpp.getValue() != v.get()) {
-        if (msgbuf.length() > 0) {
-          msgbuf.append("; ");
-        }
-        msgbuf.append(val.getName());
-        if (applyApprovals) {
-          mycatpp.setValue(v.get());
-          mycatpp.setGranted();
-          db.patchSetApprovals().update(Collections.singleton(mycatpp), txn);
-        }
-      }
-    }
-    if (applyApprovals) {
-      db.patchSetApprovals().delete(have.values(), txn);
-    }
-
-    if (msgbuf.length() > 0) {
-      msgbuf.insert(0, "Patch Set " + psid.get() + ": ");
-      msgbuf.append("\n\n");
-    } else if (!iscurrent) {
-      msgbuf.append("Patch Set " + psid.get() + ":\n\n");
-    }
-    if (messageText != null) {
-      msgbuf.append(messageText);
-    }
-    if (msgbuf.length() > 0) {
-      r.message =
-          new ChangeMessage(new ChangeMessage.Key(r.change.getId(), ChangeUtil
-              .messageUUID(db)), me);
-      r.message.setMessage(msgbuf.toString());
-      db.changeMessages().insert(Collections.singleton(r.message), txn);
-    }
-
-    ChangeUtil.updated(r.change);
-    db.changes().update(Collections.singleton(r.change), txn);
-    return r;
-  }
-
   public void addReviewers(final Change.Id id, final List<String> reviewers,
       final AsyncCallback<AddReviewerResult> callback) {
     addReviewerFactory.create(id, reviewers).to(callback);
@@ -317,8 +172,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
   public void userApprovals(final Set<Change.Id> cids, final Account.Id aid,
       final AsyncCallback<ApprovalSummarySet> callback) {
     run(callback, new Action<ApprovalSummarySet>() {
-      public ApprovalSummarySet run(ReviewDb db)
+      public ApprovalSummarySet run(ReviewDb db) throws OrmException {
-        throws OrmException {
         final Map<Change.Id, ApprovalSummary> approvals =
             new HashMap<Change.Id, ApprovalSummary>();
         final AccountInfoCacheFactory aicFactory =
@@ -350,8 +204,9 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
 
             approvals.put(id, new ApprovalSummary(psas.values()));
           } catch (NoSuchChangeException nsce) {
-            /* The user has no access to see this change, so we
+            /*
-             * simply do not provide any details about it.
+             * The user has no access to see this change, so we simply do not
+             * provide any details about it.
              */
           }
         }
@@ -363,8 +218,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
   public void strongestApprovals(final Set<Change.Id> cids,
       final AsyncCallback<ApprovalSummarySet> callback) {
     run(callback, new Action<ApprovalSummarySet>() {
-      public ApprovalSummarySet run(ReviewDb db)
+      public ApprovalSummarySet run(ReviewDb db) throws OrmException {
-          throws OrmException {
         final Map<Change.Id, ApprovalSummary> approvals =
             new HashMap<Change.Id, ApprovalSummary>();
         final AccountInfoCacheFactory aicFactory =
@@ -380,8 +234,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
             final FunctionState fs =
                 functionStateFactory.create(change, ps_id, psas.values());
 
-            for (PatchSetApproval ca : db.patchSetApprovals()
+            for (PatchSetApproval ca : db.patchSetApprovals().byPatchSet(ps_id)) {
-                .byPatchSet(ps_id)) {
               final ApprovalCategory.Id category = ca.getCategoryId();
               if (change.getStatus().isOpen()) {
                 fs.normalize(approvalTypes.getApprovalType(category), ca);
@@ -394,9 +247,9 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
               if (psas.containsKey(category)) {
                 final short oldValue = psas.get(category).getValue();
                 final short newValue = ca.getValue();
-                keep = (Math.abs(oldValue) < Math.abs(newValue))
+                keep =
-                    || ((Math.abs(oldValue) == Math.abs(newValue)
+                    (Math.abs(oldValue) < Math.abs(newValue))
-                        && (newValue < oldValue)));
+                        || ((Math.abs(oldValue) == Math.abs(newValue) && (newValue < oldValue)));
               }
               if (keep) {
                 aicFactory.want(ca.getAccountId());
@@ -406,8 +259,9 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
 
             approvals.put(id, new ApprovalSummary(psas.values()));
           } catch (NoSuchChangeException nsce) {
-            /* The user has no access to see this change, so we
+            /*
-             * simply do not provide any details about it.
+             * The user has no access to see this change, so we simply do not
+             * provide any details about it.
              */
           }
         }

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/Change.java

@@ -436,8 +436,12 @@ public final class Change {
    * <p>
    * <b>Note: This makes the change dirty. Call update() after.</b>
    */
-  public PatchSet.Id newPatchSetId() {
+  public void nextPatchSetId() {
-    return new PatchSet.Id(changeId, ++nbrPatchSets);
+    ++nbrPatchSets;
+  }
+
+  public PatchSet.Id currPatchSetId() {
+    return new PatchSet.Id(changeId, nbrPatchSets);
   }
 
   public Status getStatus() {

gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java

@@ -16,11 +16,14 @@ package com.google.gerrit.server;
 
 import com.google.gerrit.reviewdb.Change;
 import com.google.gerrit.reviewdb.ReviewDb;
+import com.google.gwtorm.client.OrmConcurrencyException;
 import com.google.gwtorm.client.OrmException;
 
 import org.eclipse.jgit.util.Base64;
 import org.eclipse.jgit.util.NB;
 
+import java.util.Collections;
+
 public class ChangeUtil {
   private static int uuidPrefix;
   private static int uuidSeq;
@@ -49,6 +52,16 @@ public class ChangeUtil {
     NB.encodeInt32(raw, 4, uuidSeq--);
   }
 
+  public static void touch(final Change change, ReviewDb db)
+      throws OrmException {
+    try {
+      updated(change);
+      db.changes().update(Collections.singleton(change));
+    } catch (OrmConcurrencyException e) {
+      // Ignore a concurrent update, we just wanted to tag it as newer.
+    }
+  }
+
   public static void updated(final Change c) {
     c.resetLastUpdatedOn();
     computeSortKey(c);

gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java

@@ -27,7 +27,6 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.config.AuthConfig;
 import com.google.gwtorm.client.OrmException;
 import com.google.gwtorm.client.SchemaFactory;
-import com.google.gwtorm.client.Transaction;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 
@@ -131,13 +130,9 @@ public class AccountManager {
   }
 
   private void update(final ReviewDb db, final AuthRequest who,
-      final AccountExternalId extId) throws OrmException, AccountException {
+      final AccountExternalId extId) throws OrmException {
-    final Transaction txn = db.beginTransaction();
+    final IdentifiedUser user = userFactory.create(extId.getAccountId());
-    final Account account = db.accounts().get(extId.getAccountId());
+    Account toUpdate = null;
-    boolean updateAccount = false;
-    if (account == null) {
-      throw new AccountException("Account has been deleted");
-    }
 
     // If the email address was modified by the authentication provider,
     // update our records to match the changed email.
@@ -145,40 +140,51 @@ public class AccountManager {
     final String newEmail = who.getEmailAddress();
     final String oldEmail = extId.getEmailAddress();
     if (newEmail != null && !newEmail.equals(oldEmail)) {
-      if (oldEmail != null && oldEmail.equals(account.getPreferredEmail())) {
+      if (oldEmail != null
-        updateAccount = true;
+          && oldEmail.equals(user.getAccount().getPreferredEmail())) {
-        account.setPreferredEmail(newEmail);
+        toUpdate = load(toUpdate, user.getAccountId(), db);
+        toUpdate.setPreferredEmail(newEmail);
       }
 
       extId.setEmailAddress(newEmail);
+      db.accountExternalIds().update(Collections.singleton(extId));
     }
 
     if (!realm.allowsEdit(Account.FieldName.FULL_NAME)
-        && !eq(account.getFullName(), who.getDisplayName())) {
+        && !eq(user.getAccount().getFullName(), who.getDisplayName())) {
-      updateAccount = true;
+      toUpdate = load(toUpdate, user.getAccountId(), db);
-      account.setFullName(who.getDisplayName());
+      toUpdate.setFullName(who.getDisplayName());
-    }
-    if (!realm.allowsEdit(Account.FieldName.USER_NAME)
-        && !eq(account.getUserName(), who.getUserName())) {
-      updateAccount = true;
-      account.setUserName(who.getUserName());
     }
 
-    db.accountExternalIds().update(Collections.singleton(extId), txn);
+    if (!realm.allowsEdit(Account.FieldName.USER_NAME)
-    if (updateAccount) {
+        && !eq(user.getUserName(), who.getUserName())) {
-      db.accounts().update(Collections.singleton(account), txn);
+      changeUserNameFactory.create(db, user, who.getUserName());
+    }
+
+    if (toUpdate != null) {
+      db.accounts().update(Collections.singleton(toUpdate));
     }
-    txn.commit();
 
     if (newEmail != null && !newEmail.equals(oldEmail)) {
       byEmailCache.evict(oldEmail);
       byEmailCache.evict(newEmail);
     }
-    if (updateAccount) {
+    if (toUpdate != null) {
-      byIdCache.evict(account.getId());
+      byIdCache.evict(toUpdate.getId());
     }
   }
 
+  private Account load(Account toUpdate, Account.Id accountId, ReviewDb db)
+      throws OrmException {
+    if (toUpdate == null) {
+      toUpdate = db.accounts().get(accountId);
+      if (toUpdate == null) {
+        throw new OrmException("Account " + accountId + " has been deleted");
+      }
+    }
+    return toUpdate;
+  }
+
   private static boolean eq(final String a, final String b) {
     return (a == null && b == null) || (a != null && a.equals(b));
   }
@@ -223,10 +229,8 @@ public class AccountManager {
 
         if (openId.size() == 1) {
           final AccountExternalId oldId = openId.get(0);
-          final Transaction txn = db.beginTransaction();
+          db.accountExternalIds().upsert(Collections.singleton(newId));
-          db.accountExternalIds().delete(Collections.singleton(oldId), txn);
+          db.accountExternalIds().delete(Collections.singleton(oldId));
-          db.accountExternalIds().insert(Collections.singleton(newId), txn);
-          txn.commit();
         } else {
           db.accountExternalIds().insert(Collections.singleton(newId));
         }
@@ -241,10 +245,8 @@ public class AccountManager {
         final AccountExternalId newId = createId(oldId.getAccountId(), who);
         newId.setEmailAddress(who.getEmailAddress());
 
-        final Transaction txn = db.beginTransaction();
+        db.accountExternalIds().upsert(Collections.singleton(newId));
-        db.accountExternalIds().delete(Collections.singleton(oldId), txn);
+        db.accountExternalIds().delete(Collections.singleton(oldId));
-        db.accountExternalIds().insert(Collections.singleton(newId), txn);
-        txn.commit();
         return new AuthResult(newId.getAccountId(), newId.getKey(), false);
 
       } else if (v1.size() > 1) {
@@ -260,9 +262,8 @@ public class AccountManager {
     account.setFullName(who.getDisplayName());
     account.setPreferredEmail(extId.getEmailAddress());
 
-    final Transaction txn = db.beginTransaction();
+    db.accounts().insert(Collections.singleton(account));
-    db.accounts().insert(Collections.singleton(account), txn);
+    db.accountExternalIds().insert(Collections.singleton(extId));
-    db.accountExternalIds().insert(Collections.singleton(extId), txn);
 
     if (firstAccount.get() && firstAccount.compareAndSet(true, false)) {
       // This is the first user account on our site. Assume this user
@@ -272,13 +273,11 @@ public class AccountManager {
       final AccountGroup.Id admin = authConfig.getAdministratorsGroup();
       final AccountGroupMember m =
           new AccountGroupMember(new AccountGroupMember.Key(newId, admin));
-      db.accountGroupMembers().insert(Collections.singleton(m), txn);
       db.accountGroupMembersAudit().insert(
-          Collections.singleton(new AccountGroupMemberAudit(m, newId)), txn);
+          Collections.singleton(new AccountGroupMemberAudit(m, newId)));
+      db.accountGroupMembers().insert(Collections.singleton(m));
     }
 
-    txn.commit();
-
     if (who.getUserName() != null) {
       // Only set if the name hasn't been used yet, but was given to us.
       //
@@ -330,19 +329,17 @@ public class AccountManager {
           update(db, who, extId);
 
         } else {
-          final Transaction txn = db.beginTransaction();
           extId = createId(to, who);
           extId.setEmailAddress(who.getEmailAddress());
-          db.accountExternalIds().insert(Collections.singleton(extId), txn);
+          db.accountExternalIds().insert(Collections.singleton(extId));
 
           if (who.getEmailAddress() != null) {
             final Account a = db.accounts().get(to);
             if (a.getPreferredEmail() == null) {
               a.setPreferredEmail(who.getEmailAddress());
-              db.accounts().update(Collections.singleton(a), txn);
+              db.accounts().update(Collections.singleton(a));
             }
           }
-          txn.commit();
 
           if (who.getEmailAddress() != null) {
             byEmailCache.evict(who.getEmailAddress());

gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java

@@ -117,11 +117,9 @@ public class ChangeUserName implements Callable<VoidResult> {
 
     // If we have any older user names, remove them.
     //
-    if (!old.isEmpty()) {
+    db.accountExternalIds().delete(old);
-      db.accountExternalIds().delete(old);
+    for (AccountExternalId i : old) {
-      for (AccountExternalId i : old) {
+      sshKeyCache.evict(i.getSchemeRest());
-        sshKeyCache.evict(i.getSchemeRest());
-      }
     }
 
     accountCache.evict(user.getAccountId());

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -37,11 +37,10 @@ import com.google.gerrit.server.account.Realm;
 import com.google.gerrit.server.auth.ldap.LdapModule;
 import com.google.gerrit.server.cache.CachePool;
 import com.google.gerrit.server.git.ChangeMergeQueue;
-import com.google.gerrit.server.git.LocalDiskRepositoryManager;
 import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.LocalDiskRepositoryManager;
 import com.google.gerrit.server.git.MergeOp;
 import com.google.gerrit.server.git.MergeQueue;
-import com.google.gerrit.server.git.PatchSetImporter;
 import com.google.gerrit.server.git.PushAllProjectsOp;
 import com.google.gerrit.server.git.PushReplication;
 import com.google.gerrit.server.git.ReloadSubmitQueueOp;
@@ -127,7 +126,6 @@ public class GerritGlobalModule extends FactoryModule {
         FromAddressGeneratorProvider.class).in(SINGLETON);
     bind(EmailSender.class).to(SmtpEmailSender.class).in(SINGLETON);
 
-    factory(PatchSetImporter.Factory.class);
     bind(PatchSetInfoFactory.class);
     bind(IdentifiedUser.GenericFactory.class).in(SINGLETON);
     factory(FunctionState.Factory.class);

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java

@@ -24,6 +24,7 @@ import com.google.gerrit.server.account.GroupControl;
 import com.google.gerrit.server.mail.AddReviewerSender;
 import com.google.gerrit.server.mail.CreateChangeSender;
 import com.google.gerrit.server.mail.ReplacePatchSetSender;
+import com.google.gerrit.server.patch.PublishComments;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.inject.servlet.RequestScoped;
@@ -47,6 +48,7 @@ public class GerritRequestModule extends FactoryModule {
     //
     factory(AddReviewerSender.Factory.class);
     factory(CreateChangeSender.Factory.class);
+    factory(PublishComments.Factory.class);
     factory(ReplacePatchSetSender.Factory.class);
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java

@@ -43,15 +43,16 @@ import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.workflow.CategoryFunction;
 import com.google.gerrit.server.workflow.FunctionState;
+import com.google.gwtorm.client.AtomicUpdate;
+import com.google.gwtorm.client.OrmConcurrencyException;
 import com.google.gwtorm.client.OrmException;
+import com.google.gwtorm.client.OrmRunnable;
 import com.google.gwtorm.client.SchemaFactory;
 import com.google.gwtorm.client.Transaction;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.assistedinject.Assisted;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
@@ -72,6 +73,8 @@ import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevFlag;
 import org.eclipse.jgit.revwalk.RevSort;
 import org.eclipse.jgit.revwalk.RevWalk;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -1066,52 +1069,14 @@ public class MergeOp {
   }
 
   private void setMerged(Change c, ChangeMessage msg) {
+    final Change.Id changeId = c.getId();
     final PatchSet.Id merged = c.currentPatchSetId();
-    PatchSetApproval submitter = null;
-    for (int attempts = 0; attempts < 10; attempts++) {
-      c.setStatus(Change.Status.MERGED);
-      ChangeUtil.updated(c);
-      try {
-        final Transaction txn = schema.beginTransaction();
 
-        // Flatten out all existing approvals based upon the current
+    try {
-        // permissions. Once the change is closed the approvals are
+      schema.changes().atomicUpdate(changeId, new AtomicUpdate<Change>() {
-        // not updated at presentation view time, so we need to make.
+        @Override
-        // sure they are accurate now. This way if permissions get
+        public Change update(Change c) {
-        // modified in the future, historical records stay accurate.
+          c.setStatus(Change.Status.MERGED);
-        //
-        final List<PatchSetApproval> approvals =
-            schema.patchSetApprovals().byChange(c.getId()).toList();
-        final FunctionState fs = functionState.create(c, merged, approvals);
-        for (ApprovalType at : approvalTypes.getApprovalTypes()) {
-          CategoryFunction.forCategory(at.getCategory()).run(at, fs);
-        }
-        for (PatchSetApproval a : approvals) {
-          if (a.getValue() > 0
-              && ApprovalCategory.SUBMIT.equals(a.getCategoryId())
-              && a.getPatchSetId().equals(merged)) {
-            if (submitter == null
-                || a.getGranted().compareTo(submitter.getGranted()) > 0) {
-              submitter = a;
-            }
-          }
-          a.cache(c);
-        }
-        schema.patchSetApprovals().update(approvals, txn);
-
-        if (msg != null) {
-          if (submitter != null && msg.getAuthor() == null) {
-            msg.setAuthor(submitter.getAccountId());
-          }
-          schema.changeMessages().insert(Collections.singleton(msg), txn);
-        }
-        schema.changes().update(Collections.singleton(c), txn);
-        txn.commit();
-        break;
-      } catch (OrmException e) {
-        final Change.Id id = c.getId();
-        try {
-          c = schema.changes().get(id);
           if (!merged.equals(c.currentPatchSetId())) {
             // Uncool; the patch set changed after we merged it.
             // Go back to the patch set that was actually merged.
@@ -1122,9 +1087,54 @@ public class MergeOp {
               log.error("Cannot read merged patch set " + merged, e1);
             }
           }
-        } catch (OrmException e2) {
+          ChangeUtil.updated(c);
-          log.error("Cannot set change " + id + " to merged " + merged, e2);
+          return c;
         }
+      });
+    } catch (OrmConcurrencyException err) {
+    } catch (OrmException err) {
+      log.warn("Cannot update change status", err);
+    }
+
+    // Flatten out all existing approvals based upon the current
+    // permissions. Once the change is closed the approvals are
+    // not updated at presentation view time, so we need to make.
+    // sure they are accurate now. This way if permissions get
+    // modified in the future, historical records stay accurate.
+    //
+    PatchSetApproval submitter = null;
+    try {
+      c.setStatus(Change.Status.MERGED);
+      final List<PatchSetApproval> approvals =
+          schema.patchSetApprovals().byChange(changeId).toList();
+      final FunctionState fs = functionState.create(c, merged, approvals);
+      for (ApprovalType at : approvalTypes.getApprovalTypes()) {
+        CategoryFunction.forCategory(at.getCategory()).run(at, fs);
+      }
+      for (PatchSetApproval a : approvals) {
+        if (a.getValue() > 0
+            && ApprovalCategory.SUBMIT.equals(a.getCategoryId())
+            && a.getPatchSetId().equals(merged)) {
+          if (submitter == null
+              || a.getGranted().compareTo(submitter.getGranted()) > 0) {
+            submitter = a;
+          }
+        }
+        a.cache(c);
+      }
+      schema.patchSetApprovals().update(approvals);
+    } catch (OrmException err) {
+      log.warn("Cannot normalize approvals for change " + changeId, err);
+    }
+
+    if (msg != null) {
+      if (submitter != null && msg.getAuthor() == null) {
+        msg.setAuthor(submitter.getAccountId());
+      }
+      try {
+        schema.changeMessages().insert(Collections.singleton(msg));
+      } catch (OrmException err) {
+        log.warn("Cannot store message on change", err);
       }
     }
 
@@ -1147,31 +1157,34 @@ public class MergeOp {
     sendMergeFail(c, msg, true);
   }
 
-  private void sendMergeFail(Change c, ChangeMessage msg, boolean makeNew) {
+  private void sendMergeFail(Change c, ChangeMessage msg, final boolean makeNew) {
-    for (int attempts = 0; attempts < 10; attempts++) {
+    try {
-      if (makeNew) {
+      schema.changeMessages().insert(Collections.singleton(msg));
-        c.setStatus(Change.Status.NEW);
+    } catch (OrmException err) {
-      }
+      log.warn("Cannot record merge failure message", err);
-      ChangeUtil.updated(c);
+    }
+
+    if (makeNew) {
       try {
-        final Transaction txn = schema.beginTransaction();
+        schema.changes().atomicUpdate(c.getId(), new AtomicUpdate<Change>() {
-        schema.changes().update(Collections.singleton(c), txn);
+          @Override
-        if (msg != null) {
+          public Change update(Change c) {
-          schema.changeMessages().insert(Collections.singleton(msg), txn);
+            if (c.getStatus().isOpen()) {
-        }
+              c.setStatus(Change.Status.NEW);
-        txn.commit();
+              ChangeUtil.updated(c);
-        break;
+            }
-      } catch (OrmException e) {
+            return c;
-        try {
-          c = schema.changes().get(c.getId());
-          if (c.getStatus().isClosed()) {
-            // Someone else marked it close while we noticed a failure.
-            // That's fine, leave it closed.
-            //
-            break;
           }
-        } catch (OrmException e2) {
+        });
-        }
+      } catch (OrmConcurrencyException err) {
+      } catch (OrmException err) {
+        log.warn("Cannot update change status", err);
+      }
+    } else {
+      try {
+        ChangeUtil.touch(c, schema);
+      } catch (OrmException err) {
+        log.warn("Cannot update change timestamp", err);
       }
     }
 

gerrit-server/src/main/java/com/google/gerrit/server/git/PatchSetImporter.java

@@ -1,127 +0,0 @@
-// Copyright (C) 2008 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.git;
-
-import com.google.gerrit.reviewdb.PatchSet;
-import com.google.gerrit.reviewdb.PatchSetAncestor;
-import com.google.gerrit.reviewdb.PatchSetInfo;
-import com.google.gerrit.reviewdb.RevId;
-import com.google.gerrit.reviewdb.ReviewDb;
-import com.google.gerrit.server.patch.PatchSetInfoFactory;
-import com.google.gwtorm.client.OrmException;
-import com.google.gwtorm.client.Transaction;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
-
-import org.eclipse.jgit.lib.Commit;
-import org.eclipse.jgit.revwalk.RevCommit;
-
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-/** Imports a {@link PatchSet} from a {@link Commit}. */
-public class PatchSetImporter {
-  public interface Factory {
-    PatchSetImporter create(ReviewDb dstDb, RevCommit srcCommit,
-        PatchSet dstPatchSet, boolean isNewPatchSet);
-  }
-
-  private final PatchSetInfoFactory patchSetInfoFactory;
-  private final ReviewDb db;
-  private final RevCommit src;
-  private final PatchSet dst;
-  private final boolean isNew;
-  private Transaction txn;
-
-  private PatchSetInfo info;
-
-  private final Map<Integer, PatchSetAncestor> ancestorExisting =
-      new HashMap<Integer, PatchSetAncestor>();
-  private final List<PatchSetAncestor> ancestorInsert =
-      new ArrayList<PatchSetAncestor>();
-  private final List<PatchSetAncestor> ancestorUpdate =
-      new ArrayList<PatchSetAncestor>();
-
-  @Inject
-  PatchSetImporter(final PatchSetInfoFactory psif,
-      @Assisted final ReviewDb dstDb, @Assisted final RevCommit srcCommit,
-      @Assisted final PatchSet dstPatchSet,
-      @Assisted final boolean isNewPatchSet) {
-    patchSetInfoFactory = psif;
-    db = dstDb;
-    src = srcCommit;
-    dst = dstPatchSet;
-    isNew = isNewPatchSet;
-  }
-
-  public void setTransaction(final Transaction t) {
-    txn = t;
-  }
-
-  public PatchSetInfo getPatchSetInfo() {
-    return info;
-  }
-
-  public void run() throws OrmException {
-    dst.setRevision(toRevId(src));
-
-    if (!isNew) {
-      for (final PatchSetAncestor a : db.patchSetAncestors().ancestorsOf(
-          dst.getId())) {
-        ancestorExisting.put(a.getPosition(), a);
-      }
-    }
-
-    info = patchSetInfoFactory.get(src, dst.getId());
-    importAncestors();
-
-    final boolean auto = txn == null;
-    if (auto) {
-      txn = db.beginTransaction();
-    }
-    if (isNew) {
-      db.patchSets().insert(Collections.singleton(dst), txn);
-    }
-    db.patchSetAncestors().insert(ancestorInsert, txn);
-    if (!isNew) {
-      db.patchSetAncestors().update(ancestorUpdate, txn);
-      db.patchSetAncestors().delete(ancestorExisting.values(), txn);
-    }
-    if (auto) {
-      txn.commit();
-      txn = null;
-    }
-  }
-
-  private void importAncestors() {
-    for (int p = 0; p < src.getParentCount(); p++) {
-      PatchSetAncestor a = ancestorExisting.remove(p + 1);
-      if (a == null) {
-        a = new PatchSetAncestor(new PatchSetAncestor.Id(dst.getId(), p + 1));
-        ancestorInsert.add(a);
-      } else {
-        ancestorUpdate.add(a);
-      }
-      a.setAncestorRevision(toRevId(src.getParent(p)));
-    }
-  }
-
-  private static RevId toRevId(final RevCommit src) {
-    return new RevId(src.getId().name());
-  }
-}

gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java

@@ -0,0 +1,281 @@
+// Copyright (C) 2009 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.patch;
+
+import com.google.gerrit.common.data.ApprovalType;
+import com.google.gerrit.common.data.ApprovalTypes;
+import com.google.gerrit.reviewdb.ApprovalCategory;
+import com.google.gerrit.reviewdb.ApprovalCategoryValue;
+import com.google.gerrit.reviewdb.Change;
+import com.google.gerrit.reviewdb.ChangeMessage;
+import com.google.gerrit.reviewdb.PatchLineComment;
+import com.google.gerrit.reviewdb.PatchSet;
+import com.google.gerrit.reviewdb.PatchSetApproval;
+import com.google.gerrit.reviewdb.ReviewDb;
+import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.mail.CommentSender;
+import com.google.gerrit.server.mail.EmailException;
+import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.workflow.FunctionState;
+import com.google.gwtjsonrpc.client.VoidResult;
+import com.google.gwtorm.client.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Callable;
+
+public class PublishComments implements Callable<VoidResult> {
+  private static final Logger log =
+      LoggerFactory.getLogger(PublishComments.class);
+
+  public interface Factory {
+    PublishComments create(PatchSet.Id patchSetId, String messageText,
+        Set<ApprovalCategoryValue.Id> approvals);
+  }
+
+  private final ReviewDb db;
+  private final IdentifiedUser user;
+  private final ApprovalTypes types;
+  private final CommentSender.Factory commentSenderFactory;
+  private final PatchSetInfoFactory patchSetInfoFactory;
+  private final ChangeControl.Factory changeControlFactory;
+  private final FunctionState.Factory functionStateFactory;
+
+  private final PatchSet.Id patchSetId;
+  private final String messageText;
+  private final Set<ApprovalCategoryValue.Id> approvals;
+
+  private Change change;
+  private PatchSet patchSet;
+  private ChangeMessage message;
+  private List<PatchLineComment> drafts;
+
+  @Inject
+  PublishComments(final ReviewDb db, final IdentifiedUser user,
+      final ApprovalTypes approvalTypes,
+      final CommentSender.Factory commentSenderFactory,
+      final PatchSetInfoFactory patchSetInfoFactory,
+      final ChangeControl.Factory changeControlFactory,
+      final FunctionState.Factory functionStateFactory,
+
+      @Assisted final PatchSet.Id patchSetId,
+      @Assisted final String messageText,
+      @Assisted final Set<ApprovalCategoryValue.Id> approvals) {
+    this.db = db;
+    this.user = user;
+    this.types = approvalTypes;
+    this.patchSetInfoFactory = patchSetInfoFactory;
+    this.commentSenderFactory = commentSenderFactory;
+    this.changeControlFactory = changeControlFactory;
+    this.functionStateFactory = functionStateFactory;
+
+    this.patchSetId = patchSetId;
+    this.messageText = messageText;
+    this.approvals = approvals;
+  }
+
+  @Override
+  public VoidResult call() throws NoSuchChangeException, OrmException {
+    final Change.Id changeId = patchSetId.getParentKey();
+    final ChangeControl ctl = changeControlFactory.validateFor(changeId);
+    change = ctl.getChange();
+    patchSet = db.patchSets().get(patchSetId);
+    if (patchSet == null) {
+      throw new NoSuchChangeException(changeId);
+    }
+    drafts = drafts();
+
+    publishDrafts();
+
+    final boolean isCurrent = patchSetId.equals(change.currentPatchSetId());
+    if (isCurrent && change.getStatus().isOpen()) {
+      publishApprovals();
+    } else {
+      publishMessageOnly();
+    }
+
+    touchChange();
+    email();
+    return VoidResult.INSTANCE;
+  }
+
+  private void publishDrafts() throws OrmException {
+    for (final PatchLineComment c : drafts) {
+      c.setStatus(PatchLineComment.Status.PUBLISHED);
+      c.updated();
+    }
+    db.patchComments().update(drafts);
+  }
+
+  private void publishApprovals() throws OrmException {
+    ChangeUtil.updated(change);
+
+    final Set<ApprovalCategory.Id> dirty = new HashSet<ApprovalCategory.Id>();
+    final List<PatchSetApproval> ins = new ArrayList<PatchSetApproval>();
+    final List<PatchSetApproval> upd = new ArrayList<PatchSetApproval>();
+    final Collection<PatchSetApproval> all =
+        db.patchSetApprovals().byPatchSet(patchSetId).toList();
+    final Map<ApprovalCategory.Id, PatchSetApproval> mine = mine(all);
+
+    // Ensure any new approvals are stored properly.
+    //
+    for (final ApprovalCategoryValue.Id want : approvals) {
+      PatchSetApproval a = mine.get(want.getParentKey());
+      if (a == null) {
+        a = new PatchSetApproval(new PatchSetApproval.Key(//
+            patchSetId, user.getAccountId(), want.getParentKey()), want.get());
+        a.cache(change);
+        ins.add(a);
+        all.add(a);
+        mine.put(a.getCategoryId(), a);
+        dirty.add(a.getCategoryId());
+      }
+    }
+
+    // Normalize all of the items the user is changing.
+    //
+    final FunctionState functionState =
+        functionStateFactory.create(change, patchSetId, all);
+    for (final ApprovalCategoryValue.Id want : approvals) {
+      final PatchSetApproval a = mine.get(want.getParentKey());
+      final short o = a.getValue();
+      a.setValue(want.get());
+      a.cache(change);
+      functionState.normalize(types.getApprovalType(a.getCategoryId()), a);
+      if (o != a.getValue()) {
+        // Value changed, ensure we update the database.
+        //
+        a.setGranted();
+        dirty.add(a.getCategoryId());
+      }
+      if (!ins.contains(a)) {
+        upd.add(a);
+      }
+    }
+
+    // Format a message explaining the actions taken.
+    //
+    final StringBuilder msgbuf = new StringBuilder();
+    for (final ApprovalType at : types.getApprovalTypes()) {
+      if (dirty.contains(at.getCategory().getId())) {
+        final PatchSetApproval a = mine.get(at.getCategory().getId());
+        final ApprovalCategoryValue val = at.getValue(a);
+        if (msgbuf.length() > 0) {
+          msgbuf.append("; ");
+        }
+        if (val != null && val.getName() != null && !val.getName().isEmpty()) {
+          msgbuf.append(val.getName());
+        } else {
+          msgbuf.append(at.getCategory().getName());
+          msgbuf.append(" ");
+          if (a.getValue() > 0) msgbuf.append('+');
+          msgbuf.append(a.getValue());
+        }
+      }
+    }
+
+    // Update dashboards for everyone else.
+    //
+    for (PatchSetApproval a : all) {
+      if (!user.getAccountId().equals(a.getAccountId())) {
+        a.cache(change);
+        upd.add(a);
+      }
+    }
+
+    db.patchSetApprovals().update(upd);
+    db.patchSetApprovals().insert(ins);
+    message(msgbuf.toString());
+  }
+
+  private void publishMessageOnly() throws OrmException {
+    message(null);
+  }
+
+  private void message(String actions) throws OrmException {
+    if ((actions == null || actions.isEmpty())
+        && (messageText == null || messageText.isEmpty())) {
+      // They had nothing to say?
+      //
+      return;
+    }
+
+    final StringBuilder msgbuf = new StringBuilder();
+    msgbuf.append("Patch Set " + patchSetId.get() + ":");
+    if (actions != null && !actions.isEmpty()) {
+      msgbuf.append(" ");
+      msgbuf.append(actions);
+    }
+    msgbuf.append("\n\n");
+    msgbuf.append(messageText != null ? messageText : "");
+
+    message = new ChangeMessage(new ChangeMessage.Key(change.getId(),//
+        ChangeUtil.messageUUID(db)), user.getAccountId());
+    message.setMessage(msgbuf.toString());
+    db.changeMessages().insert(Collections.singleton(message));
+  }
+
+  private Map<ApprovalCategory.Id, PatchSetApproval> mine(
+      Collection<PatchSetApproval> all) {
+    Map<ApprovalCategory.Id, PatchSetApproval> r =
+        new HashMap<ApprovalCategory.Id, PatchSetApproval>();
+    for (PatchSetApproval a : all) {
+      if (user.getAccountId().equals(a.getAccountId())) {
+        r.put(a.getCategoryId(), a);
+      }
+    }
+    return r;
+  }
+
+  private void touchChange() {
+    try {
+      ChangeUtil.touch(change, db);
+    } catch (OrmException e) {
+    }
+  }
+
+  private List<PatchLineComment> drafts() throws OrmException {
+    return db.patchComments().draft(patchSetId, user.getAccountId()).toList();
+  }
+
+  private void email() {
+    try {
+      final CommentSender cm = commentSenderFactory.create(change);
+      cm.setFrom(user.getAccountId());
+      cm.setPatchSet(patchSet, patchSetInfoFactory.get(patchSetId));
+      cm.setChangeMessage(message);
+      cm.setPatchLineComments(drafts);
+      cm.setReviewDb(db);
+      cm.send();
+    } catch (EmailException e) {
+      log.error("Cannot send comments by email for patch set " + patchSetId, e);
+    } catch (PatchSetInfoNotAvailableException e) {
+      log.error("Failed to obtain PatchSetInfo for patch set " + patchSetId, e);
+    }
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaCreator.java

@@ -28,7 +28,6 @@ import com.google.gerrit.server.workflow.NoOpFunction;
 import com.google.gerrit.server.workflow.SubmitFunction;
 import com.google.gwtjsonrpc.server.SignedToken;
 import com.google.gwtorm.client.OrmException;
-import com.google.gwtorm.client.Transaction;
 import com.google.gwtorm.jdbc.JdbcExecutor;
 import com.google.gwtorm.jdbc.JdbcSchema;
 import com.google.gwtorm.schema.sql.DialectH2;
@@ -158,7 +157,6 @@ public class SchemaCreator {
   }
 
   private void initVerifiedCategory(final ReviewDb c) throws OrmException {
-    final Transaction txn = c.beginTransaction();
     final ApprovalCategory cat;
     final ArrayList<ApprovalCategoryValue> vals;
 
@@ -169,14 +167,12 @@ public class SchemaCreator {
     vals.add(value(cat, 1, "Verified"));
     vals.add(value(cat, 0, "No score"));
     vals.add(value(cat, -1, "Fails"));
-    c.approvalCategories().insert(Collections.singleton(cat), txn);
+    c.approvalCategories().insert(Collections.singleton(cat));
-    c.approvalCategoryValues().insert(vals, txn);
+    c.approvalCategoryValues().insert(vals);
-    txn.commit();
   }
 
   private void initCodeReviewCategory(final ReviewDb c,
       final SystemConfig sConfig) throws OrmException {
-    final Transaction txn = c.beginTransaction();
     final ApprovalCategory cat;
     final ArrayList<ApprovalCategoryValue> vals;
 
@@ -190,9 +186,8 @@ public class SchemaCreator {
     vals.add(value(cat, 0, "No score"));
     vals.add(value(cat, -1, "I would prefer that you didn't submit this"));
     vals.add(value(cat, -2, "Do not submit"));
-    c.approvalCategories().insert(Collections.singleton(cat), txn);
+    c.approvalCategories().insert(Collections.singleton(cat));
-    c.approvalCategoryValues().insert(vals, txn);
+    c.approvalCategoryValues().insert(vals);
-    txn.commit();
 
     final ProjectRight approve =
         new ProjectRight(new ProjectRight.Key(DEFAULT_WILD_NAME, cat.getId(),
@@ -203,7 +198,6 @@ public class SchemaCreator {
   }
 
   private void initOwnerCategory(final ReviewDb c) throws OrmException {
-    final Transaction txn = c.beginTransaction();
     final ApprovalCategory cat;
     final ArrayList<ApprovalCategoryValue> vals;
 
@@ -212,14 +206,12 @@ public class SchemaCreator {
     cat.setFunctionName(NoOpFunction.NAME);
     vals = new ArrayList<ApprovalCategoryValue>();
     vals.add(value(cat, 1, "Administer All Settings"));
-    c.approvalCategories().insert(Collections.singleton(cat), txn);
+    c.approvalCategories().insert(Collections.singleton(cat));
-    c.approvalCategoryValues().insert(vals, txn);
+    c.approvalCategoryValues().insert(vals);
-    txn.commit();
   }
 
   private void initReadCategory(final ReviewDb c, final SystemConfig sConfig)
       throws OrmException {
-    final Transaction txn = c.beginTransaction();
     final ApprovalCategory cat;
     final ArrayList<ApprovalCategoryValue> vals;
 
@@ -230,9 +222,9 @@ public class SchemaCreator {
     vals.add(value(cat, 2, "Upload permission"));
     vals.add(value(cat, 1, "Read access"));
     vals.add(value(cat, -1, "No access"));
-    c.approvalCategories().insert(Collections.singleton(cat), txn);
+    c.approvalCategories().insert(Collections.singleton(cat));
-    c.approvalCategoryValues().insert(vals, txn);
+    c.approvalCategoryValues().insert(vals);
-    txn.commit();
+
     {
       final ProjectRight read =
           new ProjectRight(new ProjectRight.Key(DEFAULT_WILD_NAME, cat.getId(),
@@ -260,7 +252,6 @@ public class SchemaCreator {
   }
 
   private void initSubmitCategory(final ReviewDb c) throws OrmException {
-    final Transaction txn = c.beginTransaction();
     final ApprovalCategory cat;
     final ArrayList<ApprovalCategoryValue> vals;
 
@@ -269,13 +260,11 @@ public class SchemaCreator {
     cat.setFunctionName(SubmitFunction.NAME);
     vals = new ArrayList<ApprovalCategoryValue>();
     vals.add(value(cat, 1, "Submit"));
-    c.approvalCategories().insert(Collections.singleton(cat), txn);
+    c.approvalCategories().insert(Collections.singleton(cat));
-    c.approvalCategoryValues().insert(vals, txn);
+    c.approvalCategoryValues().insert(vals);
-    txn.commit();
   }
 
   private void initPushTagCategory(final ReviewDb c) throws OrmException {
-    final Transaction txn = c.beginTransaction();
     final ApprovalCategory cat;
     final ArrayList<ApprovalCategoryValue> vals;
 
@@ -287,14 +276,12 @@ public class SchemaCreator {
     vals.add(value(cat, ApprovalCategory.PUSH_TAG_ANNOTATED,
         "Create Annotated Tag"));
     vals.add(value(cat, ApprovalCategory.PUSH_TAG_ANY, "Create Any Tag"));
-    c.approvalCategories().insert(Collections.singleton(cat), txn);
+    c.approvalCategories().insert(Collections.singleton(cat));
-    c.approvalCategoryValues().insert(vals, txn);
+    c.approvalCategoryValues().insert(vals);
-    txn.commit();
   }
 
   private void initPushUpdateBranchCategory(final ReviewDb c)
       throws OrmException {
-    final Transaction txn = c.beginTransaction();
     final ApprovalCategory cat;
     final ArrayList<ApprovalCategoryValue> vals;
 
@@ -306,9 +293,8 @@ public class SchemaCreator {
     vals.add(value(cat, ApprovalCategory.PUSH_HEAD_CREATE, "Create Branch"));
     vals.add(value(cat, ApprovalCategory.PUSH_HEAD_REPLACE,
         "Force Push Branch; Delete Branch"));
-    c.approvalCategories().insert(Collections.singleton(cat), txn);
+    c.approvalCategories().insert(Collections.singleton(cat));
-    c.approvalCategoryValues().insert(vals, txn);
+    c.approvalCategoryValues().insert(vals);
-    txn.commit();
   }
 
   private static ApprovalCategoryValue value(final ApprovalCategory cat,

gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_22.java

@@ -21,7 +21,6 @@ import com.google.gerrit.reviewdb.AccountExternalId;
 import com.google.gerrit.reviewdb.ReviewDb;
 import com.google.gerrit.reviewdb.AccountExternalId.Key;
 import com.google.gwtorm.client.OrmException;
-import com.google.gwtorm.client.Transaction;
 import com.google.gwtorm.jdbc.JdbcSchema;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -60,11 +59,7 @@ class Schema_22 extends SchemaVersion {
     } finally {
       queryStmt.close();
     }
-    if (!ids.isEmpty()) {
+    db.accountExternalIds().insert(ids);
-      Transaction t = db.beginTransaction();
-      db.accountExternalIds().insert(ids, t);
-      t.commit();
-    }
   }
 
   private Key toKey(final String userName) {

gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_23.java

@@ -18,7 +18,6 @@ import com.google.gerrit.reviewdb.AccountGroup;
 import com.google.gerrit.reviewdb.AccountGroupName;
 import com.google.gerrit.reviewdb.ReviewDb;
 import com.google.gwtorm.client.OrmException;
-import com.google.gwtorm.client.Transaction;
 import com.google.gwtorm.jdbc.JdbcSchema;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -53,11 +52,7 @@ class Schema_23 extends SchemaVersion {
     } finally {
       queryStmt.close();
     }
-    if (!names.isEmpty()) {
+    db.accountGroupNames().insert(names);
-      Transaction t = db.beginTransaction();
-      db.accountGroupNames().insert(names, t);
-      t.commit();
-    }
   }
 
   private AccountGroup.NameKey toKey(final String name) {

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminCreateProject.java

@@ -26,7 +26,6 @@ import com.google.gerrit.server.git.ReplicationQueue;
 import com.google.gerrit.sshd.AdminCommand;
 import com.google.gerrit.sshd.BaseCommand;
 import com.google.gwtorm.client.OrmException;
-import com.google.gwtorm.client.Transaction;
 import com.google.inject.Inject;
 
 import org.apache.sshd.server.Environment;
@@ -89,16 +88,12 @@ final class AdminCreateProject extends BaseCommand {
         try {
           validateParameters();
 
-          Transaction txn = db.beginTransaction();
-
-          createProject(txn);
-
           Repository repo = repoManager.createRepository(projectName);
           repo.create(true);
           repo.writeSymref(Constants.HEAD, branch);
           repoManager.setProjectDescription(projectName, projectDescription);
 
-          txn.commit();
+          createProject();
 
           rq.replicateNewProject(new Project.NameKey(projectName), branch);
         } catch (Exception e) {
@@ -111,23 +106,23 @@ final class AdminCreateProject extends BaseCommand {
     });
   }
 
-  private void createProject(Transaction txn) throws OrmException {
+  private void createProject() throws OrmException {
     final Project.NameKey newProjectNameKey = new Project.NameKey(projectName);
-    final Project newProject = new Project(newProjectNameKey);
-
-    newProject.setDescription(projectDescription);
-    newProject.setSubmitType(submitType);
-    newProject.setUseContributorAgreements(contributorAgreements);
-    newProject.setUseSignedOffBy(signedOffBy);
-
-    db.projects().insert(Collections.singleton(newProject), txn);
 
     final ProjectRight.Key prk =
         new ProjectRight.Key(newProjectNameKey, ApprovalCategory.OWN, ownerId);
     final ProjectRight pr = new ProjectRight(prk);
     pr.setMaxValue((short) 1);
     pr.setMinValue((short) 1);
-    db.projectRights().insert(Collections.singleton(pr), txn);
+    db.projectRights().insert(Collections.singleton(pr));
+
+    final Project newProject = new Project(newProjectNameKey);
+    newProject.setDescription(projectDescription);
+    newProject.setSubmitType(submitType);
+    newProject.setUseContributorAgreements(contributorAgreements);
+    newProject.setUseSignedOffBy(signedOffBy);
+
+    db.projects().insert(Collections.singleton(newProject));
   }
 
   private void validateParameters() throws Failure {

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ApproveCommand.java

@@ -38,7 +38,6 @@ import com.google.gerrit.sshd.BaseCommand;
 import com.google.gerrit.util.cli.CmdLineParser;
 import com.google.gwtorm.client.OrmException;
 import com.google.gwtorm.client.ResultSet;
-import com.google.gwtorm.client.Transaction;
 import com.google.inject.Inject;
 
 import org.apache.sshd.server.Environment;
@@ -150,7 +149,6 @@ public class ApproveCommand extends BaseCommand {
       throw error("change " + changeId + " is closed");
     }
 
-    final Transaction txn = db.beginTransaction();
     final StringBuffer msgBuf = new StringBuffer();
     msgBuf.append("Patch Set ");
     msgBuf.append(patchSetId.get());
@@ -166,11 +164,11 @@ public class ApproveCommand extends BaseCommand {
       Short score = co.value();
 
       if (score != null) {
-        addApproval(psaKey, score, change, co, txn);
+        addApproval(psaKey, score, change, co);
       } else {
         if (psa == null) {
           score = 0;
-          addApproval(psaKey, score, change, co, txn);
+          addApproval(psaKey, score, change, co);
         } else {
           score = psa.getValue();
         }
@@ -194,10 +192,9 @@ public class ApproveCommand extends BaseCommand {
         new ChangeMessage(new ChangeMessage.Key(changeId, uuid), currentUser
             .getAccountId());
     cm.setMessage(msgBuf.toString());
-    db.changeMessages().insert(Collections.singleton(cm), txn);
+    db.changeMessages().insert(Collections.singleton(cm));
-    ChangeUtil.updated(change);
+
-    db.changes().update(Collections.singleton(change), txn);
+    ChangeUtil.touch(change, db);
-    txn.commit();
     sendMail(change, change.currentPatchSetId(), cm);
   }
 
@@ -279,16 +276,9 @@ public class ApproveCommand extends BaseCommand {
   }
 
   private void addApproval(final PatchSetApproval.Key psaKey,
-      final Short score, final Change c, final ApproveOption co,
+      final Short score, final Change c, final ApproveOption co)
-      final Transaction txn) throws OrmException, UnloggedFailure {
+      throws OrmException, UnloggedFailure {
-    PatchSetApproval psa = db.patchSetApprovals().get(psaKey);
+    final PatchSetApproval psa = new PatchSetApproval(psaKey, score);
-    boolean insert = false;
-
-    if (psa == null) {
-      insert = true;
-      psa = new PatchSetApproval(psaKey, score);
-    }
-
     final List<PatchSetApproval> approvals = Collections.emptyList();
     final FunctionState fs =
         functionStateFactory.create(c, psaKey.getParentKey(), approvals);
@@ -299,12 +289,7 @@ public class ApproveCommand extends BaseCommand {
     }
 
     psa.setGranted();
-
+    db.patchSetApprovals().upsert(Collections.singleton(psa));
-    if (insert) {
-      db.patchSetApprovals().insert(Collections.singleton(psa), txn);
-    } else {
-      db.patchSetApprovals().update(Collections.singleton(psa), txn);
-    }
   }
 
   private void initOptionList() {

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Receive.java

@@ -36,8 +36,10 @@ import com.google.gerrit.reviewdb.Change;
 import com.google.gerrit.reviewdb.ChangeMessage;
 import com.google.gerrit.reviewdb.ContributorAgreement;
 import com.google.gerrit.reviewdb.PatchSet;
+import com.google.gerrit.reviewdb.PatchSetAncestor;
 import com.google.gerrit.reviewdb.PatchSetApproval;
 import com.google.gerrit.reviewdb.PatchSetInfo;
+import com.google.gerrit.reviewdb.RevId;
 import com.google.gerrit.reviewdb.ReviewDb;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.GerritPersonIdent;
@@ -45,16 +47,14 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountResolver;
 import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.config.Nullable;
-import com.google.gerrit.server.git.PatchSetImporter;
 import com.google.gerrit.server.git.ReplicationQueue;
 import com.google.gerrit.server.mail.CreateChangeSender;
 import com.google.gerrit.server.mail.EmailException;
 import com.google.gerrit.server.mail.MergedSender;
 import com.google.gerrit.server.mail.ReplacePatchSetSender;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
+import com.google.gwtorm.client.AtomicUpdate;
 import com.google.gwtorm.client.OrmException;
-import com.google.gwtorm.client.OrmRunnable;
-import com.google.gwtorm.client.Transaction;
 import com.google.inject.Inject;
 
 import org.eclipse.jgit.errors.MissingObjectException;
@@ -150,9 +150,6 @@ final class Receive extends AbstractGitCommand {
   @Inject
   private ReplicationQueue replication;
 
-  @Inject
-  private PatchSetImporter.Factory importFactory;
-
   @Inject
   private PatchSetInfoFactory patchSetInfoFactory;
 
@@ -186,7 +183,8 @@ final class Receive extends AbstractGitCommand {
   protected void runImpl() throws IOException, Failure {
     if (!canUpload()) {
       final String reqName = project.getName();
-      throw new Failure(1, "fatal: Upload denied for project '" + reqName + "'",
+      throw new Failure(1,
+          "fatal: Upload denied for project '" + reqName + "'",
           new SecurityException("Account lacks Upload permission"));
     }
 
@@ -809,20 +807,21 @@ final class Receive extends AbstractGitCommand {
     cc.remove(me);
     cc.removeAll(reviewers);
 
-    final Transaction txn = db.beginTransaction();
     final Change change =
         new Change(changeKey, new Change.Id(db.nextChangeId()), me, destBranch);
-    final PatchSet ps = new PatchSet(change.newPatchSetId());
+    change.nextPatchSetId();
+
+    final PatchSet ps = new PatchSet(change.currPatchSetId());
     ps.setCreatedOn(change.getCreatedOn());
     ps.setUploader(me);
+    ps.setRevision(toRevId(c));
+    insertAncestors(ps.getId(), c);
+    db.patchSets().insert(Collections.singleton(ps));
 
-    final PatchSetImporter imp = importFactory.create(db, c, ps, true);
+    final PatchSetInfo info = patchSetInfoFactory.get(c, ps.getId());
-    imp.setTransaction(txn);
+    change.setCurrentPatchSet(info);
-    imp.run();
-
-    change.setCurrentPatchSet(imp.getPatchSetInfo());
     ChangeUtil.updated(change);
-    db.changes().insert(Collections.singleton(change), txn);
+    db.changes().insert(Collections.singleton(change));
 
     final Set<Account.Id> haveApprovals = new HashSet<Account.Id>();
     final List<ApprovalType> allTypes = approvalTypes.getApprovalTypes();
@@ -830,28 +829,24 @@ final class Receive extends AbstractGitCommand {
 
     if (allTypes.size() > 0) {
       final Account.Id authorId =
-          imp.getPatchSetInfo().getAuthor() != null ? imp.getPatchSetInfo()
+          info.getAuthor() != null ? info.getAuthor().getAccount() : null;
-              .getAuthor().getAccount() : null;
       final Account.Id committerId =
-          imp.getPatchSetInfo().getCommitter() != null ? imp.getPatchSetInfo()
+          info.getCommitter() != null ? info.getCommitter().getAccount() : null;
-              .getCommitter().getAccount() : null;
       final ApprovalCategory.Id catId =
           allTypes.get(allTypes.size() - 1).getCategory().getId();
       if (authorId != null && haveApprovals.add(authorId)) {
-        insertDummyApproval(change, ps.getId(), authorId, catId, db, txn);
+        insertDummyApproval(change, ps.getId(), authorId, catId, db);
       }
       if (committerId != null && haveApprovals.add(committerId)) {
-        insertDummyApproval(change, ps.getId(), committerId, catId, db, txn);
+        insertDummyApproval(change, ps.getId(), committerId, catId, db);
       }
       for (final Account.Id reviewer : reviewers) {
         if (haveApprovals.add(reviewer)) {
-          insertDummyApproval(change, ps.getId(), reviewer, catId, db, txn);
+          insertDummyApproval(change, ps.getId(), reviewer, catId, db);
         }
       }
     }
 
-    txn.commit();
-
     final RefUpdate ru = repo.updateRef(ps.getRefName());
     ru.setNewObjectId(c);
     ru.disableRefLog();
@@ -867,7 +862,7 @@ final class Receive extends AbstractGitCommand {
       final CreateChangeSender cm;
       cm = createChangeSenderFactory.create(change);
       cm.setFrom(me);
-      cm.setPatchSet(ps, imp.getPatchSetInfo());
+      cm.setPatchSet(ps, info);
       cm.setReviewDb(db);
       cm.addReviewers(reviewers);
       cm.addExtraCC(cc);
@@ -932,221 +927,238 @@ final class Receive extends AbstractGitCommand {
     cc.remove(me);
     cc.removeAll(reviewers);
 
-    final ReplaceResult result;
+    final ReplaceResult result = new ReplaceResult();
-
     final Set<Account.Id> oldReviewers = new HashSet<Account.Id>();
     final Set<Account.Id> oldCC = new HashSet<Account.Id>();
 
-    result = db.run(new OrmRunnable<ReplaceResult, ReviewDb>() {
+    Change change = db.changes().get(request.ontoChange);
-      public ReplaceResult run(final ReviewDb db, final Transaction txn,
+    if (change == null) {
-          final boolean isRetry) throws OrmException {
+      reject(request.cmd, "change " + request.ontoChange + " not found");
-        final Change change = db.changes().get(request.ontoChange);
+      return null;
-        if (change == null) {
+    }
-          reject(request.cmd, "change " + request.ontoChange + " not found");
+    if (change.getStatus().isClosed()) {
-          return null;
+      reject(request.cmd, "change " + request.ontoChange + " closed");
-        }
+      return null;
-        if (change.getStatus().isClosed()) {
+    }
-          reject(request.cmd, "change " + request.ontoChange + " closed");
-          return null;
-        }
 
-        final PatchSet.Id priorPatchSet = change.currentPatchSetId();
+    final PatchSet.Id priorPatchSet = change.currentPatchSetId();
-        for (final PatchSet ps : db.patchSets().byChange(request.ontoChange)) {
+    for (final PatchSet ps : db.patchSets().byChange(request.ontoChange)) {
-          if (ps.getRevision() == null) {
+      if (ps.getRevision() == null) {
-            reject(request.cmd, "change state corrupt");
+        reject(request.cmd, "change state corrupt");
-            return null;
+        return null;
-          }
-
-          final String revIdStr = ps.getRevision().get();
-          final ObjectId commitId;
-          try {
-            commitId = ObjectId.fromString(revIdStr);
-          } catch (IllegalArgumentException e) {
-            log.warn("Invalid revision in " + ps.getId() + ": " + revIdStr);
-            reject(request.cmd, "change state corrupt");
-            return null;
-          }
-
-          try {
-            final RevCommit prior = rp.getRevWalk().parseCommit(commitId);
-
-            // Don't allow a change to directly depend upon itself. This is a
-            // very common error due to users making a new commit rather than
-            // amending when trying to address review comments.
-            //
-            if (rp.getRevWalk().isMergedInto(prior, c)) {
-              reject(request.cmd, "squash commits first");
-              return null;
-            }
-
-            // Don't allow the same commit to appear twice on the same change
-            //
-            if (c == prior) {
-              reject(request.cmd, "commit already exists");
-              return null;
-            }
-
-            // Don't allow the same tree if the commit message is unmodified
-            // or no parents were updated (rebase), else warn that only part
-            // of the commit was modified.
-            //
-            if (priorPatchSet.equals(ps.getId())
-                && c.getTree() == prior.getTree()) {
-              rp.getRevWalk().parseBody(prior);
-              final boolean messageEq =
-                  c.getFullMessage().equals(prior.getFullMessage());
-              final boolean parentsEq = parentsEqual(c, prior);
-
-              if (messageEq && parentsEq) {
-                reject(request.cmd, "no changes made");
-                return null;
-              } else {
-                err.write(Constants.encode("warning: " //
-                    + change.getKey().abbreviate() + ": " //
-                    + " no files changed, but" //
-                    + (!messageEq ? " message updated" : "") //
-                    + (!messageEq && !parentsEq ? " and" : "") //
-                    + (!parentsEq ? " was rebased" : "") //
-                    + "\n" //
-                ));
-              }
-            }
-          } catch (IOException e) {
-            log.error("Change " + change.getId() + " missing " + revIdStr, e);
-            reject(request.cmd, "change state corrupt");
-            return null;
-          }
-        }
-
-        final PatchSet ps = new PatchSet(change.newPatchSetId());
-        ps.setCreatedOn(new Timestamp(System.currentTimeMillis()));
-        ps.setUploader(currentUser.getAccountId());
-
-        final PatchSetImporter imp = importFactory.create(db, c, ps, true);
-        imp.setTransaction(txn);
-        imp.run();
-
-        final Ref mergedInto = findMergedInto(change.getDest().get(), c);
-        final ReplaceResult result = new ReplaceResult();
-        result.mergedIntoRef = mergedInto != null ? mergedInto.getName() : null;
-        result.change = change;
-        result.patchSet = ps;
-        result.info = imp.getPatchSetInfo();
-
-        final Account.Id authorId =
-            imp.getPatchSetInfo().getAuthor() != null ? imp.getPatchSetInfo()
-                .getAuthor().getAccount() : null;
-        final Account.Id committerId =
-            imp.getPatchSetInfo().getCommitter() != null ? imp
-                .getPatchSetInfo().getCommitter().getAccount() : null;
-
-        boolean haveAuthor = false;
-        boolean haveCommitter = false;
-        final Set<Account.Id> haveApprovals = new HashSet<Account.Id>();
-
-        oldReviewers.clear();
-        oldCC.clear();
-
-        for (PatchSetApproval a : db.patchSetApprovals().byChange(
-            change.getId())) {
-          haveApprovals.add(a.getAccountId());
-
-          if (a.getValue() != 0) {
-            oldReviewers.add(a.getAccountId());
-          } else {
-            oldCC.add(a.getAccountId());
-          }
-
-          final ApprovalType type =
-              approvalTypes.getApprovalType(a.getCategoryId());
-          if (a.getPatchSetId().equals(priorPatchSet)
-              && type.getCategory().isCopyMinScore() && type.isMaxNegative(a)) {
-            // If there was a negative vote on the prior patch set, carry it
-            // into this patch set.
-            //
-            db.patchSetApprovals()
-                .insert(
-                    Collections.singleton(new PatchSetApproval(ps.getId(), a)),
-                    txn);
-          }
-
-          if (!haveAuthor && authorId != null
-              && a.getAccountId().equals(authorId)) {
-            haveAuthor = true;
-          }
-          if (!haveCommitter && committerId != null
-              && a.getAccountId().equals(committerId)) {
-            haveCommitter = true;
-          }
-        }
-
-        final ChangeMessage msg =
-            new ChangeMessage(new ChangeMessage.Key(change.getId(), ChangeUtil
-                .messageUUID(db)), me, ps.getCreatedOn());
-        msg.setMessage("Uploaded patch set " + ps.getPatchSetId() + ".");
-        db.changeMessages().insert(Collections.singleton(msg), txn);
-        result.msg = msg;
-
-        if (result.mergedIntoRef != null) {
-          // Change was already submitted to a branch, close it.
-          //
-          markChangeMergedByPush(db, txn, result);
-        } else {
-          // Change should be new, so it can go through review again.
-          //
-          change.setStatus(Change.Status.NEW);
-          change.setCurrentPatchSet(imp.getPatchSetInfo());
-          ChangeUtil.updated(change);
-          db.changes().update(Collections.singleton(change), txn);
-        }
-
-        final List<ApprovalType> allTypes = approvalTypes.getApprovalTypes();
-        if (allTypes.size() > 0) {
-          final ApprovalCategory.Id catId =
-              allTypes.get(allTypes.size() - 1).getCategory().getId();
-          if (authorId != null && haveApprovals.add(authorId)) {
-            insertDummyApproval(result, authorId, catId, db, txn);
-          }
-          if (committerId != null && haveApprovals.add(committerId)) {
-            insertDummyApproval(result, committerId, catId, db, txn);
-          }
-          for (final Account.Id reviewer : reviewers) {
-            if (haveApprovals.add(reviewer)) {
-              insertDummyApproval(result, reviewer, catId, db, txn);
-            }
-          }
-        }
-        return result;
       }
-    });
+
-    if (result != null) {
+      final String revIdStr = ps.getRevision().get();
-      final PatchSet ps = result.patchSet;
+      final ObjectId commitId;
-      final RefUpdate ru = repo.updateRef(ps.getRefName());
+      try {
-      ru.setNewObjectId(c);
+        commitId = ObjectId.fromString(revIdStr);
-      ru.disableRefLog();
+      } catch (IllegalArgumentException e) {
-      if (ru.update(rp.getRevWalk()) != RefUpdate.Result.NEW) {
+        log.warn("Invalid revision in " + ps.getId() + ": " + revIdStr);
-        throw new IOException("Failed to create ref " + ps.getRefName()
+        reject(request.cmd, "change state corrupt");
-            + " in " + repo.getDirectory() + ": " + ru.getResult());
+        return null;
       }
-      replication.scheduleUpdate(project.getNameKey(), ru.getName());
-      request.cmd.setResult(ReceiveCommand.Result.OK);
 
       try {
-        final ReplacePatchSetSender cm;
+        final RevCommit prior = rp.getRevWalk().parseCommit(commitId);
-        cm = replacePatchSetFactory.create(result.change);
+
-        cm.setFrom(me);
+        // Don't allow a change to directly depend upon itself. This is a
-        cm.setPatchSet(ps, result.info);
+        // very common error due to users making a new commit rather than
-        cm.setChangeMessage(result.msg);
+        // amending when trying to address review comments.
-        cm.setReviewDb(db);
+        //
-        cm.addReviewers(reviewers);
+        if (rp.getRevWalk().isMergedInto(prior, c)) {
-        cm.addExtraCC(cc);
+          reject(request.cmd, "squash commits first");
-        cm.addReviewers(oldReviewers);
+          return null;
-        cm.addExtraCC(oldCC);
+        }
-        cm.send();
+
-      } catch (EmailException e) {
+        // Don't allow the same commit to appear twice on the same change
-        log.error("Cannot send email for new patch set " + ps.getId(), e);
+        //
+        if (c == prior) {
+          reject(request.cmd, "commit already exists");
+          return null;
+        }
+
+        // Don't allow the same tree if the commit message is unmodified
+        // or no parents were updated (rebase), else warn that only part
+        // of the commit was modified.
+        //
+        if (priorPatchSet.equals(ps.getId()) && c.getTree() == prior.getTree()) {
+          rp.getRevWalk().parseBody(prior);
+          final boolean messageEq =
+              c.getFullMessage().equals(prior.getFullMessage());
+          final boolean parentsEq = parentsEqual(c, prior);
+
+          if (messageEq && parentsEq) {
+            reject(request.cmd, "no changes made");
+            return null;
+          } else {
+            err.write(Constants.encode("warning: " //
+                + change.getKey().abbreviate() + ": " //
+                + " no files changed, but" //
+                + (!messageEq ? " message updated" : "") //
+                + (!messageEq && !parentsEq ? " and" : "") //
+                + (!parentsEq ? " was rebased" : "") //
+                + "\n" //
+            ));
+          }
+        }
+      } catch (IOException e) {
+        log.error("Change " + change.getId() + " missing " + revIdStr, e);
+        reject(request.cmd, "change state corrupt");
+        return null;
       }
     }
+
+    change =
+        db.changes().atomicUpdate(change.getId(), new AtomicUpdate<Change>() {
+          @Override
+          public Change update(Change change) {
+            if (change.getStatus().isOpen()) {
+              change.nextPatchSetId();
+              return change;
+            } else {
+              return null;
+            }
+          }
+        });
+    if (change == null) {
+      reject(request.cmd, "change is closed");
+      return null;
+    }
+
+    final PatchSet ps = new PatchSet(change.currPatchSetId());
+    ps.setCreatedOn(new Timestamp(System.currentTimeMillis()));
+    ps.setUploader(currentUser.getAccountId());
+    ps.setRevision(toRevId(c));
+    insertAncestors(ps.getId(), c);
+    db.patchSets().insert(Collections.singleton(ps));
+
+    final Ref mergedInto = findMergedInto(change.getDest().get(), c);
+    result.mergedIntoRef = mergedInto != null ? mergedInto.getName() : null;
+    result.change = change;
+    result.patchSet = ps;
+    result.info = patchSetInfoFactory.get(c, ps.getId());
+
+    final Account.Id authorId =
+        result.info.getAuthor() != null ? result.info.getAuthor().getAccount()
+            : null;
+    final Account.Id committerId =
+        result.info.getCommitter() != null ? result.info.getCommitter()
+            .getAccount() : null;
+
+    boolean haveAuthor = false;
+    boolean haveCommitter = false;
+    final Set<Account.Id> haveApprovals = new HashSet<Account.Id>();
+
+    oldReviewers.clear();
+    oldCC.clear();
+
+    for (PatchSetApproval a : db.patchSetApprovals().byChange(change.getId())) {
+      haveApprovals.add(a.getAccountId());
+
+      if (a.getValue() != 0) {
+        oldReviewers.add(a.getAccountId());
+      } else {
+        oldCC.add(a.getAccountId());
+      }
+
+      final ApprovalType type =
+          approvalTypes.getApprovalType(a.getCategoryId());
+      if (a.getPatchSetId().equals(priorPatchSet)
+          && type.getCategory().isCopyMinScore() && type.isMaxNegative(a)) {
+        // If there was a negative vote on the prior patch set, carry it
+        // into this patch set.
+        //
+        db.patchSetApprovals().insert(
+            Collections.singleton(new PatchSetApproval(ps.getId(), a)));
+      }
+
+      if (!haveAuthor && authorId != null && a.getAccountId().equals(authorId)) {
+        haveAuthor = true;
+      }
+      if (!haveCommitter && committerId != null
+          && a.getAccountId().equals(committerId)) {
+        haveCommitter = true;
+      }
+    }
+
+    final ChangeMessage msg =
+        new ChangeMessage(new ChangeMessage.Key(change.getId(), ChangeUtil
+            .messageUUID(db)), me, ps.getCreatedOn());
+    msg.setMessage("Uploaded patch set " + ps.getPatchSetId() + ".");
+    db.changeMessages().insert(Collections.singleton(msg));
+    result.msg = msg;
+
+    if (result.mergedIntoRef != null) {
+      // Change was already submitted to a branch, close it.
+      //
+      markChangeMergedByPush(db, result);
+    } else {
+      // Change should be new, so it can go through review again.
+      //
+      change =
+          db.changes().atomicUpdate(change.getId(), new AtomicUpdate<Change>() {
+            @Override
+            public Change update(Change change) {
+              if (change.getStatus().isOpen()) {
+                change.setStatus(Change.Status.NEW);
+                change.setCurrentPatchSet(result.info);
+                ChangeUtil.updated(change);
+                return change;
+              } else {
+                return null;
+              }
+            }
+          });
+      if (change == null) {
+        db.patchSets().delete(Collections.singleton(ps));
+        db.changeMessages().delete(Collections.singleton(msg));
+        reject(request.cmd, "change is closed");
+        return null;
+      }
+    }
+
+    final List<ApprovalType> allTypes = approvalTypes.getApprovalTypes();
+    if (allTypes.size() > 0) {
+      final ApprovalCategory.Id catId =
+          allTypes.get(allTypes.size() - 1).getCategory().getId();
+      if (authorId != null && haveApprovals.add(authorId)) {
+        insertDummyApproval(result, authorId, catId, db);
+      }
+      if (committerId != null && haveApprovals.add(committerId)) {
+        insertDummyApproval(result, committerId, catId, db);
+      }
+      for (final Account.Id reviewer : reviewers) {
+        if (haveApprovals.add(reviewer)) {
+          insertDummyApproval(result, reviewer, catId, db);
+        }
+      }
+    }
+
+    final RefUpdate ru = repo.updateRef(ps.getRefName());
+    ru.setNewObjectId(c);
+    ru.disableRefLog();
+    if (ru.update(rp.getRevWalk()) != RefUpdate.Result.NEW) {
+      throw new IOException("Failed to create ref " + ps.getRefName() + " in "
+          + repo.getDirectory() + ": " + ru.getResult());
+    }
+    replication.scheduleUpdate(project.getNameKey(), ru.getName());
+    request.cmd.setResult(ReceiveCommand.Result.OK);
+
+    try {
+      final ReplacePatchSetSender cm;
+      cm = replacePatchSetFactory.create(result.change);
+      cm.setFrom(me);
+      cm.setPatchSet(ps, result.info);
+      cm.setChangeMessage(result.msg);
+      cm.setReviewDb(db);
+      cm.addReviewers(reviewers);
+      cm.addExtraCC(cc);
+      cm.addReviewers(oldReviewers);
+      cm.addExtraCC(oldCC);
+      cm.send();
+    } catch (EmailException e) {
+      log.error("Cannot send email for new patch set " + ps.getId(), e);
+    }
     sendMergedEmail(result);
     return result != null ? result.info.getKey() : null;
   }
@@ -1165,19 +1177,19 @@ final class Receive extends AbstractGitCommand {
 
   private void insertDummyApproval(final ReplaceResult result,
       final Account.Id forAccount, final ApprovalCategory.Id catId,
-      final ReviewDb db, final Transaction txn) throws OrmException {
+      final ReviewDb db) throws OrmException {
     insertDummyApproval(result.change, result.patchSet.getId(), forAccount,
-        catId, db, txn);
+        catId, db);
   }
 
   private void insertDummyApproval(final Change change, final PatchSet.Id psId,
       final Account.Id forAccount, final ApprovalCategory.Id catId,
-      final ReviewDb db, final Transaction txn) throws OrmException {
+      final ReviewDb db) throws OrmException {
     final PatchSetApproval ca =
         new PatchSetApproval(new PatchSetApproval.Key(psId, forAccount, catId),
             (short) 0);
     ca.cache(change);
-    db.patchSetApprovals().insert(Collections.singleton(ca), txn);
+    db.patchSetApprovals().insert(Collections.singleton(ca));
   }
 
   private Ref findMergedInto(final String first, final RevCommit commit) {
@@ -1327,35 +1339,29 @@ final class Receive extends AbstractGitCommand {
       final RevCommit commit) throws OrmException {
     final String refName = cmd.getRefName();
     final Change.Id cid = psi.getParentKey();
-    final ReplaceResult result =
-        db.run(new OrmRunnable<ReplaceResult, ReviewDb>() {
-          @Override
-          public ReplaceResult run(ReviewDb db, Transaction txn, boolean retry)
-              throws OrmException {
-            final Change change = db.changes().get(cid);
-            final PatchSet ps = db.patchSets().get(psi);
-            if (change == null || ps == null) {
-              log.warn(project.getName() + " " + psi + " is missing");
-              return null;
-            }
 
-            if (change.getStatus() == Change.Status.MERGED) {
+    final Change change = db.changes().get(cid);
-              // If its already merged, don't make further updates, it
+    final PatchSet ps = db.patchSets().get(psi);
-              // might just be moving from an experimental branch into
+    if (change == null || ps == null) {
-              // a more stable branch.
+      log.warn(project.getName() + " " + psi + " is missing");
-              //
+      return;
-              return null;
+    }
-            }
 
-            final ReplaceResult result = new ReplaceResult();
+    if (change.getStatus() == Change.Status.MERGED) {
-            result.change = change;
+      // If its already merged, don't make further updates, it
-            result.patchSet = ps;
+      // might just be moving from an experimental branch into
-            result.info = patchSetInfoFactory.get(commit, psi);
+      // a more stable branch.
-            result.mergedIntoRef = refName;
+      //
-            markChangeMergedByPush(db, txn, result);
+      return;
-            return result;
+    }
-          }
+
-        });
+    final ReplaceResult result = new ReplaceResult();
+    result.change = change;
+    result.patchSet = ps;
+    result.info = patchSetInfoFactory.get(commit, psi);
+    result.mergedIntoRef = refName;
+
+    markChangeMergedByPush(db, result);
     sendMergedEmail(result);
   }
 
@@ -1379,7 +1385,7 @@ final class Receive extends AbstractGitCommand {
     return r;
   }
 
-  private void markChangeMergedByPush(final ReviewDb db, final Transaction txn,
+  private void markChangeMergedByPush(final ReviewDb db,
       final ReplaceResult result) throws OrmException {
     final Change change = result.change;
     final String mergedIntoRef = result.mergedIntoRef;
@@ -1393,6 +1399,7 @@ final class Receive extends AbstractGitCommand {
     for (PatchSetApproval a : approvals) {
       a.cache(change);
     }
+    db.patchSetApprovals().update(approvals);
 
     final StringBuilder msgBuf = new StringBuilder();
     msgBuf.append("Change has been successfully pushed");
@@ -1411,9 +1418,19 @@ final class Receive extends AbstractGitCommand {
             .messageUUID(db)), currentUser.getAccountId());
     msg.setMessage(msgBuf.toString());
 
-    db.patchSetApprovals().update(approvals, txn);
+    db.changeMessages().insert(Collections.singleton(msg));
-    db.changeMessages().insert(Collections.singleton(msg), txn);
+
-    db.changes().update(Collections.singleton(change), txn);
+    db.changes().atomicUpdate(change.getId(), new AtomicUpdate<Change>() {
+      @Override
+      public Change update(Change change) {
+        if (change.getStatus().isOpen()) {
+          change.setCurrentPatchSet(result.info);
+          change.setStatus(Change.Status.MERGED);
+          ChangeUtil.updated(change);
+        }
+        return change;
+      }
+    });
   }
 
   private void sendMergedEmail(final ReplaceResult result) {
@@ -1433,6 +1450,24 @@ final class Receive extends AbstractGitCommand {
     }
   }
 
+  private void insertAncestors(PatchSet.Id id, RevCommit src)
+      throws OrmException {
+    final int cnt = src.getParentCount();
+    List<PatchSetAncestor> toInsert = new ArrayList<PatchSetAncestor>(cnt);
+    for (int p = 0; p < cnt; p++) {
+      PatchSetAncestor a;
+
+      a = new PatchSetAncestor(new PatchSetAncestor.Id(id, p + 1));
+      a.setAncestorRevision(toRevId(src.getParent(p)));
+      toInsert.add(a);
+    }
+    db.patchSetAncestors().insert(toInsert);
+  }
+
+  private static RevId toRevId(final RevCommit src) {
+    return new RevId(src.getId().name());
+  }
+
   private boolean canPerform(final ApprovalCategory.Id actionId, final short val) {
     return projectControl.canPerform(actionId, val);
   }