Skip to content

Commit 3ef9d5e

Browse files
committed
Disable skip metadata for CQL4 for broken cases
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", "always-on", "always-off". Default is "smart". It makes driver disable skip metadata flag only for wildcard selects and selects that returns udts (including collections and maps)
1 parent ce575f3 commit 3ef9d5e

File tree

10 files changed

+440
-5
lines changed

10 files changed

+440
-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: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,29 +23,41 @@
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);
4961

5062
private final ByteBuffer id;
5163
private final RepreparePayload repreparePayload;
@@ -69,6 +81,7 @@ public class DefaultPreparedStatement implements PreparedStatement {
6981
private final Duration timeoutForBoundStatements;
7082
private final Partitioner partitioner;
7183
private final boolean isLWT;
84+
private volatile boolean skipMetadata;
7285

7386
public DefaultPreparedStatement(
7487
ByteBuffer id,
@@ -122,6 +135,9 @@ public DefaultPreparedStatement(
122135
this.codecRegistry = codecRegistry;
123136
this.protocolVersion = protocolVersion;
124137
this.isLWT = isLWT;
138+
this.skipMetadata =
139+
resolveSkipMetadata(
140+
query, resultMetadataId, resultSetDefinitions, this.executionProfileForBoundStatements);
125141
}
126142

127143
@NonNull
@@ -147,6 +163,10 @@ public Partitioner getPartitioner() {
147163
return partitioner;
148164
}
149165

166+
public boolean isSkipMetadata() {
167+
return skipMetadata;
168+
}
169+
150170
@NonNull
151171
@Override
152172
public List<Integer> getPartitionKeyIndices() {
@@ -172,6 +192,13 @@ public boolean isLWT() {
172192
@Override
173193
public void setResultMetadata(
174194
@NonNull ByteBuffer newResultMetadataId, @NonNull ColumnDefinitions newResultSetDefinitions) {
195+
this.skipMetadata =
196+
resolveSkipMetadata(
197+
this.getQuery(),
198+
newResultMetadataId,
199+
newResultSetDefinitions,
200+
executionProfileForBoundStatements);
201+
175202
this.resultMetadata = new ResultMetadata(newResultMetadataId, newResultSetDefinitions);
176203
}
177204

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

0 commit comments

Comments
 (0)