CNDB-13305: change IndexComponentDiscovery#discoverComponents signature to use SSTableReader and add more loggings to index tracking (#1644)

Author: jasonstack

src/java/org/apache/cassandra/index/sai/IndexContext.java

@@ -925,7 +925,7 @@ public Pair<Set<SSTableIndex>, Set<SSTableContext>> getBuiltIndexes(Collection<S
             var perIndexComponents = perSSTableComponents.indexDescriptor().perIndexComponents(this);
             if (!perSSTableComponents.isComplete() || !perIndexComponents.isComplete())
             {
-                logger.debug(logMessage("An on-disk index build for SSTable {} has not completed."), context.descriptor());
+                logger.debug(logMessage("An on-disk index build for SSTable {} has not completed (per-index components={})."), context.descriptor(), perIndexComponents.all());
                 return;
             }
 

src/java/org/apache/cassandra/index/sai/SSTableContextManager.java

@@ -82,6 +82,7 @@ public Optional<Set<SSTableContext>> update(Collection<SSTableReader> removed, I
         {
             if (sstable.isMarkedCompacted())
             {
+                logger.debug("Skipped tracking sstable {} because it's marked compacted", sstable);
                 continue;
             }
 
@@ -94,6 +95,7 @@ public Optional<Set<SSTableContext>> update(Collection<SSTableReader> removed, I
                 // validation (it would fail), and we also don't want to add it to the returned contexts, since it's
                 // not ready yet. We know a future call of this method will be triggered for that sstable once the
                 // index finishes building.
+                logger.debug("Skipped tracking sstable {} because per sstable components are not complete (components={})", sstable, perSSTableComponents.all());
                 continue;
             }
 

src/java/org/apache/cassandra/index/sai/disk/format/DefaultIndexComponentDiscovery.java

@@ -19,16 +19,17 @@
 package org.apache.cassandra.index.sai.disk.format;
 
 import org.apache.cassandra.io.sstable.Descriptor;
+import org.apache.cassandra.io.sstable.format.SSTableReader;
 import org.apache.cassandra.schema.TableMetadata;
 
 public class DefaultIndexComponentDiscovery extends IndexComponentDiscovery
 {
     @Override
-    public SSTableIndexComponentsState discoverComponents(Descriptor descriptor, TableMetadata metadata)
+    public SSTableIndexComponentsState discoverComponents(SSTableReader sstable)
     {
-        SSTableIndexComponentsState groups = tryDiscoverComponentsFromTOC(descriptor);
+        SSTableIndexComponentsState groups = tryDiscoverComponentsFromTOC(sstable.getDescriptor());
         return groups == null
-               ? discoverComponentsFromDiskFallback(descriptor)
+               ? discoverComponentsFromDiskFallback(sstable.getDescriptor())
                : groups;
     }
 }

src/java/org/apache/cassandra/index/sai/disk/format/IndexComponentDiscovery.java

@@ -35,6 +35,7 @@
 import org.apache.cassandra.io.sstable.Component;
 import org.apache.cassandra.io.sstable.Descriptor;
 import org.apache.cassandra.io.sstable.SSTable;
+import org.apache.cassandra.io.sstable.format.SSTableReader;
 import org.apache.cassandra.io.util.PathUtils;
 import org.apache.cassandra.schema.TableMetadata;
 import org.apache.cassandra.utils.FBUtilities;
@@ -74,15 +75,14 @@ public static IndexComponentDiscovery instance()
      * Note that "discovery" in this method only means finding out the "build ID" (version and generation) that should
      * be used for each group of components (per-sstable and per-index).
      *
-     * @param descriptor the descriptor of the sstable for which to discover components.
-     * @param metadata the metadata of the table the sstable belongs to.
+     * @param sstable the sstable reader for which to discover components.
      * @return the discovered {@link ComponentsBuildId} to use for both per-sstable and each per-index components. The
      * returned build IDs should usually correspond to existing index components on disk but this is not a strong
      * asumption: if some group of components corresponding to the returned build ID has no completion marker or is
      * missing files, the group will not be usuable (and the corresponding index/indexes will not be usable) but this
      * should be handled "gracefully" by callers.
      */
-    public abstract SSTableIndexComponentsState discoverComponents(Descriptor descriptor, TableMetadata metadata);
+    public abstract SSTableIndexComponentsState discoverComponents(SSTableReader sstable);
 
     protected static IndexComponentType completionMarker(@Nullable String name)
     {

src/java/org/apache/cassandra/index/sai/disk/format/IndexDescriptor.java

@@ -52,7 +52,6 @@
 import org.apache.cassandra.io.storage.StorageProvider;
 import org.apache.cassandra.io.util.File;
 import org.apache.cassandra.io.util.FileHandle;
-import org.apache.cassandra.schema.TableMetadata;
 import org.apache.cassandra.utils.NoSpamLogger;
 import org.apache.lucene.store.BufferedChecksumIndexInput;
 import org.apache.lucene.store.ChecksumIndexInput;
@@ -108,7 +107,7 @@ public static IndexDescriptor empty(Descriptor descriptor)
 
     public static IndexDescriptor load(SSTableReader sstable, Set<IndexContext> indices)
     {
-        SSTableIndexComponentsState discovered = IndexComponentDiscovery.instance().discoverComponents(sstable.descriptor, sstable.metadata());
+        SSTableIndexComponentsState discovered = IndexComponentDiscovery.instance().discoverComponents(sstable);
         IndexDescriptor descriptor = new IndexDescriptor(sstable.descriptor);
         descriptor.initialize(indices, discovered);
         return descriptor;
@@ -221,13 +220,14 @@ private static Component customComponentFor(ComponentsBuildId buildId, IndexComp
      * where (post-flush) index building is done on separate dedicated services, and this method allows to reload the
      * result of such external services once it is made available locally.
      *
-     * @param metadata The metadata of the table this descriptor is for.
+     * @param sstable the sstable reader to reload index components
      * @param indices The set of indices to should part of the reloaded descriptor.
      * @return this descriptor, for chaining purpose.
      */
-    public IndexDescriptor reload(TableMetadata metadata, Set<IndexContext> indices)
+    public IndexDescriptor reload(SSTableReader sstable, Set<IndexContext> indices)
     {
-        SSTableIndexComponentsState discovered = IndexComponentDiscovery.instance().discoverComponents(descriptor, metadata);
+        Preconditions.checkArgument(sstable.getDescriptor().equals(this.descriptor));
+        SSTableIndexComponentsState discovered = IndexComponentDiscovery.instance().discoverComponents(sstable);
 
         // We want to make sure the descriptor only has data for the provided `indices` on reload, so we remove any
         // index data that is not in the ones provided. This essentially make sure we don't hold up memory for

test/unit/org/apache/cassandra/index/sai/disk/format/IndexDescriptorTest.java

@@ -39,8 +39,10 @@
 import org.apache.cassandra.index.sai.SAITester;
 import org.apache.cassandra.io.sstable.Component;
 import org.apache.cassandra.io.sstable.Descriptor;
+import org.apache.cassandra.io.sstable.format.SSTableReader;
 import org.apache.cassandra.io.util.File;
 import org.apache.cassandra.io.util.PathUtils;
+import org.mockito.Mockito;
 import org.mortbay.util.IO;
 
 import static org.apache.cassandra.index.sai.SAIUtil.setLatestVersion;
@@ -90,9 +92,9 @@ private IndexDescriptor loadDescriptor(IndexContext... contexts)
     static IndexDescriptor loadDescriptor(Descriptor sstableDescriptor, IndexContext... contexts)
     {
         IndexDescriptor indexDescriptor = IndexDescriptor.empty(sstableDescriptor);
-        // Note: passing `null` as metadata is a bit hacky, but it only exists to be passed to the underlying index
-        // discovery class, and the default one ignores it so ... keeping it simple.
-        indexDescriptor.reload(null, new HashSet<>(Arrays.asList(contexts)));
+        SSTableReader sstable = Mockito.mock(SSTableReader.class);
+        Mockito.when(sstable.getDescriptor()).thenReturn(sstableDescriptor);
+        indexDescriptor.reload(sstable, new HashSet<>(Arrays.asList(contexts)));
         return indexDescriptor;
     }
 
@@ -232,8 +234,9 @@ public void testReload() throws Throwable
         createFakePerSSTableComponents(descriptor, latest, 0);
         createFakePerIndexComponents(descriptor, indexContext, latest, 0);
 
-        // See comment on `loadDescriptor` regarding passing null
-        indexDescriptor.reload(null, Set.of(indexContext));
+        SSTableReader sstable = Mockito.mock(SSTableReader.class);
+        Mockito.when(sstable.getDescriptor()).thenReturn(descriptor);
+        indexDescriptor.reload(sstable, Set.of(indexContext));
 
         // Both the perSSTableComponents and perIndexComponents should now be complete and the components should be present
 

test/unit/org/apache/cassandra/index/sai/disk/v1/LegacyOnDiskFormatTest.java

@@ -97,7 +97,7 @@ public void setup() throws Throwable
                                      .addRegularColumn("text_value", UTF8Type.instance)
                                      .build();
         sstable = TrieIndexFormat.instance.getReaderFactory().openNoValidation(descriptor, TableMetadataRef.forOfflineTools(tableMetadata));
-        indexDescriptor = IndexDescriptor.empty(sstable.descriptor).reload(tableMetadata, Set.of(intContext, textContext));
+        indexDescriptor = IndexDescriptor.empty(sstable.descriptor).reload(sstable, Set.of(intContext, textContext));
         pkFactory = indexDescriptor.perSSTableComponents().version().onDiskFormat().newPrimaryKeyFactory(tableMetadata.comparator);
     }