You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@asterixdb.apache.org by mb...@apache.org on 2025/11/06 17:28:26 UTC
(asterixdb) 09/25: [NO ISSUE][*DB][STO] Fix IndexOutOfBoundsException removing last page...
This is an automated email from the ASF dual-hosted git repository.
mblow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git
commit b20e3c66d9a8a9c695f0314060aadd43954e47a2
Author: Michael Blow <mi...@couchbase.com>
AuthorDate: Thu Oct 2 18:14:09 2025 -0400
[NO ISSUE][*DB][STO] Fix IndexOutOfBoundsException removing last page...
...when flushPtr is also that page
(cherry picked from commit 75dd626ac1)
Ext-ref:MB-68818
Change-Id: I1a710fa4773b2d75ff4f252399f4dadccdf3ed8d
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20471
Reviewed-by: Michael Blow <mb...@apache.org>
Reviewed-by: Ali Alsuliman <al...@gmail.com>
Tested-by: Michael Blow <mb...@apache.org>
---
.../dataflow/GlobalVirtualBufferCacheTest.java | 100 +++++++++++++++++++++
.../common/context/GlobalVirtualBufferCache.java | 6 +-
2 files changed, 103 insertions(+), 3 deletions(-)
diff --git a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/dataflow/GlobalVirtualBufferCacheTest.java b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/dataflow/GlobalVirtualBufferCacheTest.java
index b056bbe162..0a1fba2020 100644
--- a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/dataflow/GlobalVirtualBufferCacheTest.java
+++ b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/dataflow/GlobalVirtualBufferCacheTest.java
@@ -19,6 +19,7 @@
package org.apache.asterix.test.dataflow;
import java.io.File;
+import java.lang.reflect.Field;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Collections;
@@ -32,6 +33,7 @@ import org.apache.asterix.app.nc.NCAppRuntimeContext;
import org.apache.asterix.common.api.IDatasetLifecycleManager;
import org.apache.asterix.common.config.DatasetConfig.DatasetType;
import org.apache.asterix.common.config.StorageProperties.Option;
+import org.apache.asterix.common.context.GlobalVirtualBufferCache;
import org.apache.asterix.common.metadata.DataverseName;
import org.apache.asterix.common.transactions.ITransactionContext;
import org.apache.asterix.common.transactions.ITransactionManager;
@@ -60,6 +62,7 @@ import org.apache.hyracks.storage.am.common.dataflow.IndexDataflowHelperFactory;
import org.apache.hyracks.storage.am.common.impls.AbstractTreeIndex;
import org.apache.hyracks.storage.am.lsm.btree.impl.TestLsmBtree;
import org.apache.hyracks.storage.am.lsm.common.api.ILSMDiskComponent;
+import org.apache.hyracks.storage.am.lsm.common.api.ILSMIndex;
import org.apache.hyracks.storage.am.lsm.common.impls.NoMergePolicyFactory;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -175,6 +178,103 @@ public class GlobalVirtualBufferCacheTest {
}
}
+ @Test
+ public void testFlushPtrBoundsAfterRemovingLastIndex() throws Exception {
+ try {
+ // Create a scenario with multiple indexes to manipulate flushPtr
+ DataverseName dvName = DataverseName.createSinglePartName(StorageTestUtils.DATAVERSE_NAME);
+
+ // Create 3 additional datasets to have multiple indexes
+ Dataset dataset2 = new TestDataset(dvName, "ds2", dvName, StorageTestUtils.DATA_TYPE_NAME,
+ StorageTestUtils.NODE_GROUP_NAME, NoMergePolicyFactory.NAME, null,
+ new InternalDatasetDetails(null, PartitioningStrategy.HASH, StorageTestUtils.PARTITIONING_KEYS,
+ null, null, null, false, null, null),
+ null, DatasetType.INTERNAL, StorageTestUtils.DATASET_ID + 2, 0);
+
+ Dataset dataset3 = new TestDataset(dvName, "ds3", dvName, StorageTestUtils.DATA_TYPE_NAME,
+ StorageTestUtils.NODE_GROUP_NAME, NoMergePolicyFactory.NAME, null,
+ new InternalDatasetDetails(null, PartitioningStrategy.HASH, StorageTestUtils.PARTITIONING_KEYS,
+ null, null, null, false, null, null),
+ null, DatasetType.INTERNAL, StorageTestUtils.DATASET_ID + 3, 0);
+
+ // Create indexes for first partition only for simplicity
+ PrimaryIndexInfo index2Info = StorageTestUtils.createPrimaryIndex(nc, dataset2, 0);
+ PrimaryIndexInfo index3Info = StorageTestUtils.createPrimaryIndex(nc, dataset3, 0);
+
+ // Create index instances
+ IIndexDataflowHelperFactory factory2 =
+ new IndexDataflowHelperFactory(nc.getStorageManager(), index2Info.getFileSplitProvider());
+ IIndexDataflowHelper helper2 = factory2.create(testCtxs[0].getJobletContext().getServiceContext(), 0);
+ helper2.open();
+ TestLsmBtree index2 = (TestLsmBtree) helper2.getIndexInstance();
+ helper2.close();
+
+ IIndexDataflowHelperFactory factory3 =
+ new IndexDataflowHelperFactory(nc.getStorageManager(), index3Info.getFileSplitProvider());
+ IIndexDataflowHelper helper3 = factory3.create(testCtxs[0].getJobletContext().getServiceContext(), 0);
+ helper3.open();
+ TestLsmBtree index3 = (TestLsmBtree) helper3.getIndexInstance();
+ helper3.close();
+
+ // Access the global VBC through reflection to manipulate flushPtr
+ GlobalVirtualBufferCache globalVBC = (GlobalVirtualBufferCache) ncAppCtx.getVirtualBufferCache();
+
+ // Use reflection to access private fields
+ Field flushPtrField = GlobalVirtualBufferCache.class.getDeclaredField("flushPtr");
+ flushPtrField.setAccessible(true);
+
+ Field primaryIndexesField = GlobalVirtualBufferCache.class.getDeclaredField("primaryIndexes");
+ primaryIndexesField.setAccessible(true);
+
+ // Register memory components to add indexes to the list
+ // We already have primaryIndexes[0] and filteredPrimaryIndexes[0] registered
+ globalVBC.register(index2.getCurrentMemoryComponent());
+ globalVBC.register(index3.getCurrentMemoryComponent());
+
+ // Get the current state
+ @SuppressWarnings("unchecked")
+ List<ILSMIndex> primaryIndexes = (List<ILSMIndex>) primaryIndexesField.get(globalVBC);
+ int currentSize = primaryIndexes.size();
+
+ // Set flushPtr to point to the last index (should be currentSize - 1)
+ flushPtrField.set(globalVBC, currentSize - 1);
+ int flushPtrBefore = (Integer) flushPtrField.get(globalVBC);
+
+ // Verify we have the expected setup
+ Assert.assertTrue("Should have at least 3 indexes", currentSize >= 3);
+ Assert.assertEquals("flushPtr should point to last index", currentSize - 1, flushPtrBefore);
+
+ // Remove the last index (the one flushPtr is pointing to)
+ ILSMIndex lastIndex = primaryIndexes.get(currentSize - 1);
+ globalVBC.unregister(lastIndex.getCurrentMemoryComponent());
+
+ // Check the state after removal
+ int newSize = primaryIndexes.size();
+ int flushPtrAfter = (Integer) flushPtrField.get(globalVBC);
+
+ // This demonstrates the bug: flushPtr should be within bounds [0, newSize)
+ // With the current buggy code (flushPtr > pos), flushPtr would be (currentSize-1-1) % newSize
+ // But if flushPtr == pos (pointing to removed index), it's not adjusted and points out of bounds
+
+ Assert.assertEquals("List should be smaller after removal", currentSize - 1, newSize);
+
+ // The bug manifests when flushPtr equals the position of the removed index
+ // In this case, flushPtr is not adjusted and points beyond the list
+ if (flushPtrBefore == currentSize - 1) { // We removed the index flushPtr was pointing to
+ Assert.assertTrue("flushPtr should be within bounds after removal",
+ flushPtrAfter >= 0 && flushPtrAfter < newSize);
+ }
+
+ // Clean up
+ helper2.destroy();
+ helper3.destroy();
+
+ } catch (Exception e) {
+ LOGGER.error("testFlushPtrBoundsAfterRemovingLastIndex failed", e);
+ Assert.fail(e.getMessage());
+ }
+ }
+
private void initializeNc() throws Exception {
List<Pair<IOption, Object>> opts = new ArrayList<>();
opts.add(Pair.of(Option.STORAGE_MEMORYCOMPONENT_GLOBALBUDGET, 128 * 1024L));
diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/GlobalVirtualBufferCache.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/GlobalVirtualBufferCache.java
index f8771430e4..12c660da40 100644
--- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/GlobalVirtualBufferCache.java
+++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/GlobalVirtualBufferCache.java
@@ -74,7 +74,7 @@ public class GlobalVirtualBufferCache implements IVirtualBufferCache, ILifeCycle
private final Set<ILSMIndex> flushingIndexes = Collections.synchronizedSet(new HashSet<>());
private final Set<ILSMMemoryComponent> flushingComponents = Collections.synchronizedSet(new HashSet<>());
- private volatile int flushPtr;
+ private int flushPtr;
private final int filteredMemoryComponentMaxNumPages;
private final int flushPageBudget;
@@ -87,7 +87,7 @@ public class GlobalVirtualBufferCache implements IVirtualBufferCache, ILifeCycle
this.vbc = new VirtualBufferCache(allocator, storageProperties.getBufferCachePageSize(),
(int) (storageProperties.getMemoryComponentGlobalBudget()
/ storageProperties.getMemoryComponentPageSize()));
- this.flushPageBudget = (int) (storageProperties.getMemoryComponentGlobalBudget()
+ this.flushPageBudget = (int) ((double) storageProperties.getMemoryComponentGlobalBudget()
/ storageProperties.getMemoryComponentPageSize()
* storageProperties.getMemoryComponentFlushThreshold());
this.filteredMemoryComponentMaxNumPages = storageProperties.getFilteredMemoryComponentMaxNumPages();
@@ -145,7 +145,7 @@ public class GlobalVirtualBufferCache implements IVirtualBufferCache, ILifeCycle
}
if (primaryIndexes.isEmpty()) {
flushPtr = 0;
- } else if (flushPtr > pos) {
+ } else if (flushPtr >= pos) {
// If the removed index is before flushPtr, we should decrement flushPtr by 1 so that
// it still points to the same index.
flushPtr = (flushPtr - 1) % primaryIndexes.size();