CNDB-12620 Users should get INDEX_BUILD_IN_PROGRESS instead of INDEX NOT AVAILABLE when they cannot query because an index is building.

We add a new reason, `RequestFailureReason.INDEX_BUILD_IN_PROGRESS`, to differentiate indexes that are currently building in error messages

Co-authored-by: Sergio Bossa <[email protected]>

Author: ekaterinadimitrova2

src/java/org/apache/cassandra/exceptions/RequestFailureReason.java

@@ -18,17 +18,20 @@
 package org.apache.cassandra.exceptions;
 
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 
 import com.google.common.primitives.Ints;
 
 import org.apache.cassandra.db.filter.TombstoneOverwhelmingException;
+import org.apache.cassandra.index.IndexBuildInProgressException;
+import org.apache.cassandra.index.IndexNotAvailableException;
 import org.apache.cassandra.index.sai.utils.AbortedOperationException;
 import org.apache.cassandra.io.IVersionedSerializer;
 import org.apache.cassandra.io.util.DataInputPlus;
 import org.apache.cassandra.io.util.DataOutputPlus;
 import org.apache.cassandra.utils.vint.VIntCoding;
 
-import static java.lang.Math.max;
 import static org.apache.cassandra.net.MessagingService.VERSION_40;
 
 public enum RequestFailureReason
@@ -37,10 +40,13 @@ public enum RequestFailureReason
     READ_TOO_MANY_TOMBSTONES (1),
     TIMEOUT                  (2),
     INCOMPATIBLE_SCHEMA      (3),
-    INDEX_NOT_AVAILABLE      (4),
-    UNKNOWN_COLUMN           (5),
-    UNKNOWN_TABLE            (6),
-    REMOTE_STORAGE_FAILURE   (7);
+    INDEX_NOT_AVAILABLE      (6), // We match it to Apache Cassandra's INDEX_NOT_AVAILABLE code introduced in 5.0
+    // The following codes are not present in Apache Cassandra's RequestFailureReason
+    // We should add new codes in HCD (which do not exist in Apache Cassandra) only with big numbers, to avoid conflicts
+    UNKNOWN_COLUMN           (500),
+    UNKNOWN_TABLE            (501),
+    REMOTE_STORAGE_FAILURE   (502),
+    INDEX_BUILD_IN_PROGRESS  (503);
 
     public static final Serializer serializer = new Serializer();
 
@@ -58,26 +64,29 @@ public int codeForNativeProtocol()
         return code;
     }
 
-    private static final RequestFailureReason[] codeToReasonMap;
+    private static final Map<Integer, RequestFailureReason> codeToReasonMap = new HashMap<>();
+    private static final Map<Class<? extends Throwable>, RequestFailureReason> exceptionToReasonMap = new HashMap<>();
 
     static
     {
         RequestFailureReason[] reasons = values();
 
-        int max = -1;
-        for (RequestFailureReason r : reasons)
-            max = max(r.code, max);
-
-        RequestFailureReason[] codeMap = new RequestFailureReason[max + 1];
-
         for (RequestFailureReason reason : reasons)
         {
-            if (codeMap[reason.code] != null)
+            if (codeToReasonMap.put(reason.code, reason) != null)
                 throw new RuntimeException("Two RequestFailureReason-s that map to the same code: " + reason.code);
-            codeMap[reason.code] = reason;
         }
 
-        codeToReasonMap = codeMap;
+        exceptionToReasonMap.put(TombstoneOverwhelmingException.class, READ_TOO_MANY_TOMBSTONES);
+        exceptionToReasonMap.put(IncompatibleSchemaException.class, INCOMPATIBLE_SCHEMA);
+        exceptionToReasonMap.put(AbortedOperationException.class, TIMEOUT);
+        exceptionToReasonMap.put(IndexNotAvailableException.class, INDEX_NOT_AVAILABLE);
+        exceptionToReasonMap.put(UnknownColumnException.class, UNKNOWN_COLUMN);
+        exceptionToReasonMap.put(UnknownTableException.class, UNKNOWN_TABLE);
+        exceptionToReasonMap.put(IndexBuildInProgressException.class, INDEX_BUILD_IN_PROGRESS);
+
+        if (exceptionToReasonMap.size() != reasons.length-2)
+            throw new RuntimeException("A new RequestFailureReasons was probably added and you may need to update the exceptionToReasonMap");
     }
 
     public static RequestFailureReason fromCode(int code)
@@ -86,19 +95,18 @@ public static RequestFailureReason fromCode(int code)
             throw new IllegalArgumentException("RequestFailureReason code must be non-negative (got " + code + ')');
 
         // be forgiving and return UNKNOWN if we aren't aware of the code - for forward compatibility
-        return code < codeToReasonMap.length ? codeToReasonMap[code] : UNKNOWN;
+        return codeToReasonMap.getOrDefault(code, UNKNOWN);
     }
 
     public static RequestFailureReason forException(Throwable t)
     {
-        if (t instanceof TombstoneOverwhelmingException)
-            return READ_TOO_MANY_TOMBSTONES;
-
-        if (t instanceof IncompatibleSchemaException)
-            return INCOMPATIBLE_SCHEMA;
+        RequestFailureReason r = exceptionToReasonMap.get(t.getClass());
+        if (r != null)
+            return r;
 
-        if (t instanceof AbortedOperationException)
-            return TIMEOUT;
+        for (Map.Entry<Class<? extends Throwable>, RequestFailureReason> entry : exceptionToReasonMap.entrySet())
+            if (entry.getKey().isInstance(t))
+                return entry.getValue();
 
         return UNKNOWN;
     }

src/java/org/apache/cassandra/index/IndexBuildInProgressException.java

@@ -0,0 +1,36 @@
+/*
+ * Copyright DataStax, Inc.
+ *
+ * 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
+ *
+ * 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;
+
+import org.apache.cassandra.exceptions.RequestFailureReason;
+import org.apache.cassandra.exceptions.UncheckedInternalRequestExecutionException;
+
+/**
+ * Thrown if a secondary index is not currently available because it is building.
+ */
+public final class IndexBuildInProgressException extends UncheckedInternalRequestExecutionException
+{
+    /**
+     * Creates a new <code>IndexIsBuildingException</code> for the specified index.
+     * @param index the index
+     */
+    public IndexBuildInProgressException(Index index)
+    {
+        super(RequestFailureReason.INDEX_BUILD_IN_PROGRESS,
+                String.format("The secondary index '%s' is not yet available as it is building", index.getIndexMetadata().name));
+    }
+}

src/java/org/apache/cassandra/index/SecondaryIndexManager.java

@@ -357,17 +357,30 @@ public synchronized Future<?> addIndex(IndexMetadata indexDef, boolean isNewCF)
 
     /**
      * Throws an {@link IndexNotAvailableException} if any of the indexes in the specified {@link Index.QueryPlan} is
-     * not queryable, as it's defined by {@link #isIndexQueryable(Index)}.
+     * not queryable, as it's defined by {@link #isIndexQueryable(Index)}. If the reason for the index to be not available
+     * is that it's building, it will throw an {@link IndexBuildInProgressException}.
      *
      * @param queryPlan a query plan
      * @throws IndexNotAvailableException if the query plan has any index that is not queryable
      */
     public void checkQueryability(Index.QueryPlan queryPlan)
     {
+        InetAddressAndPort endpoint = FBUtilities.getBroadcastAddressAndPort();
+
         for (Index index : queryPlan.getIndexes())
         {
+            String indexName = index.getIndexMetadata().name;
+            Index.Status indexStatus = getIndexStatus(endpoint, keyspace.getName(), indexName);
+
             if (!isIndexQueryable(index))
+            {
+                // In Astra index can be queryable during index build, thus we need to check both not queryable and building
+                // Plus isQueryable is always true for non-SAI index implementations
+                if (indexStatus == Index.Status.FULL_REBUILD_STARTED)
+                    throw new IndexBuildInProgressException(index);
+
                 throw new IndexNotAvailableException(index);
+            }
         }
     }
 
@@ -839,6 +852,8 @@ private synchronized void markIndexFailed(Index index, boolean isInitialBuild)
 
             if (!index.getSupportedLoadTypeOnFailure(isInitialBuild).supportsReads() && queryableIndexes.remove(indexName))
                 logger.info("Index [{}] became not-queryable because of failed build.", indexName);
+
+            makeIndexNonQueryable(index, Index.Status.BUILD_FAILED);
         }
     }
 
@@ -1810,13 +1825,18 @@ public static void shutdownAndWait(long timeout, TimeUnit units) throws Interrup
      */
     public static <E extends Endpoints<E>> E filterForQuery(E liveEndpoints, Keyspace keyspace, Index.QueryPlan indexQueryPlan, ConsistencyLevel level)
     {
+        Map<InetAddressAndPort, Index.Status> indexStatusMap = new HashMap<>();
+
         E queryableEndpoints = liveEndpoints.filter(replica -> {
 
             for (Index index : indexQueryPlan.getIndexes())
             {
                 Index.Status status = getIndexStatus(replica.endpoint(), keyspace.getName(), index.getIndexMetadata().name);
                 if (!index.isQueryable(status))
+                {
+                    indexStatusMap.put(replica.endpoint(), status);
                     return false;
+                }
             }
 
             return true;
@@ -1834,7 +1854,13 @@ public static <E extends Endpoints<E>> E filterForQuery(E liveEndpoints, Keyspac
             {
                 Map<InetAddressAndPort, RequestFailureReason> failureReasons = new HashMap<>();
                 liveEndpoints.without(queryableEndpoints.endpoints())
-                             .forEach(replica -> failureReasons.put(replica.endpoint(), RequestFailureReason.INDEX_NOT_AVAILABLE));
+                             .forEach(replica -> {
+                                 Index.Status status = indexStatusMap.get(replica.endpoint());
+                                 if (status == Index.Status.FULL_REBUILD_STARTED)
+                                     failureReasons.put(replica.endpoint(), RequestFailureReason.INDEX_BUILD_IN_PROGRESS);
+                                 else
+                                     failureReasons.put(replica.endpoint(), RequestFailureReason.INDEX_NOT_AVAILABLE);
+                             });
 
                 throw new ReadFailureException(level, filtered, required, false, failureReasons);
             }
@@ -1941,9 +1967,6 @@ public synchronized static void receivePeerIndexStatus(InetAddressAndPort endpoi
 
     private synchronized static void propagateLocalIndexStatus(String keyspace, String index, Index.Status status)
     {
-        if (!Gossiper.instance.isEnabled())
-            return;
-
         try
         {
             Map<String, Index.Status> states = peerIndexStatus.computeIfAbsent(FBUtilities.getBroadcastAddressAndPort(),
@@ -1955,6 +1978,11 @@ private synchronized static void propagateLocalIndexStatus(String keyspace, Stri
             else
                 states.put(keyspaceIndex, status);
 
+            // Make sure to check this after peerIndexStatus is populated but before the JSON is built: we don't need
+            // the latter if there's no Gossiper (as in CNDB), so we can avoid the related CPU and memory usage.
+            if (!Gossiper.instance.isEnabled())
+                return;
+
             String newStatus = JSONValue.toJSONString(states.entrySet().stream()
                                                             .collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().toString())));
             statusPropagationExecutor.submit(() -> {