Uploaded image for project: 'Blazegraph (by SYSTAP)'
  1. Blazegraph (by SYSTAP)
  2. BLZG-506

RWStore does not prune the CommitRecordIndex

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Affects Version/s: TERMS_REFACTOR_BRANCH
    • Fix Version/s: None
    • Component/s: RWStore

      Description

      The RWStore does not prune the head of the CommitRecordIndex as commit points are released. It also might be failing to remove the CommitRecord and logged root block for those historical commits.

      Deleting the old CommitRecordIndex records is somewhat tricky as the commit record index is only modified during the commit protocol. The deletes on the CommitRecordIndex occur after we have already checkpointed the deferred free blocks for the commit. In order to break this recursion, the deleted records for the commit record index need a special logging structure, or perhaps simple a 2nd deferred free block checkpoint. "Immediate" delete of those data is not safe since that would make it impossible to restore an old root block and then roll the state of the database forward from there.

      See https://sourceforge.net/apps/trac/bigdata/ticket/290 (RWStore (and scale-out data service) should indicate the earliest available commit point.)

      See https://sourceforge.net/apps/trac/bigdata/ticket/375 (Persistent memory leaks (RWStore/DISK))

        Activity

        Hide
        bryanthompson bryanthompson added a comment -

        RWStore#2241 in r4962 had an assertion related to this issue which has been disabled. The assertion can in fact be tripped by the delete of records for leaves and nodes in the commit record index during update as part of the commit protocol.

        			// There must be no buffered deferred frees
        			assert m_deferredFreeOut.getBytesWritten() == 0;
        
        Show
        bryanthompson bryanthompson added a comment - RWStore#2241 in r4962 had an assertion related to this issue which has been disabled. The assertion can in fact be tripped by the delete of records for leaves and nodes in the commit record index during update as part of the commit protocol. // There must be no buffered deferred frees assert m_deferredFreeOut.getBytesWritten() == 0;
        Hide
        bryanthompson bryanthompson added a comment -

        Basic on discussions with Martyn, it seems that the best way to handle this would be to patch the CommitRecord as one of the very last actions taken before the atomic commit point. In order to do this the CommitRecord serialized format can not use compression for the addresses (it does not, but we should document this in the code) since that would cause the size of the record to change. This is governed by the CommitRecordSerializer. The address at which the commit record will be written and the prepped commit record will both have to be handed off to the RWStore since it can not patch/write the commit record until it has finished with all deferred deletes, including those for the CommitRecordIndex.

        We will also have to explicitly prune the head of the CommitRecordIndex, removing entries for timestamps which are no longer visible.

        AbstractJournal#commitNow() must delete the old commit record index and old journaled root block when it creates new versions of those records. This will wind up being logged on the delete blocks and eventually deleted.

        The delete blocks themselves should be deleted. This is a bit circular. We can't delete them inside of the commit point whose deletes they are logging, so maybe we want to log their deletes as either the first/last action of the next commit point.

        Show
        bryanthompson bryanthompson added a comment - Basic on discussions with Martyn, it seems that the best way to handle this would be to patch the CommitRecord as one of the very last actions taken before the atomic commit point. In order to do this the CommitRecord serialized format can not use compression for the addresses (it does not, but we should document this in the code) since that would cause the size of the record to change. This is governed by the CommitRecordSerializer. The address at which the commit record will be written and the prepped commit record will both have to be handed off to the RWStore since it can not patch/write the commit record until it has finished with all deferred deletes, including those for the CommitRecordIndex. We will also have to explicitly prune the head of the CommitRecordIndex, removing entries for timestamps which are no longer visible. AbstractJournal#commitNow() must delete the old commit record index and old journaled root block when it creates new versions of those records. This will wind up being logged on the delete blocks and eventually deleted. The delete blocks themselves should be deleted. This is a bit circular. We can't delete them inside of the commit point whose deletes they are logging, so maybe we want to log their deletes as either the first/last action of the next commit point.
        Hide
        bryanthompson bryanthompson added a comment -

        One comment. While I think that we are better served by deleting the old commit record, journaled root blocks, delete blocks, etc. when we create the new ones we could prune such stuff out of an existing RWStores the first time we prune the CommitRecordIndex. I am not saying we should do this, just that we could.

        Show
        bryanthompson bryanthompson added a comment - One comment. While I think that we are better served by deleting the old commit record, journaled root blocks, delete blocks, etc. when we create the new ones we could prune such stuff out of an existing RWStores the first time we prune the CommitRecordIndex. I am not saying we should do this, just that we could.
        Hide
        bryanthompson bryanthompson added a comment -

        Change sets for RWStore history and persistent memory leaks following code review with MCutcher.

        Committed Revision r5809 (against 1.0.x maintenance branch and TERMS_REFACTOR_BRANCH)

        @see https://sourceforge.net/apps/trac/bigdata/ticket/217 (BTreeCounters does not track bytes released)
        @see https://sourceforge.net/apps/trac/bigdata/ticket/329 (B+Tree should delete bloom filter when it is disabled)
        @see https://sourceforge.net/apps/trac/bigdata/ticket/372 (RWStore does not prune commit record index).
        @see https://sourceforge.net/apps/trac/bigdata/ticket/375 (Persistent memory leaks (RWStore/DISK))

        Show
        bryanthompson bryanthompson added a comment - Change sets for RWStore history and persistent memory leaks following code review with MCutcher. Committed Revision r5809 (against 1.0.x maintenance branch and TERMS_REFACTOR_BRANCH) @see https://sourceforge.net/apps/trac/bigdata/ticket/217 (BTreeCounters does not track bytes released) @see https://sourceforge.net/apps/trac/bigdata/ticket/329 (B+Tree should delete bloom filter when it is disabled) @see https://sourceforge.net/apps/trac/bigdata/ticket/372 (RWStore does not prune commit record index). @see https://sourceforge.net/apps/trac/bigdata/ticket/375 (Persistent memory leaks (RWStore/DISK))

          People

          • Assignee:
            martyncutcher martyncutcher
            Reporter:
            bryanthompson bryanthompson
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: