Uploaded image for project: 'Jackrabbit Content Repository'
  1. Jackrabbit Content Repository
  2. JCR-2129

Prevent data inconsistencies due to incorrect or missed merges in the ItemStateManagers

Details

    • Bug
    • Status: Closed
    • Critical
    • Resolution: Fixed
    • core 1.4.5
    • core 1.4.12, 1.5.7
    • jackrabbit-core
    • None

    Description

      There are two places where transient state changes are merged with persisted state: in the SessionISM.stateModified call back method (typically called by a saving thread after it's changes have been saved) and (ii) in the SharedISM.Update.begin method. Sometimes the merge strategy fails in the sense that corrupt data is written to database.

      How to reproduce:
      The attached test program contains steps to reproduce the issue. The testInconsistency1 method concurrently adds a node D to a node A and moves child node B of A to another place. This sometimes results in a situation in which node A still has a child reference to node B whereas B has another parent. In this situation, Jackrabbit cannot be started anymore if the search index is missing:

      Jun 8, 2009 2:16:23 PM org.apache.jackrabbit.core.query.OnWorkspaceInconsistency$1 handleMissingChildNode
      SEVERE: Node /A (a7a29e8c-8d13-4fbd-b0ca-4f93f9c0ef42) has missing child 'B' (80aa13c5-1db6-4f62-b576-5e7f626d90c1)
      Jun 8, 2009 2:16:23 PM org.apache.jackrabbit.core.RepositoryImpl initStartupWorkspaces
      SEVERE: Failed to initialize workspace 'wm9'

      The testInconsistency2 method concurrently adds a reference property to a node B (the threads do exactly the same). This sometimes results in a situation in which the referenced node can never be removed anymore because there is a "ghost" reference to it which cannot be removed. (It gives a ReferentialIntegrityException).

      Jun 8, 2009 2:19:51 PM org.apache.jackrabbit.core.state.MLRUItemStateCache cache
      WARNING: overwriting cached entry bc28ff87-216d-4ccd-bd73-03e7499ab54e/{}ref to B
      Exception in thread "main" javax.jcr.ReferentialIntegrityException: 9f025634-d3e1-448e-904c-1c285f6b1bf6: the node cannot be removed because it is still being referenced.

      It seems the first problem (the parent-child relation) is caused by an incorrect merge in the NodeStateMerger class. The second problem might also be caused by an incorrect or missed merge, but I am not sure whether that's the real problem.

      Attachments

        1. InconsistencyTest.java
          7 kB
          Martijn Hendriks
        2. repository.xml
          5 kB
          Martijn Hendriks
        3. JCR-2129-testcase.patch
          9 kB
          Martijn Hendriks
        4. JCR-2129.patch
          3 kB
          Martijn Hendriks
        5. JCR-2129.patch
          32 kB
          Marcel Reutegger

        Activity

          Attached a patch which contains two single threaded JUnit testcases for the mentioned problems.

          martijnh Martijn Hendriks added a comment - Attached a patch which contains two single threaded JUnit testcases for the mentioned problems.

          Created a separate issue for the references issue because it seems unrelated to the parent-child thing: JCR-2138.

          martijnh Martijn Hendriks added a comment - Created a separate issue for the references issue because it seems unrelated to the parent-child thing: JCR-2138 .

          Attached a testcase + patch.

          The problem seems to be that the NodeStateMerger cannot distinguish externally added child nodes from locally moved child nodes (both show up in the getRemovedChildNodeEntries result).

          martijnh Martijn Hendriks added a comment - Attached a testcase + patch. The problem seems to be that the NodeStateMerger cannot distinguish externally added child nodes from locally moved child nodes (both show up in the getRemovedChildNodeEntries result).

          I'd like to extend the test cases to cover more cases including any combination of add/remove node by one session and move node to/from by another session.

          I'll post an updated patch when it is ready....

          mreutegg Marcel Reutegger added a comment - I'd like to extend the test cases to cover more cases including any combination of add/remove node by one session and move node to/from by another session. I'll post an updated patch when it is ready....

          Attached is an extended version of Martijns initial patch. It contains more test cases covering various combinations of modifications as well as previously untested modifications.

          I also extended the NodeStateMerger.MergeContext with an additional method that allows the merge to distinguish between an externally added/removed node and a locally moved node.

          Martijn, can you please test the patch and report back if it works for you? thanks.

          mreutegg Marcel Reutegger added a comment - Attached is an extended version of Martijns initial patch. It contains more test cases covering various combinations of modifications as well as previously untested modifications. I also extended the NodeStateMerger.MergeContext with an additional method that allows the merge to distinguish between an externally added/removed node and a locally moved node. Martijn, can you please test the patch and report back if it works for you? thanks.

          Marcel, thanks for extending the test cases and creating a new patch, which works fine for me! I see that you improved the merging strategy which is nicer than throwing an ItemStateException.

          martijnh Martijn Hendriks added a comment - Marcel, thanks for extending the test cases and creating a new patch, which works fine for me! I see that you improved the merging strategy which is nicer than throwing an ItemStateException.

          Thank you for the feedback.

          Committed my version of the patch in revision: 789279

          mreutegg Marcel Reutegger added a comment - Thank you for the feedback. Committed my version of the patch in revision: 789279
          jukkaz Jukka Zitting added a comment -

          Merged to the 1.x branch in revision 791842.

          jukkaz Jukka Zitting added a comment - Merged to the 1.x branch in revision 791842.
          jukkaz Jukka Zitting added a comment -

          Merged to the 1.5 branch in revision 794292.

          jukkaz Jukka Zitting added a comment - Merged to the 1.5 branch in revision 794292.
          jukkaz Jukka Zitting added a comment -

          Merged to the 1.4 branch in revision 948381.

          jukkaz Jukka Zitting added a comment - Merged to the 1.4 branch in revision 948381.

          People

            Unassigned Unassigned
            martijnh Martijn Hendriks
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: