This is an automated email from the ASF dual-hosted git repository.
maedhroz pushed a commit to branch cassandra-5.0
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/cassandra-5.0 by this push:
new e45c1092f9 Correctly remove Index.Group from IndexRegistry
e45c1092f9 is described below
commit e45c1092f91edd63591f562b2120ea6a5fd3edd5
Author: Mike Adamson <madam...@datastax.com>
AuthorDate: Wed Oct 4 11:27:50 2023 +0100
Correctly remove Index.Group from IndexRegistry
The Index.Group was being left in the list indexGroups in the
SecondaryIndexManager because the incorrect
key was being used to remove it from the map
patch by Mike Adamson; reviewed by Caleb Rackliffe and Zhao Yang for
CASSANDRA-18905
Co-authored-by: Zhao Yang <zhaoyangsingap...@gmail.com>
---
CHANGES.txt | 1 +
.../org/apache/cassandra/db/lifecycle/Tracker.java | 8 ++-
src/java/org/apache/cassandra/index/Index.java | 64 +++++++++++++++--
.../org/apache/cassandra/index/IndexRegistry.java | 26 +++++--
.../cassandra/index/SecondaryIndexManager.java | 60 +++++++---------
.../cassandra/index/SingletonIndexGroup.java | 17 +----
.../cassandra/index/sai/StorageAttachedIndex.java | 8 ++-
.../index/sai/StorageAttachedIndexGroup.java | 17 +++--
.../org/apache/cassandra/index/sasi/SASIIndex.java | 2 +-
.../apache/cassandra/index/CustomIndexTest.java | 75 +++++++++-----------
.../org/apache/cassandra/index/StubIndexGroup.java | 6 ++
.../index/sai/cql/IndexGroupLifecycleTest.java | 81 ++++++++++++++++++++++
.../index/sai/cql/StorageAttachedIndexDDLTest.java | 6 +-
.../index/sai/functional/CompactionTest.java | 5 +-
.../index/sai/metrics/IndexGroupMetricsTest.java | 4 +-
.../index/sai/metrics/QueryMetricsTest.java | 6 +-
.../index/sai/metrics/StateMetricsTest.java | 6 +-
17 files changed, 261 insertions(+), 131 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index 9377b7a421..9a49af3955 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
5.0-alpha2
+ * Correctly remove Index.Group from IndexRegistry (CASSANDRA-18905)
* Fix vector type to support DDM's mask_default function (CASSANDRA-18889)
* Remove unnecessary reporter-config3 dependency (CASSANDRA-18907)
* Remove support for empty values on the vector data type (CASSANDRA-18876)
diff --git a/src/java/org/apache/cassandra/db/lifecycle/Tracker.java
b/src/java/org/apache/cassandra/db/lifecycle/Tracker.java
index 061765bd52..e959c72fa9 100644
--- a/src/java/org/apache/cassandra/db/lifecycle/Tracker.java
+++ b/src/java/org/apache/cassandra/db/lifecycle/Tracker.java
@@ -86,7 +86,7 @@ public class Tracker
{
private static final Logger logger =
LoggerFactory.getLogger(Tracker.class);
- private final Collection<INotificationConsumer> subscribers = new
CopyOnWriteArrayList<>();
+ private final List<INotificationConsumer> subscribers = new
CopyOnWriteArrayList<>();
public final ColumnFamilyStore cfstore;
final AtomicReference<View> view;
@@ -560,6 +560,12 @@ public class Tracker
subscribers.add(consumer);
}
+ @VisibleForTesting
+ public boolean contains(INotificationConsumer consumer)
+ {
+ return subscribers.contains(consumer);
+ }
+
public void unsubscribe(INotificationConsumer consumer)
{
subscribers.remove(consumer);
diff --git a/src/java/org/apache/cassandra/index/Index.java
b/src/java/org/apache/cassandra/index/Index.java
index f116fdb3e0..fc5f5a6f00 100644
--- a/src/java/org/apache/cassandra/index/Index.java
+++ b/src/java/org/apache/cassandra/index/Index.java
@@ -23,6 +23,7 @@ package org.apache.cassandra.index;
import java.io.UncheckedIOException;
import java.util.Collection;
import java.util.Collections;
+import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.Callable;
@@ -55,8 +56,8 @@ import
org.apache.cassandra.index.transactions.IndexTransaction;
import org.apache.cassandra.io.sstable.Component;
import org.apache.cassandra.io.sstable.Descriptor;
import org.apache.cassandra.io.sstable.ReducingKeyIterator;
-import org.apache.cassandra.io.sstable.SSTableFlushObserver;
import org.apache.cassandra.io.sstable.SSTable;
+import org.apache.cassandra.io.sstable.SSTableFlushObserver;
import org.apache.cassandra.io.sstable.format.SSTableReader;
import org.apache.cassandra.schema.ColumnMetadata;
import org.apache.cassandra.schema.IndexMetadata;
@@ -275,6 +276,17 @@ public interface Index
*/
public void register(IndexRegistry registry);
+ /**
+ * Unregister current index when it's removed from system
+ *
+ * @param registry the index registry to register the instance with
+ */
+ default void unregister(IndexRegistry registry)
+ {
+ // for singleton index, the group key is the index itself
+ registry.unregisterIndex(this, new Index.Group.Key(this));
+ }
+
/**
* If the index implementation uses a local table to store its index data,
this method should return a
* handle to it. If not, an empty {@link Optional} should be returned.
This exists to support legacy
@@ -677,11 +689,39 @@ public interface Index
* Class providing grouped operations for indexes that communicate with
each other.
*
* Index implementations should provide a {@code Group} implementation
calling to
- * {@link SecondaryIndexManager#registerIndex(Index, Object, Supplier)}
during index registering
+ * {@link SecondaryIndexManager#registerIndex(Index, Index.Group.Key,
Supplier)} during index registering
* at {@link #register(IndexRegistry)} method.
*/
interface Group
{
+ /**
+ * Group key is used to uniquely identify a {@link Group} within a
table
+ */
+ class Key
+ {
+ private final Object object;
+
+ public Key(Object object)
+ {
+ this.object = object;
+ }
+
+ @Override
+ public boolean equals(Object o)
+ {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ Key key = (Key) o;
+ return Objects.equals(object, key.object);
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return Objects.hash(object);
+ }
+ }
+
/**
* Returns the indexes that are members of this group.
*
@@ -694,14 +734,16 @@ public interface Index
*
* @param index the index to be added
*/
- void addIndex(Index index);
+ default void addIndex(Index index)
+ {}
/**
* Removes the specified {@link Index} from the members of this group.
*
* @param index the index to be removed
*/
- void removeIndex(Index index);
+ default void removeIndex(Index index)
+ {}
/**
* Returns if this group contains the specified {@link Index}.
@@ -711,6 +753,16 @@ public interface Index
*/
boolean containsIndex(Index index);
+ /**
+ * Returns whether this group can only ever contain a single index.
+ *
+ * @return {@code true} if this group only contains a single index,
{@code false} otherwise
+ */
+ default boolean isSingleton()
+ {
+ return true;
+ }
+
/**
* Creates an new {@code Indexer} object for updates to a given
partition.
*
@@ -769,8 +821,8 @@ public interface Index
}
/**
- * Called when the table associated with this group has been
invalidated. Implementations
- * should dispose of any resources tied to the lifecycle of the {@link
Group}.
+ * Called when the table associated with this group has been
invalidated or all indexes in the group are removed.
+ * Implementations should dispose of any resources tied to the
lifecycle of the {@link Group}.
*/
default void invalidate() { }
diff --git a/src/java/org/apache/cassandra/index/IndexRegistry.java
b/src/java/org/apache/cassandra/index/IndexRegistry.java
index 308aeacd7a..46e87f357f 100644
--- a/src/java/org/apache/cassandra/index/IndexRegistry.java
+++ b/src/java/org/apache/cassandra/index/IndexRegistry.java
@@ -64,7 +64,12 @@ public interface IndexRegistry
IndexRegistry EMPTY = new IndexRegistry()
{
@Override
- public void registerIndex(Index index, Object groupKey,
Supplier<Index.Group> groupSupplier)
+ public void registerIndex(Index index, Index.Group.Key groupKey,
Supplier<Index.Group> groupSupplier)
+ {
+ }
+
+ @Override
+ public void unregisterIndex(Index index, Index.Group.Key groupKey)
{
}
@@ -125,7 +130,11 @@ public interface IndexRegistry
public void register(IndexRegistry registry)
{
+ }
+ @Override
+ public void unregister(IndexRegistry registry)
+ {
}
public Optional<ColumnFamilyStore> getBackingTable()
@@ -245,7 +254,12 @@ public interface IndexRegistry
}
};
- public void registerIndex(Index index, Object groupKey,
Supplier<Index.Group> groupSupplier)
+ public void registerIndex(Index index, Index.Group.Key groupKey,
Supplier<Index.Group> groupSupplier)
+ {
+ }
+
+ @Override
+ public void unregisterIndex(Index index, Index.Group.Key groupKey)
{
}
@@ -277,9 +291,13 @@ public interface IndexRegistry
default void registerIndex(Index index)
{
- registerIndex(index, index, () -> new SingletonIndexGroup(index));
+ registerIndex(index, new Index.Group.Key(index), () -> new
SingletonIndexGroup(index));
}
- void registerIndex(Index index, Object groupKey, Supplier<Index.Group>
groupSupplier);
+
+ void registerIndex(Index index, Index.Group.Key groupKey,
Supplier<Index.Group> groupSupplier);
+
+ void unregisterIndex(Index index, Index.Group.Key groupKey);
+
Collection<Index.Group> listIndexGroups();
Index getIndex(IndexMetadata indexMetadata);
diff --git a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java
b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java
index 19985c304f..9fd24bcec3 100644
--- a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java
+++ b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java
@@ -162,7 +162,7 @@ public class SecondaryIndexManager implements
IndexRegistry, INotificationConsum
/**
* The groups of all the registered indexes
*/
- private final Map<Object, Index.Group> indexGroups =
Maps.newConcurrentMap();
+ private final Map<Index.Group.Key, Index.Group> indexGroups =
Maps.newConcurrentMap();
/**
* The count of pending index builds for each index.
@@ -344,15 +344,17 @@ public class SecondaryIndexManager implements
IndexRegistry, INotificationConsum
public synchronized void removeIndex(String indexName)
{
- Index index = unregisterIndex(indexName);
- if (null != index)
+ Index removedIndex = indexes.remove(indexName);
+
+ if (removedIndex != null)
{
+ removedIndex.unregister(this);
+
markIndexRemoved(indexName);
- executeBlocking(index.getInvalidateTask(), null);
+ executeBlocking(removedIndex.getInvalidateTask(), null);
}
}
-
public Set<IndexMetadata> getDependentIndexes(ColumnMetadata column)
{
if (indexes.isEmpty())
@@ -1302,7 +1304,8 @@ public class SecondaryIndexManager implements
IndexRegistry, INotificationConsum
/*
* IndexRegistry methods
*/
- public void registerIndex(Index index, Object groupKey,
Supplier<Index.Group> groupSupplier)
+ @Override
+ public void registerIndex(Index index, Index.Group.Key groupKey,
Supplier<Index.Group> groupSupplier)
{
String name = index.getIndexMetadata().name;
indexes.put(name, index);
@@ -1312,41 +1315,26 @@ public class SecondaryIndexManager implements
IndexRegistry, INotificationConsum
Index.Group group = indexGroups.computeIfAbsent(groupKey, k ->
groupSupplier.get());
// add the created index to its group if it is not a singleton group
- if (!(group instanceof SingletonIndexGroup))
- {
- if (index.getBackingTable().isPresent())
- throw new InvalidRequestException("Indexes belonging to a
group of indexes shouldn't have a backing table");
-
- group.addIndex(index);
- }
+ group.addIndex(index);
}
- private Index unregisterIndex(String name)
+ @Override
+ public void unregisterIndex(Index removed, Index.Group.Key groupKey)
{
- Index removed = indexes.remove(name);
- logger.trace(removed == null ? "Index {} was not registered" :
"Removed index {} from registry", name);
-
- if (removed != null)
+ Index.Group group = indexGroups.get(groupKey);
+ if (group != null && group.containsIndex(removed))
{
- // Remove the index from any non-singleton groups...
- for (Index.Group group : listIndexGroups())
- {
- if (!(group instanceof SingletonIndexGroup) &&
group.containsIndex(removed))
- {
- group.removeIndex(removed);
+ // Remove the index from non-singleton groups...
+ group.removeIndex(removed);
- if (group.getIndexes().isEmpty())
- {
- indexGroups.remove(group);
- }
- }
+ // if the group is a singleton or there are no more indexes left
in the group, remove it
+ if (group.isSingleton() || group.getIndexes().isEmpty())
+ {
+ Index.Group removedGroup = indexGroups.remove(groupKey);
+ if (removedGroup != null)
+ removedGroup.invalidate();
}
-
- // ...and remove singleton groups entirely.
- indexGroups.remove(removed);
}
-
- return removed;
}
public Index getIndex(IndexMetadata metadata)
@@ -1364,14 +1352,14 @@ public class SecondaryIndexManager implements
IndexRegistry, INotificationConsum
return ImmutableSet.copyOf(indexGroups.values());
}
- public Index.Group getIndexGroup(Object key)
+ public Index.Group getIndexGroup(Index.Group.Key key)
{
return indexGroups.get(key);
}
/**
* Returns the {@link Index.Group} the specified index belongs to, as
specified during registering with
- * {@link #registerIndex(Index, Object, Supplier)}.
+ * {@link #registerIndex(Index, Index.Group.Key, Supplier)}.
*
* @param metadata the index metadata
* @return the group the index belongs to, or {@code null} if the index is
not registered or if it hasn't been
diff --git a/src/java/org/apache/cassandra/index/SingletonIndexGroup.java
b/src/java/org/apache/cassandra/index/SingletonIndexGroup.java
index 304d35d6d4..162247fd74 100644
--- a/src/java/org/apache/cassandra/index/SingletonIndexGroup.java
+++ b/src/java/org/apache/cassandra/index/SingletonIndexGroup.java
@@ -62,18 +62,6 @@ public class SingletonIndexGroup implements Index.Group
return delegate;
}
- @Override
- public void addIndex(Index index)
- {
- throw new UnsupportedOperationException();
- }
-
- @Override
- public void removeIndex(Index index)
- {
- throw new UnsupportedOperationException();
- }
-
@Override
public boolean containsIndex(Index index)
{
@@ -89,9 +77,8 @@ public class SingletonIndexGroup implements Index.Group
IndexTransaction.Type transactionType,
Memtable memtable)
{
- return indexSelector.test(delegate)
- ? delegate.indexerFor(key, columns, nowInSec, ctx,
transactionType, memtable)
- : null;
+ return indexSelector.test(delegate) ? delegate.indexerFor(key,
columns, nowInSec, ctx, transactionType, memtable)
+ : null;
}
@Override
diff --git a/src/java/org/apache/cassandra/index/sai/StorageAttachedIndex.java
b/src/java/org/apache/cassandra/index/sai/StorageAttachedIndex.java
index a0cdf9c3ac..b09f04d80d 100644
--- a/src/java/org/apache/cassandra/index/sai/StorageAttachedIndex.java
+++ b/src/java/org/apache/cassandra/index/sai/StorageAttachedIndex.java
@@ -272,7 +272,13 @@ public class StorageAttachedIndex implements Index
public void register(IndexRegistry registry)
{
// index will be available for writes
- registry.registerIndex(this, StorageAttachedIndexGroup.class, () ->
new StorageAttachedIndexGroup(baseCfs));
+ registry.registerIndex(this, StorageAttachedIndexGroup.GROUP_KEY, ()
-> new StorageAttachedIndexGroup(baseCfs));
+ }
+
+ @Override
+ public void unregister(IndexRegistry registry)
+ {
+ registry.unregisterIndex(this, StorageAttachedIndexGroup.GROUP_KEY);
}
@Override
diff --git
a/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexGroup.java
b/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexGroup.java
index de4c8b3570..5dac6edd47 100644
--- a/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexGroup.java
+++ b/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexGroup.java
@@ -22,6 +22,7 @@ import java.util.Collections;
import java.util.HashSet;
import java.util.Objects;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
@@ -29,7 +30,6 @@ import javax.annotation.concurrent.ThreadSafe;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Sets;
import com.google.common.primitives.Ints;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -75,10 +75,12 @@ public class StorageAttachedIndexGroup implements
Index.Group, INotificationCons
{
private static final Logger logger =
LoggerFactory.getLogger(StorageAttachedIndexGroup.class);
+ public static final Index.Group.Key GROUP_KEY = new
Index.Group.Key(StorageAttachedIndexGroup.class);
+
private final TableQueryMetrics queryMetrics;
private final TableStateMetrics stateMetrics;
private final IndexGroupMetrics groupMetrics;
- private final Set<StorageAttachedIndex> indexes =
Sets.newConcurrentHashSet();
+ private final Set<StorageAttachedIndex> indexes =
ConcurrentHashMap.newKeySet();
private final ColumnFamilyStore baseCfs;
private final SSTableContextManager contextManager;
@@ -98,7 +100,7 @@ public class StorageAttachedIndexGroup implements
Index.Group, INotificationCons
@Nullable
public static StorageAttachedIndexGroup getIndexGroup(ColumnFamilyStore
cfs)
{
- return (StorageAttachedIndexGroup)
cfs.indexManager.getIndexGroup(StorageAttachedIndexGroup.class);
+ return (StorageAttachedIndexGroup)
cfs.indexManager.getIndexGroup(StorageAttachedIndexGroup.GROUP_KEY);
}
@Override
@@ -128,14 +130,13 @@ public class StorageAttachedIndexGroup implements
Index.Group, INotificationCons
for (SSTableReader sstable : contextManager.sstables())
sstable.unregisterComponents(IndexDescriptor.create(sstable).getLivePerSSTableComponents(),
baseCfs.getTracker());
deletePerSSTableFiles(baseCfs.getLiveSSTables());
- baseCfs.getTracker().unsubscribe(this);
}
}
@Override
public void invalidate()
{
- // in case of dropping table, sstable contexts should already been
removed by SSTableListChangedNotification.
+ // in case of removing last index from group, sstable contexts should
already been removed by removeIndex
queryMetrics.release();
groupMetrics.release();
stateMetrics.release();
@@ -149,6 +150,12 @@ public class StorageAttachedIndexGroup implements
Index.Group, INotificationCons
return indexes.contains(index);
}
+ @Override
+ public boolean isSingleton()
+ {
+ return false;
+ }
+
@Override
public Index.Indexer indexerFor(Predicate<Index> indexSelector,
DecoratedKey key,
diff --git a/src/java/org/apache/cassandra/index/sasi/SASIIndex.java
b/src/java/org/apache/cassandra/index/sasi/SASIIndex.java
index 1ae3a04943..93448f9e78 100644
--- a/src/java/org/apache/cassandra/index/sasi/SASIIndex.java
+++ b/src/java/org/apache/cassandra/index/sasi/SASIIndex.java
@@ -186,7 +186,7 @@ public class SASIIndex implements Index,
INotificationConsumer
@Override
public void register(IndexRegistry registry)
{
- registry.registerIndex(this, this, () -> new SASIIndexGroup(this));
+ registry.registerIndex(this, new Group.Key(this), () -> new
SASIIndexGroup(this));
}
public IndexMetadata getIndexMetadata()
diff --git a/test/unit/org/apache/cassandra/index/CustomIndexTest.java
b/test/unit/org/apache/cassandra/index/CustomIndexTest.java
index 5c5f89f644..79987f34b1 100644
--- a/test/unit/org/apache/cassandra/index/CustomIndexTest.java
+++ b/test/unit/org/apache/cassandra/index/CustomIndexTest.java
@@ -25,6 +25,7 @@ import java.util.concurrent.Callable;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Predicate;
+import java.util.function.Supplier;
import java.util.stream.Collectors;
import com.google.common.collect.ImmutableList;
@@ -1399,13 +1400,13 @@ public class CustomIndexTest extends CQLTester
// create two indexes belonging to the same group and verify that only
one group is added to the manager
String idx1 = createIndex(String.format("CREATE CUSTOM INDEX ON
%%s(v1) USING '%s'", indexClassName));
String idx2 = createIndex(String.format("CREATE CUSTOM INDEX ON
%%s(v2) USING '%s'", indexClassName));
- IndexWithSharedGroup.Group group = indexManager.listIndexGroups()
- .stream()
- .filter(g -> g
instanceof IndexWithSharedGroup.Group)
- .map(g ->
(IndexWithSharedGroup.Group) g)
- .findAny()
-
.orElseThrow(AssertionError::new);
-
+ Supplier<IndexWithSharedGroup.Group> groupSupplier =
+ () -> indexManager.listIndexGroups().stream()
+ .filter(g -> g instanceof
IndexWithSharedGroup.Group)
+ .map(g ->
(IndexWithSharedGroup.Group) g)
+ .findAny()
+ .orElse(null);
+ IndexWithSharedGroup.Group group = groupSupplier.get();
// verify that only one group has been added to the manager
assertEquals(2, indexManager.listIndexes().size());
assertEquals(1, indexManager.listIndexGroups().size());
@@ -1435,20 +1436,26 @@ public class CustomIndexTest extends CQLTester
assertEquals(2, indexManager.listIndexes().size());
assertEquals(1, indexManager.listIndexGroups().size());
- // drop the remaining members of the shared group and verify that it
is kept empty in the manager
+ // drop the remaining members of the shared group and verify that it
no longer exists in the manager
dropIndex("DROP INDEX %s." + idx2);
dropIndex("DROP INDEX %s." + idx5);
assertEquals(0, indexManager.listIndexes().size());
- assertEquals(1, indexManager.listIndexGroups().size());
+ assertEquals(0, indexManager.listIndexGroups().size());
assertEquals(0, group.indexes.size());
- // create the sharing group members again and verify that they are
added to the existing group instance
+ // create the sharing group members again and verify that they are
added to a new group instance
createIndex(String.format("CREATE CUSTOM INDEX %s ON %%s(v1) USING
'%s'", idx1, indexClassName));
createIndex(String.format("CREATE CUSTOM INDEX %s ON %%s(v2) USING
'%s'", idx2, indexClassName));
createIndex(String.format("CREATE CUSTOM INDEX %s ON %%s(v3) USING
'%s'", idx3, indexClassName));
+ IndexWithSharedGroup.Group newGroup = indexManager.listIndexGroups()
+ .stream()
+ .filter(g -> g
instanceof IndexWithSharedGroup.Group)
+ .map(g ->
(IndexWithSharedGroup.Group) g)
+ .findAny()
+
.orElseThrow(AssertionError::new);
assertEquals(3, indexManager.listIndexes().size());
assertEquals(1, indexManager.listIndexGroups().size());
- assertEquals(3, group.indexes.size());
+ assertEquals(3, newGroup.indexes.size());
}
/**
@@ -1471,7 +1478,13 @@ public class CustomIndexTest extends CQLTester
@Override
public void register(IndexRegistry registry)
{
- registry.registerIndex(this, Group.class, Group::new);
+ registry.registerIndex(this, new Group.Key(Group.class),
Group::new);
+ }
+
+ @Override
+ public void unregister(IndexRegistry registry)
+ {
+ registry.unregisterIndex(this, new Group.Key(Group.class));
}
private static class Group implements Index.Group
@@ -1533,6 +1546,12 @@ public class CustomIndexTest extends CQLTester
return indexes.containsKey(index.getIndexMetadata().name);
}
+ @Override
+ public boolean isSingleton()
+ {
+ return false;
+ }
+
@Override
public Index.Indexer indexerFor(Predicate<Index> indexSelector,
DecoratedKey key,
@@ -1663,36 +1682,4 @@ public class CustomIndexTest extends CQLTester
}
}
}
-
- @Test
- public void testMulticolumnIndexWithBaseTable() throws Throwable
- {
- createTable("CREATE TABLE %s(k int PRIMARY KEY, v int)");
- assertInvalidMessage("Indexes belonging to a group of indexes
shouldn't have a backing table",
- String.format("CREATE CUSTOM INDEX ON %%s(v)
USING '%s'",
-
MulticolumnIndexWithBaseTable.class.getName()));
- }
-
- public static final class MulticolumnIndexWithBaseTable extends StubIndex
- {
- private final ColumnFamilyStore baseCfs;
-
- public MulticolumnIndexWithBaseTable(ColumnFamilyStore baseCfs,
IndexMetadata metadata)
- {
- super(baseCfs, metadata);
- this.baseCfs = baseCfs;
- }
-
- @Override
- public void register(IndexRegistry registry)
- {
- registry.registerIndex(this, MulticolumnIndexWithBaseTable.class,
StubIndexGroup::new);
- }
-
- @Override
- public Optional<ColumnFamilyStore> getBackingTable()
- {
- return Optional.of(baseCfs);
- }
- }
}
diff --git a/test/unit/org/apache/cassandra/index/StubIndexGroup.java
b/test/unit/org/apache/cassandra/index/StubIndexGroup.java
index 0b6ad1c37d..22dfbe262b 100644
--- a/test/unit/org/apache/cassandra/index/StubIndexGroup.java
+++ b/test/unit/org/apache/cassandra/index/StubIndexGroup.java
@@ -67,6 +67,12 @@ public class StubIndexGroup implements Index.Group
return indexes.contains(index);
}
+ @Override
+ public boolean isSingleton()
+ {
+ return false;
+ }
+
@Override
public Index.Indexer indexerFor(Predicate<Index> indexSelector,
DecoratedKey key,
diff --git
a/test/unit/org/apache/cassandra/index/sai/cql/IndexGroupLifecycleTest.java
b/test/unit/org/apache/cassandra/index/sai/cql/IndexGroupLifecycleTest.java
new file mode 100644
index 0000000000..13965439ab
--- /dev/null
+++ b/test/unit/org/apache/cassandra/index/sai/cql/IndexGroupLifecycleTest.java
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.index.sai.cql;
+
+import org.junit.Test;
+
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.db.lifecycle.Tracker;
+import org.apache.cassandra.index.sai.SAITester;
+import org.apache.cassandra.index.sai.StorageAttachedIndexGroup;
+
+import static java.lang.String.format;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+public class IndexGroupLifecycleTest extends SAITester
+{
+ @Test
+ public void testDropAndRecreate() throws Throwable
+ {
+ createTable("CREATE TABLE %s (pk text, value text, PRIMARY KEY (pk))");
+ populateOneSSTable();
+
+ ColumnFamilyStore cfs = getCurrentColumnFamilyStore();
+ cfs.disableAutoCompaction();
+ Tracker tracker = cfs.getTracker();
+
+ // create index and drop it: StorageAttachedIndexGroup should be
removed
+ createIndex("CREATE CUSTOM INDEX sai ON %s(value) USING
'StorageAttachedIndex'");
+
+ StorageAttachedIndexGroup group = (StorageAttachedIndexGroup)
cfs.indexManager.getIndexGroup(StorageAttachedIndexGroup.GROUP_KEY);
+ assertTrue(tracker.contains(group));
+ assertEquals(1, group.sstableContextManager().size());
+
+ dropIndex(format("DROP INDEX %s.sai", KEYSPACE));
+ assertFalse(tracker.contains(group));
+ assertEquals(0, group.sstableContextManager().size()); // sstable
should be cleared from old group
+
assertNull(cfs.indexManager.getIndexGroup(StorageAttachedIndexGroup.GROUP_KEY));
+
+ // populate 2nd sstable. Old group should not track it
+ populateOneSSTable();
+ assertEquals(0, group.sstableContextManager().size());
+
+ // create index again: expect a new StorageAttachedIndexGroup to be
registered into tracker
+ createIndex("CREATE CUSTOM INDEX sai ON %s(value) USING
'StorageAttachedIndex'");
+
+ StorageAttachedIndexGroup newGroup = (StorageAttachedIndexGroup)
cfs.indexManager.getIndexGroup(StorageAttachedIndexGroup.GROUP_KEY);
+ assertNotSame(group, newGroup);
+ assertTrue(tracker.contains(newGroup));
+ assertEquals(2, newGroup.sstableContextManager().size());
+
+ // populate 3rd sstable. new group should track it
+ populateOneSSTable();
+ assertEquals(3, newGroup.sstableContextManager().size());
+ }
+
+ private void populateOneSSTable()
+ {
+ execute("INSERT INTO %s(pk, value) VALUES('k', 'v')");
+ flush();
+ }
+}
\ No newline at end of file
diff --git
a/test/unit/org/apache/cassandra/index/sai/cql/StorageAttachedIndexDDLTest.java
b/test/unit/org/apache/cassandra/index/sai/cql/StorageAttachedIndexDDLTest.java
index 6dedd5ed2d..f10149855d 100644
---
a/test/unit/org/apache/cassandra/index/sai/cql/StorageAttachedIndexDDLTest.java
+++
b/test/unit/org/apache/cassandra/index/sai/cql/StorageAttachedIndexDDLTest.java
@@ -84,6 +84,7 @@ import static java.util.Collections.singletonList;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.when;
@@ -667,8 +668,7 @@ public class StorageAttachedIndexDDLTest extends SAITester
dropIndex("DROP INDEX %s." + literalIndexName);
verifyIndexFiles(numericIndexContext, literalIndexContext, 0, 0);
- verifySSTableIndexes(numericIndexName, 0);
- verifySSTableIndexes(literalIndexName, 0);
+ assertNull(getCurrentIndexGroup());
assertEquals("Segment memory limiter should revert to zero on drop.",
0L, getSegmentBufferUsedBytes());
assertEquals("There should be no segment builders in progress.", 0L,
getColumnIndexBuildsInProgress());
@@ -1182,7 +1182,7 @@ public class StorageAttachedIndexDDLTest extends SAITester
delayIndexBuilderCompletion.disable();
- verifySSTableIndexes(indexName, 0);
+ assertNull(getCurrentIndexGroup());
assertFalse("Expect index not built",
SystemKeyspace.isIndexBuilt(KEYSPACE, indexName));
// create index again, it should succeed
diff --git
a/test/unit/org/apache/cassandra/index/sai/functional/CompactionTest.java
b/test/unit/org/apache/cassandra/index/sai/functional/CompactionTest.java
index da9a051f4f..aff3c5e862 100644
--- a/test/unit/org/apache/cassandra/index/sai/functional/CompactionTest.java
+++ b/test/unit/org/apache/cassandra/index/sai/functional/CompactionTest.java
@@ -61,8 +61,8 @@ import org.apache.cassandra.utils.TimeUUID;
import org.apache.cassandra.utils.concurrent.Refs;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -339,8 +339,7 @@ public class CompactionTest extends SAITester
}
// verify index group metrics are cleared.
- assertEquals(0, getOpenIndexFiles());
- assertEquals(0, getDiskUsage());
+ assertNull(getCurrentIndexGroup());
// verify indexes are dropped
// verify indexes are dropped
diff --git
a/test/unit/org/apache/cassandra/index/sai/metrics/IndexGroupMetricsTest.java
b/test/unit/org/apache/cassandra/index/sai/metrics/IndexGroupMetricsTest.java
index 7a54ebfc86..26a7ab6714 100644
---
a/test/unit/org/apache/cassandra/index/sai/metrics/IndexGroupMetricsTest.java
+++
b/test/unit/org/apache/cassandra/index/sai/metrics/IndexGroupMetricsTest.java
@@ -26,6 +26,7 @@ import org.apache.cassandra.index.sai.disk.format.Version;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
public class IndexGroupMetricsTest extends AbstractMetricsTest
{
@@ -76,8 +77,7 @@ public class IndexGroupMetricsTest extends AbstractMetricsTest
// drop last index, no open index files
dropIndex("DROP INDEX %s." + v1IndexName);
- assertEquals(0, getOpenIndexFiles());
- assertEquals(0, getDiskUsage());
+ assertNull(getCurrentIndexGroup());
}
protected int getOpenIndexFiles()
diff --git
a/test/unit/org/apache/cassandra/index/sai/metrics/QueryMetricsTest.java
b/test/unit/org/apache/cassandra/index/sai/metrics/QueryMetricsTest.java
index 14931b173b..e1f1c6933d 100644
--- a/test/unit/org/apache/cassandra/index/sai/metrics/QueryMetricsTest.java
+++ b/test/unit/org/apache/cassandra/index/sai/metrics/QueryMetricsTest.java
@@ -91,12 +91,8 @@ public class QueryMetricsTest extends AbstractMetricsTest
assertEquals(1L, getTableQueryMetrics(keyspace, table,
"TotalQueriesCompleted"));
- // Even if we drop the last index on the table, table-level metrics
should still be visible:
+ // If we drop the last index on the table we should no longer see the
table-level state metrics:
dropIndex(String.format("DROP INDEX %s." + index, keyspace));
- assertEquals(1L, getTableQueryMetrics(keyspace, table,
"TotalQueriesCompleted"));
-
- // When the whole table is dropped, we should finally fail to find
table-level metrics:
- dropTable(String.format("DROP TABLE %s." + table, keyspace));
assertThatThrownBy(() -> getTableQueryMetrics(keyspace, table,
"TotalQueriesCompleted")).hasCauseInstanceOf(InstanceNotFoundException.class);
}
diff --git
a/test/unit/org/apache/cassandra/index/sai/metrics/StateMetricsTest.java
b/test/unit/org/apache/cassandra/index/sai/metrics/StateMetricsTest.java
index e6ecda02fb..8e83293e61 100644
--- a/test/unit/org/apache/cassandra/index/sai/metrics/StateMetricsTest.java
+++ b/test/unit/org/apache/cassandra/index/sai/metrics/StateMetricsTest.java
@@ -55,12 +55,8 @@ public class StateMetricsTest extends AbstractMetricsTest
assertEquals(1, rows.all().size());
assertEquals(1L, getTableStateMetrics(keyspace, table,
"TotalIndexCount"));
- // If we drop the last index on the table, table-level state metrics
should still be visible:
+ // If we drop the last index on the table, we should no longer see the
table-level state metrics:
dropIndex(String.format("DROP INDEX %s." + index, keyspace));
- assertEquals(0L, getTableStateMetrics(keyspace, table,
"TotalIndexCount"));
-
- // When the whole table is dropped, we should finally fail to find
table-level state metrics:
- dropTable(String.format("DROP TABLE %s." + table, keyspace));
assertThatThrownBy(() -> getTableStateMetrics(keyspace, table,
"TotalIndexCount")).hasCauseInstanceOf(InstanceNotFoundException.class);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
