Skip to content

Commit c289ebf

Browse files
authored
Disable skip metadata for CQL4 for broken cases (#458)
CQL4 has flaws that allows scenario when after schema change affected prepare statement is not invalidated on client side. As result, data that is read by driver does no match cached metadata and client will fail to deserialize or deserialization will go wrong. More info at: scylladb/scylladb#20860 This commit introduces `PREPARE_SKIP_CQL4_METADATA_RESOLVE_METHOD(advanced.prepared-statements.skip-cql4-metadata-resolve-method)` that controls how driver resolves skip metadata flag for CQL4 prepared statements, it can be `SMART`, `ENABLED`, `DISABLED`. Default is `SMART` - It makes driver disable skip metadata flag only for wildcard selects and selects that returns udts (including collections and maps) `ENABLED` - `skip metadata` flag is always enabled, no metadata is being sent. `DISABLED` - `skip metadata` flag is disabled, metdata is being sent with every bound statement RESULT frame.
1 parent ffa12c5 commit c289ebf

File tree

10 files changed

+447
-5
lines changed

10 files changed

+447
-5
lines changed

core/src/main/java/com/datastax/dse/driver/internal/core/cql/DseConversions.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import com.datastax.oss.driver.internal.core.ProtocolVersionRegistry;
4040
import com.datastax.oss.driver.internal.core.context.InternalDriverContext;
4141
import com.datastax.oss.driver.internal.core.cql.Conversions;
42+
import com.datastax.oss.driver.internal.core.cql.DefaultPreparedStatement;
4243
import com.datastax.oss.protocol.internal.Message;
4344
import com.datastax.oss.protocol.internal.request.Execute;
4445
import com.datastax.oss.protocol.internal.request.Query;
@@ -115,8 +116,15 @@ public static Message toContinuousPagingMessage(
115116
protocolVersion, DefaultProtocolFeature.UNSET_BOUND_VALUES)) {
116117
Conversions.ensureAllSet(boundStatement);
117118
}
118-
boolean skipMetadata =
119-
boundStatement.getPreparedStatement().getResultSetDefinitions().size() > 0;
119+
120+
boolean skipMetadata;
121+
if (boundStatement.getPreparedStatement() instanceof DefaultPreparedStatement) {
122+
skipMetadata =
123+
((DefaultPreparedStatement) boundStatement.getPreparedStatement()).isSkipMetadata();
124+
} else {
125+
skipMetadata = boundStatement.getPreparedStatement().getResultSetDefinitions().size() > 0;
126+
;
127+
}
120128
DseQueryOptions queryOptions =
121129
new DseQueryOptions(
122130
consistencyCode,
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package com.datastax.oss.driver.api.core;
2+
3+
import edu.umd.cs.findbugs.annotations.NonNull;
4+
5+
public enum CQL4SkipMetadataResolveMethod {
6+
// SMART (Default) - Disables the skip metadata flag only for wildcard selects (`SELECT * FROM`)
7+
// and queries
8+
// that return UDTs (including UDT collections and maps containing UDTs).
9+
SMART("smart"),
10+
// ENABLED – Enables the `skip metadata` flag, preventing metadata from being sent
11+
ENABLED("enabled"),
12+
// DISABLED - Disables the `skip metadata` flag, ensuring metadata is included in every RESULT
13+
// frame for bound statement execution.
14+
DISABLED("disabled");
15+
16+
private final String value;
17+
18+
CQL4SkipMetadataResolveMethod(String value) {
19+
this.value = value;
20+
}
21+
22+
@Override
23+
public String toString() {
24+
return value;
25+
}
26+
27+
// Case in-sensitive version of `valueOf`. To be used at all times instead of `valueOf`
28+
@NonNull
29+
public static CQL4SkipMetadataResolveMethod fromValue(@NonNull String value)
30+
throws IllegalArgumentException {
31+
switch (value.toLowerCase()) {
32+
case "smart":
33+
return SMART;
34+
case "enabled":
35+
return ENABLED;
36+
case "disabled":
37+
return DISABLED;
38+
default:
39+
throw new IllegalArgumentException("Unsupported value " + value);
40+
}
41+
}
42+
}

core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -689,6 +689,34 @@ public enum DefaultDriverOption implements DriverOption {
689689
* <p>Value-type: boolean
690690
*/
691691
PREPARE_ON_ALL_NODES("advanced.prepared-statements.prepare-on-all-nodes"),
692+
693+
/**
694+
* CQL 4.x has a known issue where prepared statement invalidation may be bypassed on the client
695+
* side. Reference: https://github.com/scylladb/scylladb/issues/20860
696+
*
697+
* <p>When this occurs, the client's metadata can become outdated, leading to various
698+
* deserialization errors.
699+
*
700+
* <p>To mitigate this, the driver can disable the `skip metadata` flag, ensuring the server
701+
* includes metadata with every bound statement RESULT query response.
702+
*
703+
* <p>This setting determines how the driver handles the `skip metadata` flag for CQL 4 prepared
704+
* statements: - **"smart"** (default) – Disables the flag only for wildcard selects (`SELECT *
705+
* FROM`) and queries that return UDTs (including UDT collections and maps containing UDTs). -
706+
* **"enabled"** – Enables the `skip metadata` flag, preventing metadata from being sent. -
707+
* **"disabled"** – Disables the `skip metadata` flag, ensuring metadata is included in every
708+
* RESULT frame.
709+
*
710+
* <p>Sending metadata reduces performance on both the driver and server while increasing traffic.
711+
* If you need to use UDTs or wildcard selects, you must either accept the performance impact or
712+
* ensure: 1. No schema alterations are performed on tables or UDTs in use. 2. After any schema
713+
* change, all relevant prepared statements are re-prepared.
714+
*
715+
* <p>Value-type: string
716+
*/
717+
PREPARE_SKIP_CQL4_METADATA_RESOLVE_METHOD(
718+
"advanced.prepared-statements.skip-cql4-metadata-resolve-method"),
719+
692720
/**
693721
* Whether the driver tries to prepare on new nodes at all.
694722
*

core/src/main/java/com/datastax/oss/driver/api/core/config/OptionsMap.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
*/
1818
package com.datastax.oss.driver.api.core.config;
1919

20+
import com.datastax.oss.driver.api.core.CQL4SkipMetadataResolveMethod;
2021
import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList;
2122
import edu.umd.cs.findbugs.annotations.NonNull;
2223
import edu.umd.cs.findbugs.annotations.Nullable;
@@ -258,6 +259,9 @@ protected static void fillWithDriverDefaults(OptionsMap map) {
258259
map.put(TypedDriverOption.REQUEST_TIMEOUT, requestTimeout);
259260
map.put(TypedDriverOption.REQUEST_CONSISTENCY, "LOCAL_ONE");
260261
map.put(TypedDriverOption.REQUEST_PAGE_SIZE, requestPageSize);
262+
map.put(
263+
TypedDriverOption.PREPARE_SKIP_CQL4_METADATA_RESOLVE_METHOD,
264+
CQL4SkipMetadataResolveMethod.SMART.toString());
261265
map.put(TypedDriverOption.REQUEST_SERIAL_CONSISTENCY, "SERIAL");
262266
map.put(TypedDriverOption.REQUEST_DEFAULT_IDEMPOTENCE, false);
263267
map.put(TypedDriverOption.GRAPH_TRAVERSAL_SOURCE, "g");

core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,10 @@ public String toString() {
583583
/** Whether `Session.prepare` calls should be sent to all nodes in the cluster. */
584584
public static final TypedDriverOption<Boolean> PREPARE_ON_ALL_NODES =
585585
new TypedDriverOption<>(DefaultDriverOption.PREPARE_ON_ALL_NODES, GenericType.BOOLEAN);
586+
/** Method to resolve skip metadata flag for CQL4. */
587+
public static final TypedDriverOption<String> PREPARE_SKIP_CQL4_METADATA_RESOLVE_METHOD =
588+
new TypedDriverOption<>(
589+
DefaultDriverOption.PREPARE_SKIP_CQL4_METADATA_RESOLVE_METHOD, GenericType.STRING);
586590
/** Whether the driver tries to prepare on new nodes at all. */
587591
public static final TypedDriverOption<Boolean> REPREPARE_ENABLED =
588592
new TypedDriverOption<>(DefaultDriverOption.REPREPARE_ENABLED, GenericType.BOOLEAN);

core/src/main/java/com/datastax/oss/driver/internal/core/cql/Conversions.java

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,15 @@ public static Message toMessage(
194194
protocolVersion, DefaultProtocolFeature.UNSET_BOUND_VALUES)) {
195195
ensureAllSet(boundStatement);
196196
}
197-
boolean skipMetadata =
198-
boundStatement.getPreparedStatement().getResultSetDefinitions().size() > 0;
197+
198+
boolean skipMetadata;
199+
if (boundStatement.getPreparedStatement() instanceof DefaultPreparedStatement) {
200+
skipMetadata =
201+
((DefaultPreparedStatement) boundStatement.getPreparedStatement()).isSkipMetadata();
202+
} else {
203+
skipMetadata = boundStatement.getPreparedStatement().getResultSetDefinitions().size() > 0;
204+
}
205+
199206
QueryOptions queryOptions =
200207
new QueryOptions(
201208
consistencyCode,
@@ -382,6 +389,17 @@ public static DefaultPreparedStatement toPreparedStatement(
382389

383390
Partitioner partitioner = PartitionerFactory.partitioner(variableDefinitions, context);
384391

392+
DriverExecutionProfile executionProfile = request.getExecutionProfileForBoundStatements();
393+
if (executionProfile == null) {
394+
if (request.getExecutionProfileNameForBoundStatements() != null
395+
&& !request.getExecutionProfileNameForBoundStatements().isEmpty()) {
396+
executionProfile =
397+
context.getConfig().getProfile(request.getExecutionProfileNameForBoundStatements());
398+
} else {
399+
executionProfile = context.getConfig().getDefaultProfile();
400+
}
401+
}
402+
385403
return new DefaultPreparedStatement(
386404
ByteBuffer.wrap(response.preparedQueryId).asReadOnlyBuffer(),
387405
request.getQuery(),
@@ -395,7 +413,7 @@ public static DefaultPreparedStatement toPreparedStatement(
395413
partitioner,
396414
NullAllowingImmutableMap.copyOf(request.getCustomPayload()),
397415
request.getExecutionProfileNameForBoundStatements(),
398-
request.getExecutionProfileForBoundStatements(),
416+
executionProfile,
399417
request.getRoutingKeyspaceForBoundStatements(),
400418
request.getRoutingKeyForBoundStatements(),
401419
request.getRoutingTokenForBoundStatements(),

core/src/main/java/com/datastax/oss/driver/internal/core/cql/DefaultPreparedStatement.java

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,29 +23,43 @@
2323
*/
2424
package com.datastax.oss.driver.internal.core.cql;
2525

26+
import com.datastax.oss.driver.api.core.CQL4SkipMetadataResolveMethod;
2627
import com.datastax.oss.driver.api.core.ConsistencyLevel;
2728
import com.datastax.oss.driver.api.core.CqlIdentifier;
2829
import com.datastax.oss.driver.api.core.ProtocolVersion;
30+
import com.datastax.oss.driver.api.core.config.DefaultDriverOption;
2931
import com.datastax.oss.driver.api.core.config.DriverExecutionProfile;
3032
import com.datastax.oss.driver.api.core.cql.BoundStatement;
3133
import com.datastax.oss.driver.api.core.cql.BoundStatementBuilder;
34+
import com.datastax.oss.driver.api.core.cql.ColumnDefinition;
3235
import com.datastax.oss.driver.api.core.cql.ColumnDefinitions;
3336
import com.datastax.oss.driver.api.core.cql.PreparedStatement;
3437
import com.datastax.oss.driver.api.core.cql.Statement;
3538
import com.datastax.oss.driver.api.core.metadata.token.Partitioner;
3639
import com.datastax.oss.driver.api.core.metadata.token.Token;
40+
import com.datastax.oss.driver.api.core.type.ContainerType;
41+
import com.datastax.oss.driver.api.core.type.DataType;
42+
import com.datastax.oss.driver.api.core.type.MapType;
43+
import com.datastax.oss.driver.api.core.type.TupleType;
44+
import com.datastax.oss.driver.api.core.type.UserDefinedType;
3745
import com.datastax.oss.driver.api.core.type.codec.registry.CodecRegistry;
3846
import com.datastax.oss.driver.internal.core.data.ValuesHelper;
3947
import com.datastax.oss.driver.internal.core.session.RepreparePayload;
48+
import com.datastax.oss.driver.shaded.guava.common.base.Splitter;
4049
import edu.umd.cs.findbugs.annotations.NonNull;
4150
import java.nio.ByteBuffer;
4251
import java.time.Duration;
4352
import java.util.List;
4453
import java.util.Map;
4554
import net.jcip.annotations.ThreadSafe;
55+
import org.slf4j.Logger;
56+
import org.slf4j.LoggerFactory;
4657

4758
@ThreadSafe
4859
public class DefaultPreparedStatement implements PreparedStatement {
60+
private static final Logger LOGGER = LoggerFactory.getLogger(DefaultPreparedStatement.class);
61+
private static final Splitter SPACE_SPLITTER = Splitter.onPattern("\\s+");
62+
private static final Splitter COMMA_SPLITTER = Splitter.onPattern(",");
4963

5064
private final ByteBuffer id;
5165
private final RepreparePayload repreparePayload;
@@ -69,6 +83,7 @@ public class DefaultPreparedStatement implements PreparedStatement {
6983
private final Duration timeoutForBoundStatements;
7084
private final Partitioner partitioner;
7185
private final boolean isLWT;
86+
private volatile boolean skipMetadata;
7287

7388
public DefaultPreparedStatement(
7489
ByteBuffer id,
@@ -122,6 +137,9 @@ public DefaultPreparedStatement(
122137
this.codecRegistry = codecRegistry;
123138
this.protocolVersion = protocolVersion;
124139
this.isLWT = isLWT;
140+
this.skipMetadata =
141+
resolveSkipMetadata(
142+
query, resultMetadataId, resultSetDefinitions, this.executionProfileForBoundStatements);
125143
}
126144

127145
@NonNull
@@ -147,6 +165,10 @@ public Partitioner getPartitioner() {
147165
return partitioner;
148166
}
149167

168+
public boolean isSkipMetadata() {
169+
return skipMetadata;
170+
}
171+
150172
@NonNull
151173
@Override
152174
public List<Integer> getPartitionKeyIndices() {
@@ -172,6 +194,13 @@ public boolean isLWT() {
172194
@Override
173195
public void setResultMetadata(
174196
@NonNull ByteBuffer newResultMetadataId, @NonNull ColumnDefinitions newResultSetDefinitions) {
197+
this.skipMetadata =
198+
resolveSkipMetadata(
199+
this.getQuery(),
200+
newResultMetadataId,
201+
newResultSetDefinitions,
202+
executionProfileForBoundStatements);
203+
175204
this.resultMetadata = new ResultMetadata(newResultMetadataId, newResultSetDefinitions);
176205
}
177206

@@ -242,4 +271,121 @@ private ResultMetadata(ByteBuffer resultMetadataId, ColumnDefinitions resultSetD
242271
this.resultSetDefinitions = resultSetDefinitions;
243272
}
244273
}
274+
275+
private static boolean resolveSkipMetadata(
276+
String query,
277+
ByteBuffer resultMetadataId,
278+
ColumnDefinitions resultSet,
279+
DriverExecutionProfile executionProfileForBoundStatements) {
280+
if (resultSet == null || resultSet.size() == 0) {
281+
// there is no reason to send this flag, there will be no rows in the response and,
282+
// consequently, no metadata.
283+
return false;
284+
}
285+
if (resultMetadataId != null && resultMetadataId.capacity() > 0) {
286+
// Result metadata ID feature is supported, it makes prepared statement invalidation work
287+
// properly.
288+
// Skip Metadata should be enabled.
289+
// Prepared statement invalidation works perfectly no need to disable skip metadata
290+
return true;
291+
}
292+
293+
CQL4SkipMetadataResolveMethod resolveMethod = CQL4SkipMetadataResolveMethod.SMART;
294+
295+
if (executionProfileForBoundStatements != null) {
296+
String resolveMethodName =
297+
executionProfileForBoundStatements.getString(
298+
DefaultDriverOption.PREPARE_SKIP_CQL4_METADATA_RESOLVE_METHOD);
299+
try {
300+
resolveMethod = CQL4SkipMetadataResolveMethod.fromValue(resolveMethodName);
301+
} catch (IllegalArgumentException e) {
302+
LOGGER.warn(
303+
"Property advanced.prepared-statements.skip-cql4-metadata-resolve-method is incorrectly set to `{}`, "
304+
+ "available options: smart, enabled, disabled. Defaulting to `SMART`",
305+
resolveMethodName);
306+
resolveMethod = CQL4SkipMetadataResolveMethod.SMART;
307+
}
308+
}
309+
310+
switch (resolveMethod) {
311+
case ENABLED:
312+
return true;
313+
case DISABLED:
314+
return false;
315+
case SMART:
316+
break;
317+
}
318+
319+
if (isWildcardSelect(query)) {
320+
LOGGER.warn(
321+
"Prepared statement {} is a wildcard select, which can cause prepared statement invalidation issues when executed on CQL4. "
322+
+ "These issues may lead to broken deserialization or data corruption. "
323+
+ "To mitigate this, the driver ensures that the server returns metadata with each query for such statements, "
324+
+ "though this negatively impacts performance. "
325+
+ "To avoid this, consider using a targeted select instead. "
326+
+ "Find more mitigation options in description of `advanced.prepared-statements.skip-cql4-metadata-resolve-method` flag",
327+
query);
328+
return false;
329+
}
330+
// Disable skipping metadata if results contains udt and
331+
for (ColumnDefinition columnDefinition : resultSet) {
332+
if (containsUDT(columnDefinition.getType())) {
333+
LOGGER.warn(
334+
"Prepared statement {} contains UDT in result, which can cause prepared statement invalidation issues when executed on CQL4. "
335+
+ "These issues may lead to broken deserialization or data corruption. "
336+
+ "To mitigate this, the driver ensures that the server returns metadata with each query for such statements, "
337+
+ "though this negatively impacts performance. "
338+
+ "To avoid this, consider using regular columns instead of UDT. "
339+
+ "Find more mitigation options in description of `advanced.prepared-statements.skip-cql4-metadata-resolve-method` flag",
340+
query);
341+
return false;
342+
}
343+
}
344+
return true;
345+
}
346+
347+
private static boolean containsUDT(DataType dataType) {
348+
if (dataType instanceof ContainerType) {
349+
return containsUDT(((ContainerType) dataType).getElementType());
350+
} else if (dataType instanceof TupleType) {
351+
for (DataType elementType : ((TupleType) dataType).getComponentTypes()) {
352+
if (containsUDT(elementType)) {
353+
return true;
354+
}
355+
}
356+
return false;
357+
} else if (dataType instanceof MapType) {
358+
return containsUDT(((MapType) dataType).getKeyType())
359+
|| containsUDT(((MapType) dataType).getValueType());
360+
}
361+
return dataType instanceof UserDefinedType;
362+
}
363+
364+
private static boolean isWildcardSelect(String query) {
365+
List<String> chunks = SPACE_SPLITTER.splitToList(query.trim().toLowerCase());
366+
if (chunks.size() < 2) {
367+
// Weird query, assuming no result expected
368+
return false;
369+
}
370+
371+
if (!chunks.get(0).equals("select")) {
372+
// In case if non-select sneaks in, disable skip metadata for it no result expected.
373+
return false;
374+
}
375+
376+
for (String chunk : chunks) {
377+
if (chunk.equals("from")) {
378+
return false;
379+
}
380+
if (chunk.equals("*")) {
381+
return true;
382+
}
383+
for (String part : COMMA_SPLITTER.split(chunk)) {
384+
if (part.equals("*")) {
385+
return true;
386+
}
387+
}
388+
}
389+
return false;
390+
}
245391
}

0 commit comments

Comments
 (0)