Skip to content

Commit e3ef32f

Browse files
committed
Intruduce flag to disable skip metadata
CQL 4.x 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 REQUEST_DISABLE_SKIP_METADATA(basic.request.disable-skip-metadata) which is set to true by default. Which will make server reply every time with metadata in response.
1 parent ce575f3 commit e3ef32f

File tree

7 files changed

+103
-2
lines changed

7 files changed

+103
-2
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,10 @@ public static Message toContinuousPagingMessage(
116116
Conversions.ensureAllSet(boundStatement);
117117
}
118118
boolean skipMetadata =
119-
boundStatement.getPreparedStatement().getResultSetDefinitions().size() > 0;
119+
Conversions.resolveSkipMetadata(
120+
boundStatement,
121+
context,
122+
boundStatement.getPreparedStatement().getResultSetDefinitions().size() > 0);
120123
DseQueryOptions queryOptions =
121124
new DseQueryOptions(
122125
consistencyCode,

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,12 @@ public enum DefaultDriverOption implements DriverOption {
6868
* <p>Value-Type: int
6969
*/
7070
REQUEST_PAGE_SIZE("basic.request.page-size"),
71+
/**
72+
* The page size.
73+
*
74+
* <p>Value-Type: int
75+
*/
76+
REQUEST_DISABLE_SKIP_METADATA("basic.request.disable-skip-metadata"),
7177
/**
7278
* The serial consistency level.
7379
*

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ protected static void fillWithDriverDefaults(OptionsMap map) {
258258
map.put(TypedDriverOption.REQUEST_TIMEOUT, requestTimeout);
259259
map.put(TypedDriverOption.REQUEST_CONSISTENCY, "LOCAL_ONE");
260260
map.put(TypedDriverOption.REQUEST_PAGE_SIZE, requestPageSize);
261+
map.put(TypedDriverOption.REQUEST_DISABLE_SKIP_METADATA, true);
261262
map.put(TypedDriverOption.REQUEST_SERIAL_CONSISTENCY, "SERIAL");
262263
map.put(TypedDriverOption.REQUEST_DEFAULT_IDEMPOTENCE, false);
263264
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
@@ -126,6 +126,10 @@ public String toString() {
126126
/** The page size. */
127127
public static final TypedDriverOption<Integer> REQUEST_PAGE_SIZE =
128128
new TypedDriverOption<>(DefaultDriverOption.REQUEST_PAGE_SIZE, GenericType.INTEGER);
129+
/** The disable skip metadata. */
130+
public static final TypedDriverOption<Boolean> REQUEST_DISABLE_SKIP_METADATA =
131+
new TypedDriverOption<>(
132+
DefaultDriverOption.REQUEST_DISABLE_SKIP_METADATA, GenericType.BOOLEAN);
129133
/** The serial consistency level. */
130134
public static final TypedDriverOption<String> REQUEST_SERIAL_CONSISTENCY =
131135
new TypedDriverOption<>(DefaultDriverOption.REQUEST_SERIAL_CONSISTENCY, GenericType.STRING);

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,12 @@ public static Message toMessage(
194194
protocolVersion, DefaultProtocolFeature.UNSET_BOUND_VALUES)) {
195195
ensureAllSet(boundStatement);
196196
}
197+
197198
boolean skipMetadata =
198-
boundStatement.getPreparedStatement().getResultSetDefinitions().size() > 0;
199+
resolveSkipMetadata(
200+
boundStatement,
201+
context,
202+
boundStatement.getPreparedStatement().getResultSetDefinitions().size() > 0);
199203
QueryOptions queryOptions =
200204
new QueryOptions(
201205
consistencyCode,
@@ -556,6 +560,16 @@ public static boolean resolveIdempotence(Request request, InternalDriverContext
556560
: requestIsIdempotent;
557561
}
558562

563+
public static boolean resolveSkipMetadata(
564+
Request request, InternalDriverContext context, boolean responseHasMetadata) {
565+
if (!responseHasMetadata) {
566+
// If no reason to send this flag, there will be no metadata anyway.
567+
return false;
568+
}
569+
DriverExecutionProfile executionProfile = resolveExecutionProfile(request, context);
570+
return !executionProfile.getBoolean(DefaultDriverOption.REQUEST_DISABLE_SKIP_METADATA);
571+
}
572+
559573
public static Duration resolveRequestTimeout(Request request, InternalDriverContext context) {
560574
DriverExecutionProfile executionProfile = resolveExecutionProfile(request, context);
561575
return request.getTimeout() != null

core/src/main/resources/reference.conf

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,18 @@ datastax-java-driver {
134134
# Overridable in a profile: yes
135135
page-size = 5000
136136

137+
# Disable skipping metadata for every query
138+
# There is known problem in CQL 4.x when prepared statement invalidation could be voided:
139+
# https://github.com/scylladb/scylladb/issues/20860
140+
# When it happens metadata on client side does not match data and deserialization can go wrong in many ways
141+
# To avoid that turn this option on, it will make server respond with metadata on every query.
142+
# Unfortunately it comes with prise of having more traffic and more deserialization CPU footprint on client side.
143+
#
144+
# Required: yes
145+
# Modifiable at runtime: yes, the new value will be used for requests issued after the change.
146+
# Overridable in a profile: yes
147+
disable-skip-metadata = true
148+
137149
# The serial consistency level.
138150
# The allowed values are SERIAL and LOCAL_SERIAL.
139151
#

integration-tests/src/test/java/com/datastax/oss/driver/core/cql/BoundStatementCcmIT.java

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import com.datastax.oss.driver.api.core.config.DriverExecutionProfile;
3737
import com.datastax.oss.driver.api.core.cql.AsyncResultSet;
3838
import com.datastax.oss.driver.api.core.cql.BoundStatement;
39+
import com.datastax.oss.driver.api.core.cql.ColumnDefinition;
3940
import com.datastax.oss.driver.api.core.cql.ColumnDefinitions;
4041
import com.datastax.oss.driver.api.core.cql.PreparedStatement;
4142
import com.datastax.oss.driver.api.core.cql.ResultSet;
@@ -56,6 +57,7 @@
5657
import com.datastax.oss.driver.internal.core.DefaultProtocolFeature;
5758
import com.datastax.oss.driver.internal.core.ProtocolVersionRegistry;
5859
import com.datastax.oss.driver.internal.core.context.InternalDriverContext;
60+
import com.datastax.oss.driver.internal.core.type.DefaultUserDefinedType;
5961
import com.datastax.oss.driver.internal.core.util.RoutingKey;
6062
import com.datastax.oss.driver.internal.core.util.concurrent.CompletableFutures;
6163
import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList;
@@ -397,6 +399,53 @@ public void should_set_all_occurrences_of_variable() {
397399
should_set_all_occurrences_of_variable(ps.boundStatementBuilder().setInt(id, 12).build());
398400
}
399401

402+
@Test
403+
public void disable_skip_metadata() {
404+
// v0 is an int column, but we'll bind a String to it
405+
try (CqlSession session = sessionWithNoSkipMetadata()) {
406+
session.execute("CREATE TYPE IF NOT EXISTS udttype(x int, y int)");
407+
408+
session.execute("CREATE TABLE udt_test (pk int, v udttype, PRIMARY KEY (pk))");
409+
session.execute("CREATE TABLE regular_table (pk int, v int, PRIMARY KEY (pk))");
410+
411+
session.execute("INSERT INTO regular_table (pk, v) VALUES (1, 1)");
412+
session.execute("INSERT INTO udt_test (pk, v) VALUES (1, {x: 1, y: 1})");
413+
414+
PreparedStatement prepared_regular_table =
415+
session.prepare("SELECT * FROM regular_table WHERE pk = ?");
416+
PreparedStatement prepared_udt_all = session.prepare("SELECT * FROM udt_test WHERE pk = ?");
417+
PreparedStatement prepared_udt_concrete =
418+
session.prepare("SELECT pk, v FROM udt_test WHERE pk = ?");
419+
420+
Row row = session.execute(prepared_udt_all.bind(1)).one();
421+
assertThat(row.getColumnDefinitions().size()).isEqualTo(2);
422+
assertThat(get_udt_column_count(row.getColumnDefinitions().get(1))).isEqualTo(2);
423+
row = session.execute(prepared_udt_concrete.bind(1)).one();
424+
assertThat(get_udt_column_count(row.getColumnDefinitions().get(1))).isEqualTo(2);
425+
assertThat(row.getColumnDefinitions().size()).isEqualTo(2);
426+
row = session.execute(prepared_regular_table.bind(1)).one();
427+
assertThat(row.getColumnDefinitions().size()).isEqualTo(2);
428+
429+
session.execute("ALTER TABLE regular_table ADD z int;");
430+
session.execute("ALTER TYPE udttype ADD z int;");
431+
432+
row = session.execute(prepared_udt_all.bind(1)).one();
433+
row.getUdtValue(1);
434+
assertThat(row.getColumnDefinitions().size()).isEqualTo(2);
435+
assertThat(get_udt_column_count(row.getColumnDefinitions().get(1))).isEqualTo(3);
436+
row = session.execute(prepared_udt_concrete.bind(1)).one();
437+
row.getUdtValue(1);
438+
assertThat(row.getColumnDefinitions().size()).isEqualTo(2);
439+
assertThat(get_udt_column_count(row.getColumnDefinitions().get(1))).isEqualTo(3);
440+
row = session.execute(prepared_regular_table.bind(1)).one();
441+
assertThat(row.getColumnDefinitions().size()).isEqualTo(3);
442+
}
443+
}
444+
445+
private int get_udt_column_count(ColumnDefinition cd) {
446+
return ((DefaultUserDefinedType) cd.getType()).getFieldTypes().size();
447+
}
448+
400449
private void should_set_all_occurrences_of_variable(BoundStatement bs) {
401450
assertThat(bs.getInt(0)).isEqualTo(12);
402451
assertThat(bs.getInt(1)).isEqualTo(12);
@@ -448,6 +497,18 @@ private CqlSession sessionWithCustomCodec(CqlIntToStringCodec codec) {
448497
.build();
449498
}
450499

500+
private CqlSession sessionWithNoSkipMetadata() {
501+
return (CqlSession)
502+
SessionUtils.baseBuilder()
503+
.addContactEndPoints(ccmRule.getContactPoints())
504+
.withKeyspace(sessionRule.keyspace())
505+
.withConfigLoader(
506+
SessionUtils.configLoaderBuilder()
507+
.withBoolean(DefaultDriverOption.REQUEST_DISABLE_SKIP_METADATA, true)
508+
.build())
509+
.build();
510+
}
511+
451512
private boolean supportsPerRequestKeyspace(CqlSession session) {
452513
InternalDriverContext context = (InternalDriverContext) session.getContext();
453514
ProtocolVersionRegistry protocolVersionRegistry = context.getProtocolVersionRegistry();

0 commit comments

Comments
 (0)