Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.5
    • Component/s: clustering, jackrabbit-core
    • Labels:
      None

      Description

      During the prepare phase of a XATransaction, XAItemStateManager.prepare calls ShareItemStageManager.beginUpdate that, in case of a ClusterNode, calls ClusterNode.updatePrepared that does a ChangeLogRecord.write().

      This last method is located in ClusterRecord and systematically write the begin and the end of the journal record.

      As a consequence, useless corrupted records are written in the journal everytime a transaction ends without jackrabbit update! This causes polution of the journal, as other cluster nodes try to sync with the corrupted updates and fail doing so as ClusterRecordDeserializer can't deserialize it (the record identifier is empty).

      ChangeLogRecord (and even other ClusterRecord implementations too) should only write if there's effective updates.

      I propose the following solution:
      *) add the following method in Changelog so clients can know if there's effective updates:
      public boolean hasUpdates()

      { return !(addedStates.isEmpty() && modifiedStates.isEmpty() && deletedStates.isEmpty() && modifiedRefs.isEmpty()); }

      *) change ClusterRecord with:
      public final void write() throws JournalException {

      if (hasUpdates())

      { record.writeString(workspace); doWrite(); record.writeChar(END_MARKER); }

      }

      protected abstract boolean hasUpdates();

      *) implement hasUpdates for every ClusterRecord implementation:
      ----> ChangeLogRecord:
      protected boolean hasUpdates()

      { return changes.hasUpdates() || !events.isEmpty(); }

      ----> LockRecord and NamespaceRecord:
      protected boolean hasUpdates()

      { return true; }

      ----> NodeTypeRecord:
      protected boolean hasUpdates()

      { return !collection.isEmpty(); }

      Best regards,

      Stephane Landelle

        Activity

        Hide
        Ian Boston added a comment -

        Ignore my previous comment, I have a workarround in the form of a custom (ugly) AppendRecord
        For anyone else with the same problem it can be found at https://source.sakaiproject.org/contrib/k2/trunk/kernel/src/main/java/org/sakaiproject/kernel/jcr/jackrabbit/journal/ until the 1.4.x series gets updated or we move to 1.5.

        Show
        Ian Boston added a comment - Ignore my previous comment, I have a workarround in the form of a custom (ugly) AppendRecord For anyone else with the same problem it can be found at https://source.sakaiproject.org/contrib/k2/trunk/kernel/src/main/java/org/sakaiproject/kernel/jcr/jackrabbit/journal/ until the 1.4.x series gets updated or we move to 1.5.
        Hide
        Ian Boston added a comment -

        Will this get applied back to 1.4 ?

        I have tried Extending/Replacing ClusterNode to stop it emitting empty journal records with JTA transactions, and because of all the protected, final methods, its not possible. I haven't tried aspects yet, but dont really want to, and analyzing the byte stream going into the DB is just going to be a pain.

        It would be a shame to run from patched/forked version of Jackrabbit.

        I also tried 1.5, but it doesn't appear stable enough yet,

        Show
        Ian Boston added a comment - Will this get applied back to 1.4 ? I have tried Extending/Replacing ClusterNode to stop it emitting empty journal records with JTA transactions, and because of all the protected, final methods, its not possible. I haven't tried aspects yet, but dont really want to, and analyzing the byte stream going into the DB is just going to be a pain. It would be a shame to run from patched/forked version of Jackrabbit. I also tried 1.5, but it doesn't appear stable enough yet,
        Hide
        Jukka Zitting added a comment -

        Added the hasUpdates condition for non-transactional updates in revision 708619.

        Also, in revision 708609 I made the journal consumer handle such empty records more gracefully.

        Show
        Jukka Zitting added a comment - Added the hasUpdates condition for non-transactional updates in revision 708619. Also, in revision 708609 I made the journal consumer handle such empty records more gracefully.
        Hide
        Jukka Zitting added a comment -

        Hmm, you're right. I was just thinking about the original use case where a distributed transaction that has no Jackrabbit changes is committed.

        Reopening this issue so I remember to look at it more before releasing 1.5.0. I guess it would be a good to set up a clustered test case for the following code:

        Session session = repository.login(...);
        try

        { session.save(); }

        finally

        { session.logout(); }

        This should at least not cause cluster nodes to fail. It would be nice if it also didn't result in any journal entries.

        Show
        Jukka Zitting added a comment - Hmm, you're right. I was just thinking about the original use case where a distributed transaction that has no Jackrabbit changes is committed. Reopening this issue so I remember to look at it more before releasing 1.5.0. I guess it would be a good to set up a clustered test case for the following code: Session session = repository.login(...); try { session.save(); } finally { session.logout(); } This should at least not cause cluster nodes to fail. It would be nice if it also didn't result in any journal entries.
        Hide
        Stephane Landelle added a comment -

        Hello Jukka,

        Are you sure the problem is specific to XA transactions?

        I guess the same problem happens with local transactions. One might not have
        actual updates to commit, depending on the business code executed inside the
        transaction.

        So I think every SharedItemStateManager is impacted and that testing if the
        ChangeLog has actual updates should be done in the core methods.

        Am I wrong somewhere?

        Best regards,

        Stephane Landelle

        2008/10/19 Jukka Zitting (JIRA) <jira@apache.org>

        Show
        Stephane Landelle added a comment - Hello Jukka, Are you sure the problem is specific to XA transactions? I guess the same problem happens with local transactions. One might not have actual updates to commit, depending on the business code executed inside the transaction. So I think every SharedItemStateManager is impacted and that testing if the ChangeLog has actual updates should be done in the core methods. Am I wrong somewhere? Best regards, Stephane Landelle 2008/10/19 Jukka Zitting (JIRA) <jira@apache.org>
        Hide
        Jukka Zitting added a comment -

        Resolved in revision 705938. Thanks for the good report and proposed fix!

        I added the ChangeLog.hasUpdates() method as proposed, but used it already in XAItemStateManager instead of SharedItemStateManager to avoid as much extra processing as possible.

        Show
        Jukka Zitting added a comment - Resolved in revision 705938. Thanks for the good report and proposed fix! I added the ChangeLog.hasUpdates() method as proposed, but used it already in XAItemStateManager instead of SharedItemStateManager to avoid as much extra processing as possible.
        Hide
        Stephane Landelle added a comment -

        My bad, my patch writes null records instead of not writing them.

        A better solution would consist in:

        • adding the hasUpdates method to ChangeLog
        • skipping begin and end in
          org.apache.jackrabbit.core.state.SharedItemStateManager$Update if local has
          no updates with somehing like:

        if (!local.hasUpdates())

        { return; }

        Sincerely,

        Stephane Landelle

        2008/10/15 Stephane Landelle (JIRA) <jira@apache.org>

        Show
        Stephane Landelle added a comment - My bad, my patch writes null records instead of not writing them. A better solution would consist in: adding the hasUpdates method to ChangeLog skipping begin and end in org.apache.jackrabbit.core.state.SharedItemStateManager$Update if local has no updates with somehing like: if (!local.hasUpdates()) { return; } Sincerely, Stephane Landelle 2008/10/15 Stephane Landelle (JIRA) <jira@apache.org>

          People

          • Assignee:
            Jukka Zitting
            Reporter:
            Stephane Landelle
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development