From a37d85ccd634f01b8ca6604962a345f7b50c87bc Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 19 Apr 2016 09:24:18 -0400 Subject: [PATCH] Support per-BatchRefUpdate atomic transactions Repurpose RefDatabase#performsAtomicTransactions() slightly, to indicate that the backend _supports_ atomic transactions, rather than the current definition, which is that the backend always _uses_ atomic transactions regardless of whether or not the caller actually wants them. Allow BatchRefUpdate callers to turn off atomic transactions by calling setAtomic(false). Defaulting to true means this is backwards compatible. Change-Id: I6df78d7df65ab147b4cce7764bd3101db985491c --- .../eclipse/jgit/internal/JGitText.properties | 1 + .../org/eclipse/jgit/internal/JGitText.java | 1 + .../storage/dfs/InMemoryRepository.java | 2 +- .../storage/reftree/RefTreeBatch.java | 8 ++- .../org/eclipse/jgit/lib/BatchRefUpdate.java | 49 +++++++++++++++++++ .../src/org/eclipse/jgit/lib/RefDatabase.java | 21 +++++++- 6 files changed, 77 insertions(+), 5 deletions(-) diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 992e10bad..a99bd7e91 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -21,6 +21,7 @@ atLeastOnePathIsRequired=At least one path is required. atLeastOnePatternIsRequired=At least one pattern is required. atLeastTwoFiltersNeeded=At least two filters needed. atomicPushNotSupported=Atomic push not supported. +atomicRefUpdatesNotSupported=Atomic ref updates not supported authenticationNotSupported=authentication not supported badBase64InputCharacterAt=Bad Base64 input character at {0} : {1} (decimal) badEntryDelimiter=Bad entry delimiter diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 7740a2bb8..dc640fe31 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -80,6 +80,7 @@ public static JGitText get() { /***/ public String atLeastOnePatternIsRequired; /***/ public String atLeastTwoFiltersNeeded; /***/ public String atomicPushNotSupported; + /***/ public String atomicRefUpdatesNotSupported; /***/ public String authenticationNotSupported; /***/ public String badBase64InputCharacterAt; /***/ public String badEntryDelimiter; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java index ccf1b42b3..de18eadb2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java @@ -280,7 +280,7 @@ public BatchRefUpdate newBatchUpdate() { @Override public void execute(RevWalk walk, ProgressMonitor monitor) throws IOException { - if (performsAtomicTransactions()) { + if (performsAtomicTransactions() && isAtomic()) { try { lock.writeLock().lock(); batch(getCommands()); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/RefTreeBatch.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/RefTreeBatch.java index a55a9f51e..1cccd7981 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/RefTreeBatch.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/RefTreeBatch.java @@ -98,8 +98,12 @@ public void execute(RevWalk rw, ProgressMonitor monitor) } if (c.getType() == UPDATE_NONFASTFORWARD) { c.setResult(REJECTED_NONFASTFORWARD); - ReceiveCommand.abort(getCommands()); - return; + if (isAtomic()) { + ReceiveCommand.abort(getCommands()); + return; + } else { + continue; + } } } todo.add(new Command(rw, c)); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java index d7e930831..35cadd3c9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java @@ -89,6 +89,9 @@ public class BatchRefUpdate { /** Push certificate associated with this update. */ private PushCertificate pushCert; + /** Whether updates should be atomic. */ + private boolean atomic; + /** * Initialize a new batch update. * @@ -98,6 +101,7 @@ public class BatchRefUpdate { protected BatchRefUpdate(RefDatabase refdb) { this.refdb = refdb; this.commands = new ArrayList(); + this.atomic = refdb.performsAtomicTransactions(); } /** @@ -199,6 +203,36 @@ public boolean isRefLogDisabled() { return refLogMessage == null; } + /** + * Request that all updates in this batch be performed atomically. + *

+ * When atomic updates are used, either all commands apply successfully, or + * none do. Commands that might have otherwise succeeded are rejected with + * {@code REJECTED_OTHER_REASON}. + *

+ * This method only works if the underlying ref database supports atomic + * transactions, i.e. {@link RefDatabase#performsAtomicTransactions()} returns + * true. Calling this method with true if the underlying ref database does not + * support atomic transactions will cause all commands to fail with {@code + * REJECTED_OTHER_REASON}. + * + * @param atomic whether updates should be atomic. + * @return {@code this} + * @since 4.4 + */ + public BatchRefUpdate setAtomic(boolean atomic) { + this.atomic = atomic; + return this; + } + + /** + * @return atomic whether updates should be atomic. + * @since 4.4 + */ + public boolean isAtomic() { + return atomic; + } + /** * Set a push certificate associated with this update. *

@@ -271,6 +305,10 @@ public BatchRefUpdate addCommand(Collection cmd) { *

* The default implementation of this method performs a sequential reference * update over each reference. + *

+ * Implementations must respect the atomicity requirements of the underlying + * database as described in {@link #setAtomic(boolean)} and {@link + * RefDatabase#performsAtomicTransactions()}. * * @param walk * a RevWalk to parse tags in case the storage system wants to @@ -284,6 +322,17 @@ public BatchRefUpdate addCommand(Collection cmd) { */ public void execute(RevWalk walk, ProgressMonitor monitor) throws IOException { + + if (atomic && !refdb.performsAtomicTransactions()) { + for (ReceiveCommand c : commands) { + if (c.getResult() == NOT_ATTEMPTED) { + c.setResult(REJECTED_OTHER_REASON, + JGitText.get().atomicRefUpdatesNotSupported); + } + } + return; + } + monitor.beginTask(JGitText.get().updatingReferences, commands.size()); List commands2 = new ArrayList( commands.size()); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java index c0c3862c8..517c8aa53 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java @@ -207,8 +207,25 @@ public BatchRefUpdate newBatchUpdate() { } /** - * @return if the database performs {@code newBatchUpdate()} as an atomic - * transaction. + * Whether the database is capable of performing batch updates as atomic + * transactions. + *

+ * If true, by default {@link BatchRefUpdate} instances will perform updates + * atomically, meaning either all updates will succeed, or all updates will + * fail. It is still possible to turn off this behavior on a per-batch basis + * by calling {@code update.setAtomic(false)}. + *

+ * If false, {@link BatchRefUpdate} instances will never perform updates + * atomically, and calling {@code update.setAtomic(true)} will cause the + * entire batch to fail with {@code REJECTED_OTHER_REASON}. + *

+ * This definition of atomicity is stronger than what is provided by + * {@link org.eclipse.jgit.transport.ReceivePack}. {@code ReceivePack} will + * attempt to reject all commands if it knows in advance some commands may + * fail, even if the storage layer does not support atomic transactions. Here, + * atomicity applies even in the case of unforeseeable errors. + * + * @return whether transactions are atomic by default. * @since 3.6 */ public boolean performsAtomicTransactions() {