Skip to content

Commit e85b0c9

Browse files
adelapenaadelapena
authored
CNDB-12041: Add missing check of guardrail sai_sstable_indexes_per_query (#1501)
1 parent de16993 commit e85b0c9

File tree

5 files changed

+58
-26
lines changed

5 files changed

+58
-26
lines changed

‎src/java/org/apache/cassandra/index/sai/plan/FilterTree.java

+21
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,18 @@
1919

2020
import java.nio.ByteBuffer;
2121
import java.util.ArrayList;
22+
import java.util.HashSet;
2223
import java.util.Iterator;
2324
import java.util.List;
2425
import java.util.ListIterator;
26+
import java.util.Set;
2527

2628
import com.google.common.collect.ListMultimap;
2729

2830
import org.apache.cassandra.db.DecoratedKey;
2931
import org.apache.cassandra.db.rows.Row;
3032
import org.apache.cassandra.db.rows.Unfiltered;
33+
import org.apache.cassandra.index.sai.SSTableIndex;
3134
import org.apache.cassandra.index.sai.utils.TypeUtil;
3235
import org.apache.cassandra.schema.ColumnMetadata;
3336
import org.apache.cassandra.schema.ColumnMetadata.Kind;
@@ -130,4 +133,22 @@ private boolean localSatisfiedBy(DecoratedKey key, Unfiltered currentCluster, Ro
130133
private boolean shouldReturnNow(boolean result) {
131134
return (op == OperationType.AND && !result) || (op == OperationType.OR && result);
132135
}
136+
137+
/**
138+
* @return the number of unique SSTable indexes that are referenced by the expressions in this filter tree.
139+
*/
140+
public int numSSTableIndexes()
141+
{
142+
Set<SSTableIndex> referencedIndexes = new HashSet<>();
143+
sstableIndexes(referencedIndexes);
144+
return referencedIndexes.size();
145+
}
146+
147+
private void sstableIndexes(Set<SSTableIndex> indexes)
148+
{
149+
for (Expression expression : expressions.values())
150+
indexes.addAll(expression.context.getView().getIndexes());
151+
for (FilterTree child : children)
152+
child.sstableIndexes(indexes);
153+
}
133154
}

‎src/java/org/apache/cassandra/index/sai/plan/QueryController.java

-22
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import org.apache.cassandra.db.ColumnFamilyStore;
4444
import org.apache.cassandra.db.DataRange;
4545
import org.apache.cassandra.db.DecoratedKey;
46-
import org.apache.cassandra.db.MessageParams;
4746
import org.apache.cassandra.db.MultiRangeReadCommand;
4847
import org.apache.cassandra.db.PartitionPosition;
4948
import org.apache.cassandra.db.PartitionRangeReadCommand;
@@ -55,7 +54,6 @@
5554
import org.apache.cassandra.db.filter.ClusteringIndexSliceFilter;
5655
import org.apache.cassandra.db.filter.DataLimits;
5756
import org.apache.cassandra.db.filter.RowFilter;
58-
import org.apache.cassandra.db.guardrails.Guardrails;
5957
import org.apache.cassandra.db.lifecycle.SSTableSet;
6058
import org.apache.cassandra.db.marshal.AbstractType;
6159
import org.apache.cassandra.db.marshal.CollectionType;
@@ -87,7 +85,6 @@
8785
import org.apache.cassandra.index.sai.view.View;
8886
import org.apache.cassandra.io.sstable.format.SSTableReader;
8987
import org.apache.cassandra.io.util.FileUtils;
90-
import org.apache.cassandra.net.ParamType;
9188
import org.apache.cassandra.schema.TableMetadata;
9289
import org.apache.cassandra.tracing.Tracing;
9390
import org.apache.cassandra.utils.CloseableIterator;
@@ -822,25 +819,6 @@ private void closeQueryViews()
822819
}
823820
}
824821

825-
void maybeTriggerReferencedIndexesGuardrail(int numReferencedIndexes)
826-
{
827-
if (Guardrails.saiSSTableIndexesPerQuery.failsOn(numReferencedIndexes, null))
828-
{
829-
String msg = String.format("Query %s attempted to read from too many indexes (%s) but max allowed is %s; " +
830-
"query aborted (see sai_sstable_indexes_per_query_fail_threshold)",
831-
command.toCQLString(),
832-
numReferencedIndexes,
833-
Guardrails.CONFIG_PROVIDER.getOrCreate(null).getSaiSSTableIndexesPerQueryFailThreshold());
834-
Tracing.trace(msg);
835-
MessageParams.add(ParamType.TOO_MANY_REFERENCED_INDEXES_FAIL, numReferencedIndexes);
836-
throw new QueryReferencingTooManyIndexesException(msg);
837-
}
838-
else if (Guardrails.saiSSTableIndexesPerQuery.warnsOn(numReferencedIndexes, null))
839-
{
840-
MessageParams.add(ParamType.TOO_MANY_REFERENCED_INDEXES_WARN, numReferencedIndexes);
841-
}
842-
}
843-
844822
/**
845823
* Returns the {@link DataRange} list covered by the specified {@link ReadCommand}.
846824
*

‎src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexSearcher.java

+30
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,11 @@
4242
import org.apache.cassandra.db.ColumnFamilyStore;
4343
import org.apache.cassandra.db.DataRange;
4444
import org.apache.cassandra.db.DecoratedKey;
45+
import org.apache.cassandra.db.MessageParams;
4546
import org.apache.cassandra.db.PartitionPosition;
4647
import org.apache.cassandra.db.ReadCommand;
4748
import org.apache.cassandra.db.ReadExecutionController;
49+
import org.apache.cassandra.db.guardrails.Guardrails;
4850
import org.apache.cassandra.db.partitions.UnfilteredPartitionIterator;
4951
import org.apache.cassandra.db.rows.AbstractUnfilteredRowIterator;
5052
import org.apache.cassandra.db.rows.Row;
@@ -63,7 +65,9 @@
6365
import org.apache.cassandra.index.sai.utils.RangeUtil;
6466
import org.apache.cassandra.index.sai.utils.PrimaryKeyWithSortKey;
6567
import org.apache.cassandra.io.util.FileUtils;
68+
import org.apache.cassandra.net.ParamType;
6669
import org.apache.cassandra.schema.TableMetadata;
70+
import org.apache.cassandra.tracing.Tracing;
6771
import org.apache.cassandra.utils.AbstractIterator;
6872
import org.apache.cassandra.utils.Clock;
6973
import org.apache.cassandra.utils.CloseableIterator;
@@ -109,6 +113,8 @@ public UnfilteredPartitionIterator search(ReadExecutionController executionContr
109113
try
110114
{
111115
FilterTree filterTree = analyzeFilter();
116+
maybeTriggerReferencedIndexesGuardrail(filterTree);
117+
112118
Plan plan = controller.buildPlan();
113119
Iterator<? extends PrimaryKey> keysIterator = controller.buildIterator(plan);
114120

@@ -154,6 +160,30 @@ public UnfilteredPartitionIterator search(ReadExecutionController executionContr
154160
}
155161
}
156162

163+
private void maybeTriggerReferencedIndexesGuardrail(FilterTree filterTree)
164+
{
165+
if (!Guardrails.saiSSTableIndexesPerQuery.enabled())
166+
return;
167+
168+
int numReferencedIndexes = filterTree.numSSTableIndexes();
169+
170+
if (Guardrails.saiSSTableIndexesPerQuery.failsOn(numReferencedIndexes, null))
171+
{
172+
String msg = String.format("Query %s attempted to read from too many indexes (%s) but max allowed is %s; " +
173+
"query aborted (see sai_sstable_indexes_per_query_fail_threshold)",
174+
command.toCQLString(),
175+
numReferencedIndexes,
176+
Guardrails.CONFIG_PROVIDER.getOrCreate(null).getSaiSSTableIndexesPerQueryFailThreshold());
177+
Tracing.trace(msg);
178+
MessageParams.add(ParamType.TOO_MANY_REFERENCED_INDEXES_FAIL, numReferencedIndexes);
179+
throw new QueryReferencingTooManyIndexesException(msg);
180+
}
181+
else if (Guardrails.saiSSTableIndexesPerQuery.warnsOn(numReferencedIndexes, null))
182+
{
183+
MessageParams.add(ParamType.TOO_MANY_REFERENCED_INDEXES_WARN, numReferencedIndexes);
184+
}
185+
}
186+
157187
/**
158188
* Converts expressions into filter tree (which is currently just a single AND).
159189
* </p>

‎src/java/org/apache/cassandra/service/reads/thresholds/WarningsSnapshot.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,10 @@ public int hashCode()
194194
@Override
195195
public String toString()
196196
{
197-
return "(tombstones=" + tombstones + ", localReadSize=" + localReadSize + ", rowIndexTooLarge=" + rowIndexReadSize + ')';
197+
return "(tombstones=" + tombstones +
198+
", localReadSize=" + localReadSize +
199+
", rowIndexTooLarge=" + rowIndexReadSize +
200+
", indexReadSSTablesCount=" + indexReadSSTablesCount + ')';
198201
}
199202

200203
public static final class Warnings

‎test/distributed/org/apache/cassandra/distributed/test/guardrails/GuardrailNonPartitionRestrictedQueryTest.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -144,11 +144,11 @@ public void testSAIWarnThreshold()
144144

145145
assertWarnAborts(0, 0);
146146

147-
// create 3 more SSTables on each node, this will trigger warn threshold (3 > 2 but < 5)
147+
// create 3 more SSTables on each node, this will trigger warn threshold (4 > 2 but < 5)
148148
valueToQuery = createSSTables(3);
149149
String valueToQueryString = LongType.instance.toCQLString(LongType.instance.decompose(valueToQuery), true);
150150
String expectedMessage = tooManyIndexesReadWarnMessage(cluster.size(),
151-
3,
151+
4,
152152
String.format("SELECT * FROM %s.%s WHERE v1 = %s ALLOW FILTERING",
153153
KEYSPACE, tableName, valueToQueryString));
154154
assertThat(getOnlyElement(executeSelect(valueToQuery, false))).contains(expectedMessage);
@@ -168,7 +168,7 @@ public void testSAIWarnThreshold()
168168

169169
// notice we expect warnings from 2 nodes
170170
expectedMessage = tooManyIndexesReadWarnMessage(cluster.size() - 1,
171-
3,
171+
4,
172172
String.format("SELECT * FROM %s.%s WHERE v1 = %s ALLOW FILTERING",
173173
KEYSPACE, tableName, valueToQueryString));
174174

0 commit comments

Comments
 (0)