[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16636789#comment-16636789
]
Julian Reschke commented on OAK-7286:
-
Backporting all sounds even better. I'll have a look.
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Assignee: Marcel Reutegger
>Priority: Major
> Fix For: 1.10
>
> Attachments: OAK-7286-DocumentStoreException.patch,
> OAK-7286-DocumentStoreException.patch, OAK-7286.diff, OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16636659#comment-16636659
]
Marcel Reutegger commented on OAK-7286:
---
Hmm, I'm not sure. My preference is to either backport all or none of the
sub-tasks.
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Assignee: Marcel Reutegger
>Priority: Major
> Fix For: 1.10
>
> Attachments: OAK-7286-DocumentStoreException.patch,
> OAK-7286-DocumentStoreException.patch, OAK-7286.diff, OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16571480#comment-16571480
]
Julian Reschke commented on OAK-7286:
-
[~mreutegg] - I'd like to port back (to 1.8) OAK-7305 (the changes to the
exception class) and OAK-7307, in order to minify diffs when backporting other
stuff in RDB land. However, this would leave us with the modified exception,
but no code in DocumentNodeStore doing something specific with it. Would that
be a problem?
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Assignee: Marcel Reutegger
>Priority: Major
> Fix For: 1.10
>
> Attachments: OAK-7286-DocumentStoreException.patch,
> OAK-7286-DocumentStoreException.patch, OAK-7286.diff, OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16387693#comment-16387693
]
Julian Reschke commented on OAK-7286:
-
+1
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Assignee: Marcel Reutegger
>Priority: Major
> Fix For: 1.9.0, 1.10
>
> Attachments: OAK-7286-DocumentStoreException.patch,
> OAK-7286-DocumentStoreException.patch, OAK-7286.diff, OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16387664#comment-16387664
]
Marcel Reutegger commented on OAK-7286:
---
Btw, I created a sub-task for the type on the DocumentStoreException: OAK-7305
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Assignee: Marcel Reutegger
>Priority: Major
> Fix For: 1.9.0, 1.10
>
> Attachments: OAK-7286-DocumentStoreException.patch,
> OAK-7286-DocumentStoreException.patch, OAK-7286.diff, OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16387659#comment-16387659
]
Marcel Reutegger commented on OAK-7286:
---
I like your suggestion. See updated proposal.
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Assignee: Marcel Reutegger
>Priority: Major
> Fix For: 1.9.0, 1.10
>
> Attachments: OAK-7286-DocumentStoreException.patch,
> OAK-7286-DocumentStoreException.patch, OAK-7286.diff, OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16387648#comment-16387648
]
Julian Reschke commented on OAK-7286:
-
+1 in general, but we need to define what {{NETWORK}} means. Also, wouldn't the
term "transient" match better?
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Assignee: Marcel Reutegger
>Priority: Major
> Fix For: 1.9.0, 1.10
>
> Attachments: OAK-7286-DocumentStoreException.patch, OAK-7286.diff,
> OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16387632#comment-16387632
]
Marcel Reutegger commented on OAK-7286:
---
Yes, I picked up your idea again. See [^OAK-7286-DocumentStoreException.patch]
(lots more JavaDoc, but basically adds a type to the exception). The
DocumentNodeStore then may retry in case of a exception of type 'network'. WDYT?
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Assignee: Marcel Reutegger
>Priority: Major
> Fix For: 1.9.0, 1.10
>
> Attachments: OAK-7286-DocumentStoreException.patch, OAK-7286.diff,
> OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16387591#comment-16387591
]
Julian Reschke commented on OAK-7286:
-
Interesting - that could be an argument in favor of differentiating between
retryable and fatal exceptions, right?
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Assignee: Marcel Reutegger
>Priority: Major
> Fix For: 1.9.0, 1.10
>
> Attachments: OAK-7286.diff, OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16387454#comment-16387454
]
Julian Reschke commented on OAK-7286:
-
OK, thanks for checking. I'll investigate.
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Assignee: Marcel Reutegger
>Priority: Major
> Fix For: 1.9.0, 1.10
>
> Attachments: OAK-7286.diff, OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16387434#comment-16387434
]
Marcel Reutegger commented on OAK-7286:
---
Turns out it actually is wrapped in a RepositoryException. The assert error is
confusing because it only logs the message of the exception, which in fact
mentions DocumentStoreException. AFAICS this is caused by the
{{RDBDocumentStore.readDocumentsUncached()}} and how it creates a
{{DocumentStoreException}}. Using the constructor with the single Throwable
parameter will use the {{toString()}} method of the Throwable as message for
the {{DocumentStoreException}}.
We could change the {{DocumentStoreException}} constructor or the
{{RDBDocumentStore}} could use the static {{DocumentStoreException.convert()}}
instead. In any case, I think that should go into a separate issue...
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Assignee: Marcel Reutegger
>Priority: Major
> Fix For: 1.9.0, 1.10
>
> Attachments: OAK-7286.diff, OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16386128#comment-16386128
]
Julian Reschke commented on OAK-7286:
-
It seems that now the {{DocumentStoreException}} thrown by RDB on invalid IDs
surfaces on the JCR level as it. Is that intended?
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Assignee: Marcel Reutegger
>Priority: Major
> Fix For: 1.9.0, 1.10
>
> Attachments: OAK-7286.diff, OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16381783#comment-16381783
]
Marcel Reutegger commented on OAK-7286:
---
Thanks. I took it a bit further and changed the ConflictException to not extend
from DocumentStoreException anymore. This required some more changes. See
https://github.com/apache/jackrabbit-oak/commit/5211986a0e931014d4aa4e7e53fc3b040b94b58c
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Assignee: Marcel Reutegger
>Priority: Major
> Attachments: OAK-7286.diff, OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16380499#comment-16380499
]
Julian Reschke commented on OAK-7286:
-
Removing the catch causes the {{DocumentStoreException}} to be catched later on
which seems to work, see
https://issues.apache.org/jira/secure/attachment/12912450/OAK-7286.diff - I
only changed the message so the original diagnostics actually end up in the
thrown exception on the JCR layer.
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Priority: Major
> Attachments: OAK-7286.diff, OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16378835#comment-16378835
]
Marcel Reutegger commented on OAK-7286:
---
The DocumentStore implementation should never throw a {{ConflictException}}.
Only the DocumentNodeStore throws such an exception, when it cannot commit a
change due to a conflict. Which raises the question if the
{{ConflictException}} should really extend {{DocumentStoreException}}. Probably
not...
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Priority: Major
> Attachments: OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16378822#comment-16378822
]
Julian Reschke commented on OAK-7286:
-
Interesting. So would there be a case where a {{DocumentStore}} implementation
would throw a {{ConflictException}}, or is this just internal to
{{DocumentNodeStore}}?
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Priority: Major
> Attachments: OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7286) DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
[
https://issues.apache.org/jira/browse/OAK-7286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16378782#comment-16378782
]
Marcel Reutegger commented on OAK-7286:
---
bq. We probably should have a way to dinstinguish between different types of
problems.
To some degree this was already introduced with {{ConflictException}}, which
extends {{DocumentStoreException}}. I think it would be best if we can get rid
of the catch DocumentStoreException there entirely. It's quite possible that
catch clause is not necessary anymore.
> DocumentNodeStoreBranch handling of non-recoverable DocumentStoreExceptions
> ---
>
> Key: OAK-7286
> URL: https://issues.apache.org/jira/browse/OAK-7286
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: documentmk
>Reporter: Julian Reschke
>Priority: Major
> Attachments: OAK-7286.diff
>
>
> In {{DocumentNodeStoreBranch.merge()}}, any {{DocumentStoreException}} is
> mapped to a {{DocumentStoreException}} to a {{CommitFailedException}} of type
> "MERGE", which leads to the operation being retried, and a non-helpful
> exception being generated.
> The effect can be observed by enabling a test in {{ValidNamesTest}}:
> {noformat}
> --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Revision 1825371)
> +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ValidNamesTest.java
> (Arbeitskopie)
> @@ -300,7 +300,6 @@
> public void testUnpairedHighSurrogateEnd() {
> // see OAK-5506
>
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("segment"));
> -
> org.junit.Assume.assumeFalse(super.fixture.toString().toLowerCase().contains("rdb"));
> nameTest("foo" + SURROGATE_PAIR[0]);
> }
> @@ -336,6 +335,7 @@
> assertEquals("paths should be equal", p.getPath(), n.getPath());
> return p;
> } catch (RepositoryException ex) {
> +ex.printStackTrace();
> fail(ex.getMessage());
> return null;
> }
> {noformat}
> The underlying issue is that {{RDBDocumentStore}} is throwing a
> {{DocumentStoreException}} due to the invalid ID, and repeating the call will
> not help.
> We probably should have a way to dinstinguish between different types of
> problems.
> I hacked {{DocumentNodeStoreBranch}} like that:
> {noformat}
> ---
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Revision 1825371)
> +++
> oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
> (Arbeitskopie)
> @@ -520,8 +520,12 @@
> } catch (ConflictException e) {
> throw e.asCommitFailedException();
> } catch(DocumentStoreException e) {
> -throw new CommitFailedException(MERGE, 1,
> -"Failed to merge changes to the underlying
> store", e);
> +if (e.getMessage().contains("Invalid ID")) {
> +throw new CommitFailedException(OAK, 123,
> +"Failed to store changes in the underlying
> store: " + e.getMessage(), e);
> +} else {
> +throw new CommitFailedException(MERGE, 1, "Failed to
> merge changes to the underlying store", e);
> +}
> } catch (Exception e) {
> throw new CommitFailedException(OAK, 1,
> "Failed to merge changes to the underlying
> store", e);
> {noformat}
> ...which causes the exception to surface immediately (see
> https://issues.apache.org/jira/secure/attachment/12912117/OAK-7286.diff).
> (cc [~mreutegg])
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
