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

RWStore immedateFree() not removing Checkpoint addresses from the historical index cache.

    Details

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

      Description

      The RW backing file modes are responsible for removing entries in the AbstractJournal's historicalIndexCache. This is done by publishing the reference to that cache and then, for each object that is freed, attempting to remove the entry from the historical cache. In order to provide a fast path, the code does not attempt to remove entries from the cache for objects whose slot size are not equal to the slot size of the checkpoint record.

      A bug was identified where removeFromExternalCache() was being invoked with the allocation size (versus the slot size) in RWStore.immediateFree(). This bug was demonstrated by a new test suite for a named index scan in TestName2Addr.

        Activity

        Hide
        bryanthompson bryanthompson added a comment -

        Note: The fix for this ticket is in r6452 in RWStore.immediateFree(). The change set is:

        {{

        { Modified: branches/BIGDATA_RELEASE_1_2_0/bigdata/src/java/com/bigdata/rwstore/RWStore.java =================================================================== \--\- branches/BIGDATA_RELEASE_1_2_0/bigdata/src/java/com/bigdata/rwstore/RWStore.java 2012-08-17 21:07:52 UTC (rev 6451) +++ branches/BIGDATA_RELEASE_1_2_0/bigdata/src/java/com/bigdata/rwstore/RWStore.java 2012-08-17 21:27:56 UTC (rev 6452) @@ \-1743,9 +1743,19 @@ if (overrideSession | \!this.isSessionProtected()) \{ // Only overwrite if NOT committed if (\!alloc.isCommitted(addrOffset)) \{ \\- m_writeCache.clearWrite(pa); + m_writeCache.clearWrite(pa); // m_writeCache.overwrite(pa, sze); \\- removeFromExternalCache(pa, sze); + /* + * Pass the size of the allocator, NOT the size of the + * allocation. + * + * @see <a + * href="https://sourceforge.net/apps/trac/bigdata/ticket/586" + * > RWStore immedateFree() not removing Checkpoint + * addresses from the historical index cache. </a> + */ +// removeFromExternalCache(pa, sze); + removeFromExternalCache(pa, alloc.m_size); }

        }
        m_frees++;
        @@ -1764,10 +1774,38 @@
        }


        - void removeFromExternalCache(final long clr, final int sze) {
        - assert m_allocationLock.isLocked();
        - if (m_externalCache != null && sze == 0 | sze == m_cachedDatasize)
        - m_externalCache.remove(clr);
        + /**
        + * We need to remove entries from the historicalIndexCache for checkpoint
        + * records when the allocations associated with those checkpoint records are
        + * freed.
        + *
        + * @param clr
        + * The physical address that is being deleted.
        + * @param slotSize
        + * The size of the allocator slot for that physical address.
        + *
        + * @see <a href="https://sourceforge.net/apps/trac/bigdata/ticket/586">
        + * RWStore immedateFree() not removing Checkpoint addresses from the
        + * historical index cache. </a>
        + */
        + void removeFromExternalCache(final long clr, final int slotSize) {
        +
        + assert m_allocationLock.isLocked();
        +
        + if (m_externalCache == null)
        + return;
        +
        + if (slotSize == 0 | slotSize == m_cachedDatasize) {
        +
        + /*
        + * Either known to be the same slot size as a checkpoint record -or-
        + * the slot size is not known.
        + */
        +
        + m_externalCache.remove(clr);
        +
        + }
        +
        }
        /**
        ]}}

        Show
        bryanthompson bryanthompson added a comment - Note: The fix for this ticket is in r6452 in RWStore.immediateFree(). The change set is: {{ { Modified: branches/BIGDATA_RELEASE_1_2_0/bigdata/src/java/com/bigdata/rwstore/RWStore.java =================================================================== \--\- branches/BIGDATA_RELEASE_1_2_0/bigdata/src/java/com/bigdata/rwstore/RWStore.java 2012-08-17 21:07:52 UTC (rev 6451) +++ branches/BIGDATA_RELEASE_1_2_0/bigdata/src/java/com/bigdata/rwstore/RWStore.java 2012-08-17 21:27:56 UTC (rev 6452) @@ \-1743,9 +1743,19 @@ if (overrideSession | \!this.isSessionProtected()) \{ // Only overwrite if NOT committed if (\!alloc.isCommitted(addrOffset)) \{ \\- m_writeCache.clearWrite(pa); + m_writeCache.clearWrite(pa); // m_writeCache.overwrite(pa, sze); \\- removeFromExternalCache(pa, sze); + /* + * Pass the size of the allocator, NOT the size of the + * allocation. + * + * @see <a + * href="https://sourceforge.net/apps/trac/bigdata/ticket/586" + * > RWStore immedateFree() not removing Checkpoint + * addresses from the historical index cache. </a> + */ +// removeFromExternalCache(pa, sze); + removeFromExternalCache(pa, alloc.m_size); } } m_frees++; @@ -1764,10 +1774,38 @@ } - void removeFromExternalCache(final long clr, final int sze) { - assert m_allocationLock.isLocked(); - if (m_externalCache != null && sze == 0 | sze == m_cachedDatasize) - m_externalCache.remove(clr); + /** + * We need to remove entries from the historicalIndexCache for checkpoint + * records when the allocations associated with those checkpoint records are + * freed. + * + * @param clr + * The physical address that is being deleted. + * @param slotSize + * The size of the allocator slot for that physical address. + * + * @see <a href="https://sourceforge.net/apps/trac/bigdata/ticket/586"> + * RWStore immedateFree() not removing Checkpoint addresses from the + * historical index cache. </a> + */ + void removeFromExternalCache(final long clr, final int slotSize) { + + assert m_allocationLock.isLocked(); + + if (m_externalCache == null) + return; + + if (slotSize == 0 | slotSize == m_cachedDatasize) { + + /* + * Either known to be the same slot size as a checkpoint record -or- + * the slot size is not known. + */ + + m_externalCache.remove(clr); + + } + } /** ]}}
        Hide
        bryanthompson bryanthompson added a comment -

        This fix has been incorporated into the critical maintenance release 1.2.2. See [1]

        [1] https://sourceforge.net/apps/trac/bigdata/ticket/603 (Prepare critical maintenance release as branch of 1.2.1)

        Show
        bryanthompson bryanthompson added a comment - This fix has been incorporated into the critical maintenance release 1.2.2. See [1] [1] https://sourceforge.net/apps/trac/bigdata/ticket/603 (Prepare critical maintenance release as branch of 1.2.1)

          People

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

            Dates

            • Created:
              Updated:
              Resolved: