You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by re...@apache.org on 2015/07/14 16:27:50 UTC
svn commit: r1690941 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/rdb/
test/java/org/apache/jackrabbit/oak/plugins/document/
Author: reschke
Date: Tue Jul 14 14:27:50 2015
New Revision: 1690941
URL: http://svn.apache.org/r1690941
Log:
OAK-3096: improve diagnostics for failed batch inserts, add test case observing the DocumentStore's behavior wrt to atomicity
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStore.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBJDBCTools.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/BasicDocumentStoreTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStore.java?rev=1690941&r1=1690940&r2=1690941&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStore.java Tue Jul 14 14:27:50 2015
@@ -1499,15 +1499,25 @@ public class RDBDocumentStore implements
private <T extends Document> boolean insertDocuments(Collection<T> collection, List<T> documents) {
Connection connection = null;
String tableName = getTable(collection);
- List<String> ids = new ArrayList<String>();
try {
connection = this.ch.getRWConnection();
boolean result = dbInsert(connection, tableName, documents);
connection.commit();
return result;
} catch (SQLException ex) {
- LOG.debug("insert of " + ids + " failed", ex);
this.ch.rollbackConnection(connection);
+
+ List<String> ids = new ArrayList<String>();
+ for (T doc : documents) {
+ ids.add(doc.getId());
+ }
+ LOG.debug("insert of " + ids + " failed", ex);
+
+ // collect additional exceptions
+ String messages = LOG.isDebugEnabled() ? RDBJDBCTools.getAdditionalMessages(ex) : "";
+ if (!messages.isEmpty()) {
+ LOG.debug("additional diagnostics: " + messages);
+ }
throw new DocumentStoreException(ex);
} finally {
this.ch.closeConnection(connection);
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBJDBCTools.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBJDBCTools.java?rev=1690941&r1=1690940&r2=1690941&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBJDBCTools.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBJDBCTools.java Tue Jul 14 14:27:50 2015
@@ -17,6 +17,9 @@
package org.apache.jackrabbit.oak.plugins.document.rdb;
import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Locale;
import java.util.Map;
@@ -119,4 +122,22 @@ public class RDBJDBCTools {
}
return String.format("%s (%d)", name, isolationLevel);
}
+
+ /**
+ * Return a string containing additional messages from chained exceptions.
+ */
+ protected static @Nonnull String getAdditionalMessages(SQLException ex) {
+ List<String> messages = new ArrayList<String>();
+ String message = ex.getMessage();
+ SQLException next = ex.getNextException();
+ while (next != null) {
+ String m = next.getMessage();
+ if (!message.equals(m)) {
+ messages.add(m);
+ }
+ next = next.getNextException();
+ }
+
+ return messages.isEmpty() ? "" : messages.toString();
+ }
}
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/BasicDocumentStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/BasicDocumentStoreTest.java?rev=1690941&r1=1690940&r2=1690941&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/BasicDocumentStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/BasicDocumentStoreTest.java Tue Jul 14 14:27:50 2015
@@ -25,10 +25,11 @@ import static org.junit.Assert.fail;
import java.io.UnsupportedEncodingException;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.Collections;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.UUID;
import org.apache.jackrabbit.oak.plugins.document.UpdateOp.Condition;
@@ -226,26 +227,63 @@ public class BasicDocumentStoreTest exte
@Test
public void testCreatePartialFailure() {
- String id = this.getClass().getName() + ".testCreatePartialFailure";
+ String bid = this.getClass().getName() + ".testCreatePartialFailure-";
+ int cnt = 10;
+ assertTrue(cnt > 8);
+
+ // clear repo
+ for (int i = 0; i < cnt; i++) {
+ super.ds.remove(Collection.NODES, bid + i);
+ removeMe.add(bid + i);
+ }
- // remove if present
- NodeDocument nd = super.ds.find(Collection.NODES, id);
- if (nd != null) {
- super.ds.remove(Collection.NODES, id);
+ // create one of the test nodes
+ int pre = cnt / 2;
+ UpdateOp up = new UpdateOp(bid + pre, true);
+ up.set("_id", bid + pre);
+ up.set("foo", "bar");
+ assertTrue(super.ds.create(Collection.NODES, Collections.singletonList(up)));
+
+ // batch create
+ Set<String> toCreate = new HashSet<String>();
+ Set<String> toCreateFailEarly = new HashSet<String>();
+ List<UpdateOp> ups = new ArrayList<UpdateOp>();
+ for (int i = 0; i < cnt; i++) {
+ UpdateOp op = new UpdateOp(bid + i, true);
+ op.set("_id", bid + i);
+ op.set("foo", "qux");
+ ups.add(op);
+ if (i != pre) {
+ toCreate.add(bid + i);
+ }
+ if (i < pre) {
+ toCreateFailEarly.add(bid + i);
+ }
}
+ assertFalse(super.ds.create(Collection.NODES, ups));
- // create a test node
- UpdateOp up = new UpdateOp(id, true);
- up.set("_id", id);
- boolean success = super.ds.create(Collection.NODES, Collections.singletonList(up));
- assertTrue(success);
- removeMe.add(id);
+ // check how many nodes are there
+ Set<String> created = new HashSet<String>();
+ for (int i = 0; i < cnt; i++) {
+ boolean present = null != super.ds.find(Collection.NODES, bid + i, 0);
+ if (i == pre && !present) {
+ fail(super.dsname + ": batch update removed previously existing node " + (bid + i));
+ } else if (present && i != pre) {
+ created.add(bid + i);
+ }
+ }
- // try to create two nodes, of which one exists
- UpdateOp up2 = new UpdateOp(id + "2", true);
- up2.set("_id", id + "2");
- success = super.ds.create(Collection.NODES, Arrays.asList(new UpdateOp[] { up, up2 }));
- assertFalse(success);
+ // diagnostics
+ toCreate.removeAll(created);
+ if (created.isEmpty()) {
+ LOG.info(super.dsname + ": create() apparently is atomic");
+ } else if (created.size() == toCreate.size()) {
+ LOG.info(super.dsname + ": create() apparently is best-effort");
+ } else if (created.equals(toCreateFailEarly)) {
+ LOG.info(super.dsname + ": create() stops at first failure");
+ } else {
+ LOG.info(super.dsname + ": create() created: " + created + ", missing: " + toCreate);
+ }
}
@Test
Re: svn commit: r1690941 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/rdb/ test/java/org/apache/jackrabbit/oak/plugins/document/
Posted by Julian Reschke <ju...@gmx.de>.
On 2015-07-15 05:10, Chetan Mehrotra wrote:
> Hi Julian,
>
> On Tue, Jul 14, 2015 at 7:57 PM, <re...@apache.org> wrote:
>> +
>> + List<String> ids = new ArrayList<String>();
>> + for (T doc : documents) {
>> + ids.add(doc.getId());
>> + }
>> + LOG.debug("insert of " + ids + " failed", ex);
>> +
>> + // collect additional exceptions
>> + String messages = LOG.isDebugEnabled() ? RDBJDBCTools.getAdditionalMessages(ex) : "";
>> + if (!messages.isEmpty()) {
>> + LOG.debug("additional diagnostics: " + messages);
>> + }
>
> If all that work is to be done for debug logging then probably the
> whole block should be within isDebugEnabled check
>
> + LOG.debug("insert of " + ids + " failed", ex);
>
> Instead of using concatenation it would be better to use placeholders like
>
> LOG.debug("insert of {} failed",ids, ex);
>
> Chetan Mehrotra
Understood. But keep in mind this is error handling. It's not
performance relevant at all.
Best regards, Julian
Re: svn commit: r1690941 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/rdb/ test/java/org/apache/jackrabbit/oak/plugins/document/
Posted by Chetan Mehrotra <ch...@gmail.com>.
Hi Julian,
On Tue, Jul 14, 2015 at 7:57 PM, <re...@apache.org> wrote:
> +
> + List<String> ids = new ArrayList<String>();
> + for (T doc : documents) {
> + ids.add(doc.getId());
> + }
> + LOG.debug("insert of " + ids + " failed", ex);
> +
> + // collect additional exceptions
> + String messages = LOG.isDebugEnabled() ? RDBJDBCTools.getAdditionalMessages(ex) : "";
> + if (!messages.isEmpty()) {
> + LOG.debug("additional diagnostics: " + messages);
> + }
If all that work is to be done for debug logging then probably the
whole block should be within isDebugEnabled check
+ LOG.debug("insert of " + ids + " failed", ex);
Instead of using concatenation it would be better to use placeholders like
LOG.debug("insert of {} failed",ids, ex);
Chetan Mehrotra