From 740d6d080504da1a072ab98d3b0793031b87ca98 Mon Sep 17 00:00:00 2001 From: Caleb Rackliffe Date: Tue, 11 Mar 2025 17:07:28 -0500 Subject: [PATCH] Prioritize legacy 2i over SAI for columns with multiple indexes patch by Caleb Rackliffe; reviewed by David Capwell for CASSANDRA-20334 --- CHANGES.txt | 1 + NEWS.txt | 10 +++ conf/cassandra.yaml | 5 ++ .../cassandra/config/DatabaseDescriptor.java | 10 +++ .../config/StorageAttachedIndexOptions.java | 2 + .../plan/StorageAttachedIndexQueryPlan.java | 5 +- .../cassandra/service/StorageService.java | 11 +++ .../service/StorageServiceMBean.java | 3 + .../sai/metrics/AbstractMetricsTest.java | 5 ++ .../index/sai/metrics/IndexSelectionTest.java | 72 +++++++++++++++++++ .../index/sai/metrics/QueryMetricsTest.java | 5 -- 11 files changed, 120 insertions(+), 9 deletions(-) create mode 100644 test/unit/org/apache/cassandra/index/sai/metrics/IndexSelectionTest.java diff --git a/CHANGES.txt b/CHANGES.txt index fa2ab2e885..8b02c3ffae 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.0.4 + * Prioritize legacy 2i over SAI for columns with multiple indexes (CASSANDRA-20334) * Ensure only offline tools can build IntervalTrees without first/last key fields (CASSANDRA-20407) * Improve IntervalTree build throughput (CASSANDRA-19596) * Avoid limit on RFP fetch in the case of an unresolved static row (CASSANDRA-20323) diff --git a/NEWS.txt b/NEWS.txt index 717b5cf37d..5b5ec9b92a 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -65,6 +65,16 @@ restore snapshots created with the previous major version using the 'sstableloader' tool. You can upgrade the file format of your snapshots using the provided 'sstableupgrade' tool. +5.0.4 +===== + +Upgrading +--------- + - When more than one index exists on a single column, legacy table-backed indexes will now be prioritized over + storage-attached indexes (SAI) to make migration between the two safer. This behavior can be switched off via + the flag `sai_options.prioritize_over_legacy_index` (which defaults to `false`) in `cassandra.yaml` or via + `setPrioritizeSAIOverLegacyIndex(boolean)` in the JMX MBean `org.apache.cassandra.db:type=StorageService`. + 5.0.1 ===== diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index 6f70cd669c..e09518188a 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -1763,6 +1763,11 @@ transparent_data_encryption_options: ## is split between all SAI indexes being built so more indexes will mean smaller ## segment sizes. # segment_write_buffer_size: 1024MiB + # + ## When more than one index exists on a single column, this flag determines whether or not SAI + ## should take precedence over legacy table-backed indexes during reads. By default, legacy + ## indexes take precedence to maintain continuity during migration. + # prioritize_over_legacy_index: false ##################### # SAFETY THRESHOLDS # diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index bc58fe151b..9797bf82e6 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -5247,6 +5247,16 @@ public class DatabaseDescriptor return conf.sai_options.segment_write_buffer_size; } + public static boolean getPrioritizeSAIOverLegacyIndex() + { + return conf.sai_options.prioritize_over_legacy_index; + } + + public static void setPrioritizeSAIOverLegacyIndex(boolean value) + { + conf.sai_options.prioritize_over_legacy_index = value; + } + public static RepairRetrySpec getRepairRetrySpec() { return conf == null ? new RepairRetrySpec() : conf.repair.retries; diff --git a/src/java/org/apache/cassandra/config/StorageAttachedIndexOptions.java b/src/java/org/apache/cassandra/config/StorageAttachedIndexOptions.java index 18bf3506fe..48aea80f37 100644 --- a/src/java/org/apache/cassandra/config/StorageAttachedIndexOptions.java +++ b/src/java/org/apache/cassandra/config/StorageAttachedIndexOptions.java @@ -33,6 +33,8 @@ public class StorageAttachedIndexOptions "Value must be a positive integer less than " + MAXIMUM_SEGMENT_BUFFER_MB + "MiB"; public DataStorageSpec.IntMebibytesBound segment_write_buffer_size = new DataStorageSpec.IntMebibytesBound(DEFAULT_SEGMENT_BUFFER_MB); + + public volatile boolean prioritize_over_legacy_index = false; public void validate() { diff --git a/src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexQueryPlan.java b/src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexQueryPlan.java index e352f7fcdd..b53632407f 100644 --- a/src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexQueryPlan.java +++ b/src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexQueryPlan.java @@ -121,10 +121,7 @@ public class StorageAttachedIndexQueryPlan implements Index.QueryPlan @Override public long getEstimatedResultRows() { - // this is temporary (until proper QueryPlan is integrated into Cassandra) - // and allows us to priority storage-attached indexes if any in the query since they - // are going to be more efficient, to query and intersect, than built-in indexes. - return Long.MIN_VALUE; + return DatabaseDescriptor.getPrioritizeSAIOverLegacyIndex() ? Long.MIN_VALUE : Long.MAX_VALUE; } @Override diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index 533ca08e6d..9f2c818619 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -7657,4 +7657,15 @@ public class StorageService extends NotificationBroadcasterSupport implements IE DatabaseDescriptor.setEnforceNativeDeadlineForHints(value); } + @Override + public boolean getPrioritizeSAIOverLegacyIndex() + { + return DatabaseDescriptor.getPrioritizeSAIOverLegacyIndex(); + } + + @Override + public void setPrioritizeSAIOverLegacyIndex(boolean value) + { + DatabaseDescriptor.setPrioritizeSAIOverLegacyIndex(value); + } } diff --git a/src/java/org/apache/cassandra/service/StorageServiceMBean.java b/src/java/org/apache/cassandra/service/StorageServiceMBean.java index 0bfcfd500b..66396e03f6 100644 --- a/src/java/org/apache/cassandra/service/StorageServiceMBean.java +++ b/src/java/org/apache/cassandra/service/StorageServiceMBean.java @@ -1319,4 +1319,7 @@ public interface StorageServiceMBean extends NotificationEmitter * e.g. keyspace_name -> [reads, writes, paxos]. */ Map getOutOfRangeOperationCounts(); + + boolean getPrioritizeSAIOverLegacyIndex(); + void setPrioritizeSAIOverLegacyIndex(boolean value); } diff --git a/test/unit/org/apache/cassandra/index/sai/metrics/AbstractMetricsTest.java b/test/unit/org/apache/cassandra/index/sai/metrics/AbstractMetricsTest.java index 24134bccde..e09e44f326 100644 --- a/test/unit/org/apache/cassandra/index/sai/metrics/AbstractMetricsTest.java +++ b/test/unit/org/apache/cassandra/index/sai/metrics/AbstractMetricsTest.java @@ -34,4 +34,9 @@ public abstract class AbstractMetricsTest extends SAITester createMBeanServerConnection(); } + + protected long getTableQueryMetrics(String keyspace, String table, String metricsName) + { + return (long) getMetricValue(objectNameNoIndex(metricsName, keyspace, table, TableQueryMetrics.TABLE_QUERY_METRIC_TYPE)); + } } diff --git a/test/unit/org/apache/cassandra/index/sai/metrics/IndexSelectionTest.java b/test/unit/org/apache/cassandra/index/sai/metrics/IndexSelectionTest.java new file mode 100644 index 0000000000..db44c21d52 --- /dev/null +++ b/test/unit/org/apache/cassandra/index/sai/metrics/IndexSelectionTest.java @@ -0,0 +1,72 @@ +/* + * 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.metrics; + +import org.junit.Test; + +import org.apache.cassandra.config.DatabaseDescriptor; + +import static org.junit.Assert.assertEquals; + +public class IndexSelectionTest extends AbstractMetricsTest +{ + @Test + public void shouldSelectLegacyIndexByDefault() + { + createTable("CREATE TABLE %s (pk int, ck int, val1 int, val2 int, PRIMARY KEY(pk, ck))"); + createIndex("CREATE INDEX ON %s(val1) USING 'legacy_local_table'"); + createIndex("CREATE INDEX ON %s(val1) USING 'sai'"); + + execute("INSERT INTO %s(pk, ck, val1, val2) VALUES(?, ?, ?, ?)", 1, 1, 2, 1); + + assertRows(execute("SELECT pk, ck, val1, val2 FROM %s WHERE val1 = 2"), row(1, 1, 2, 1)); + assertEquals(0L, getTableQueryMetrics(KEYSPACE, currentTable(), "TotalQueriesCompleted")); + + DatabaseDescriptor.setPrioritizeSAIOverLegacyIndex(true); + assertRows(execute("SELECT pk, ck, val1, val2 FROM %s WHERE val1 = 2"), row(1, 1, 2, 1)); + assertEquals(1L, getTableQueryMetrics(KEYSPACE, currentTable(), "TotalQueriesCompleted")); + + DatabaseDescriptor.setPrioritizeSAIOverLegacyIndex(false); + assertRows(execute("SELECT pk, ck, val1, val2 FROM %s WHERE val1 = 2"), row(1, 1, 2, 1)); + assertEquals(1L, getTableQueryMetrics(KEYSPACE, currentTable(), "TotalQueriesCompleted")); + } + + @Test + public void shouldSelectLegacyIndexByDefaultForAndQueries() + { + createTable("CREATE TABLE %s (pk int, ck int, val1 int, val2 int, PRIMARY KEY(pk, ck))"); + createIndex("CREATE INDEX ON %s(val1) USING 'legacy_local_table'"); + createIndex("CREATE INDEX ON %s(val1) USING 'sai'"); + createIndex("CREATE INDEX ON %s(val2) USING 'legacy_local_table'"); + createIndex("CREATE INDEX ON %s(val2) USING 'sai'"); + + execute("INSERT INTO %s(pk, ck, val1, val2) VALUES(?, ?, ?, ?)", 1, 1, 2, 1); + + assertRows(execute("SELECT pk, ck, val1, val2 FROM %s WHERE val1 = 2 AND val2 = 1"), row(1, 1, 2, 1)); + assertEquals(0L, getTableQueryMetrics(KEYSPACE, currentTable(), "TotalQueriesCompleted")); + + DatabaseDescriptor.setPrioritizeSAIOverLegacyIndex(true); + assertRows(execute("SELECT pk, ck, val1, val2 FROM %s WHERE val1 = 2 AND val2 = 1"), row(1, 1, 2, 1)); + assertEquals(1L, getTableQueryMetrics(KEYSPACE, currentTable(), "TotalQueriesCompleted")); + + DatabaseDescriptor.setPrioritizeSAIOverLegacyIndex(false); + assertRows(execute("SELECT pk, ck, val1, val2 FROM %s WHERE val1 = 2 AND val2 = 1"), row(1, 1, 2, 1)); + assertEquals(1L, getTableQueryMetrics(KEYSPACE, currentTable(), "TotalQueriesCompleted")); + } +} 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 846024ebe0..f6b808e1e0 100644 --- a/test/unit/org/apache/cassandra/index/sai/metrics/QueryMetricsTest.java +++ b/test/unit/org/apache/cassandra/index/sai/metrics/QueryMetricsTest.java @@ -99,9 +99,4 @@ public class QueryMetricsTest extends AbstractMetricsTest dropIndex(String.format("DROP INDEX %s." + index, keyspace)); assertThatThrownBy(() -> getTableQueryMetrics(keyspace, table, "TotalQueriesCompleted")).hasCauseInstanceOf(InstanceNotFoundException.class); } - - private long getTableQueryMetrics(String keyspace, String table, String metricsName) - { - return (long) getMetricValue(objectNameNoIndex(metricsName, keyspace, table, TableQueryMetrics.TABLE_QUERY_METRIC_TYPE)); - } }