diff --git a/persistit/core/src/main/java/com/persistit/Exchange.java b/persistit/core/src/main/java/com/persistit/Exchange.java index 3842f42e9..897c7ba2d 100644 --- a/persistit/core/src/main/java/com/persistit/Exchange.java +++ b/persistit/core/src/main/java/com/persistit/Exchange.java @@ -4076,12 +4076,49 @@ boolean fixIndexHole(final long page, final int level) throws PersistitException return false; } try { + /* + * This action was enqueued by rebalanceSplit() when the page was + * reachable only through its sibling chain, and may be arbitrarily + * stale by the time the background CleanupManager runs it: a covering + * removeKeyRange may since have unlinked the page onto a garbage chain + * (leaving its content and type intact, or retyping it PAGE_TYPE_GARBAGE + * if it became a chain root), or the page may have been reused by an + * unrelated allocation. Blindly inserting a key-pointer pair for such a + * page plants a dangling index entry to a garbage or reused page -- the + * bug 1017957 failure mode, observed as transient + * CorruptVolumeExceptions ("invalid page type 30") under concurrent + * structural stress. Structure deletes hold the tree writer claim, so + * under the reader claim held here the page's reachability cannot + * change; validate it before inserting the pointer. + */ buffer = _pool.get(_volume, page, false, true); + if (buffer.getPageType() != level + PAGE_TYPE_DATA + || buffer.getKeyBlockEnd() <= buffer.getKeyBlockStart()) { + // Retyped (garbage-chain root or reused elsewhere) or empty: the + // index hole this action was created for no longer exists. + return true; + } buffer.nextKey(_spareKey2, buffer.toKeyBlock(0)); _value.setPointerValue(page); _value.setPointerPageType(buffer.getPageType()); buffer.release(); buffer = null; + /* + * A page sitting on a garbage chain keeps its old type and content, so + * the type check above is not sufficient. Verify the page is still + * reachable in this tree: a search for its current first key at this + * level must land on the page itself (via the B-link right-walk, since + * the hole means it has no parent entry yet). If the search lands + * elsewhere the action is stale and inserting the pointer would corrupt + * the index, so drop it. + */ + searchTree(_spareKey2, level, false); + final LevelCache lc = _levelCache[level]; + final long landedOn = lc._page; + lc._buffer.releaseTouched(); + if (landedOn != page) { + return true; + } storeInternal(_spareKey2, _value, level + 1, StoreOptions.NONE); return true; } finally { diff --git a/persistit/core/src/test/java/com/persistit/TrackingFileChannel.java b/persistit/core/src/test/java/com/persistit/TrackingFileChannel.java index dd4d9fc0c..41243761b 100644 --- a/persistit/core/src/test/java/com/persistit/TrackingFileChannel.java +++ b/persistit/core/src/test/java/com/persistit/TrackingFileChannel.java @@ -163,13 +163,14 @@ public List getReadPositionList() { public void assertOrdered(final boolean read, final boolean forward) { final List list = read ? _readPositions : _writePositions; - final long previous = forward ? -1 : Long.MAX_VALUE; + long previous = forward ? -1 : Long.MAX_VALUE; for (final Long position : list) { if (forward) { assertTrue("Position should be larger", position > previous); } else { assertTrue("Position should be smaller", position < previous); } + previous = position; } } } diff --git a/persistit/core/src/test/java/com/persistit/TreeTransactionalLifetimeTest.java b/persistit/core/src/test/java/com/persistit/TreeTransactionalLifetimeTest.java index 2f9109dbc..55e3a49c2 100644 --- a/persistit/core/src/test/java/com/persistit/TreeTransactionalLifetimeTest.java +++ b/persistit/core/src/test/java/com/persistit/TreeTransactionalLifetimeTest.java @@ -1,6 +1,8 @@ /** * Copyright 2013 Akiban Technologies, Inc. - * + * + * Portions Copyrighted 2026 3A Systems, LLC. + * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at @@ -235,6 +237,21 @@ private void createRemoveByStepHelper(final String treeName, final boolean primo if (crash || restart) { _persistit = new Persistit(_config); _persistit.initialize(); + /* + * Recovery itself completes synchronously inside initialize(), but + * two maintenance tasks are asynchronous after it: the + * TransactionIndex's active-transaction-cache updater thread and + * the CleanupManager's timer-driven pruneTimelyResources(). If the + * next helper iteration starts a transaction before those settle, + * residual tree-version state from this iteration's crash/restart + * can leak into its step-based MVCC visibility -- e.g. a tree + * created at step 1 shows up at step 0 -- producing an intermittent + * ComparisonFailure ("expected:<0[]...> but was:<0[:]...>") seen on + * CI. Force both here, the same way the sibling tests quiesce MVCC + * state before asserting. + */ + _persistit.getTransactionIndex().updateActiveTransactionCache(); + _persistit.pruneTimelyResources(); } assertEquals("Expected contents at steps", expected2, computeCreateRemoveState(treeName, 1)); } diff --git a/persistit/core/src/test/java/com/persistit/WarmupTest.java b/persistit/core/src/test/java/com/persistit/WarmupTest.java index 1d0141aae..8e3de26af 100644 --- a/persistit/core/src/test/java/com/persistit/WarmupTest.java +++ b/persistit/core/src/test/java/com/persistit/WarmupTest.java @@ -66,7 +66,6 @@ public void testWarmup() throws Exception { @Test public void readOrderIsSequential() throws Exception { - Exchange ex = _persistit.getExchange("persistit", "WarmupTest", true); BufferPool pool = ex.getBufferPool(); @@ -107,11 +106,24 @@ public void readOrderIsSequential() throws Exception { _persistit.copyBackPages(); _persistit.close(); - _persistit = new Persistit(); + /* + * recordBufferInventory() at the close above interleaves its own tree stores + * with the buffer scan, so some recorded pages are the inventory tree's own + * pages; the closing checkpoint flushes those to the journal with no + * copyBack(), and on restart they would be served from the journal + * (VolumeStorageV2.readPage consults readPageFromJournal first), bypassing + * the injected volume channel. Reopen with inventory recording disabled, + * drain the journal into the volume, and close again so every recorded page + * lives in the volume file; the preload below then reads them through the + * tracked channel deterministically instead of racing the journal. + */ _config.setBufferInventoryEnabled(false); _config.setBufferPreloadEnabled(false); - _persistit.setConfiguration(_config); - _persistit.initialize(); + _persistit = new Persistit(_config); + _persistit.copyBackPages(); + _persistit.close(); + + _persistit = new Persistit(_config); final Volume volume = _persistit.getVolume("persistit"); final MediatedFileChannel mfc = (MediatedFileChannel) volume.getStorage().getChannel(); @@ -119,7 +131,14 @@ public void readOrderIsSequential() throws Exception { mfc.injectChannelForTests(tfc); pool = volume.getStructure().getPool(); pool.preloadBufferInventory(); - assertTrue("Preload should have loaded pages from journal file", tfc.getReadPositionList().size() > 0); + /* + * With the recorded pages copied back to the volume above, preload reads + * them through the injected volume channel rather than the journal, so the + * channel sees a non-empty, strictly ascending sequence of reads (warmup + * issues them in page order via PageNode.READ_COMPARATOR). + */ + assertTrue("Preload should have read the recorded pages from the volume file", + tfc.getReadPositionList().size() > 0); tfc.assertOrdered(true, true); } }