ALTER KEYSPACE can break tables with UDT columns

[cvybhu]

I've created a 2 node cluster (2 DCs, 1 node in each) of the latest Scylla (5.4.0~dev-0.20230605.d2e089777bf5).

ccm create c2x1-release -n 1:1 --scylla --vnodes --install-dir /home/jancio/code/scylla/build/release

Running the following queries using cqlsh fails:

CREATE KEYSPACE ks WITH replication = {'class': 'NetworkTopologyStrategy', 'dc1': 1};
CREATE TYPE ks.my_type (a int, b int);
CREATE TABLE ks.tab (p int PRIMARY KEY, m ks.my_type);
ALTER KEYSPACE ks WITH replication = {'class': 'NetworkTopologyStrategy', 'dc1': 1, 'dc2': 1};
ALTER TABLE ks.tab WITH compression = { 'sstable_compression': 'org.apache.cassandra.io.compress.ZstdCompressor'};

NoHostAvailable: ('Unable to complete the operation against any hosts', {<Host: 127.0.0.1:9042 dc1>: <Error from server: code=0000 [Server error] message="Raft instance is stopped, reason: "background error, std::_Nested_exception<raft::state_machine_error> (State machine error at raft/server.cc:1175): exceptions::invalid_request_exception (Unknown type ks.my_type)"">})

The main problems appears to be:

Unknown type ks.my_type

It looks like the ALTER KEYSPACE query changes the UDT version, but other nodes are not aware of it and still use the old UDTs, and then fail on schema operations like ALTER TABLE or DROP TABLE.

Installation details
Scylla version (or git commit hash): 5.4.0~dev-0.20230605.d2e089777bf5
Cluster size: 2 (2 datacenters, 1 node in each DC)
OS (RHEL/CentOS/Ubuntu/AWS AMI): Fedora 37

(removed references to enterprise repository)

[mykaul]

Is that a regression?

[cvybhu]

Is that a regression?

It is. I ran the same queries on release:2020.1.0 and they completed successfully.

[mykaul]

Is that a regression?

It is. I ran the same queries on release:2020.1.0 and they completed successfully.

That's an old (enterprise) release. Let's see if we can narrow this down (with OSS versions that'd be great, btw)

[avikivity]

Please attach the backtrace so we see who can fix it.

[tgrabiec]

query changes the UDT version

What do you mean by "version"? Aren't UDTs identified by name?

[tgrabiec]

Please attach the backtrace so we see who can fix it.

The error is thrown when trying to deserialize schema definition which references the type.

[avikivity]

The issue is likely that ALTER KEYSPACE somehow forgot the type.

[avikivity]

migration_manager::prepare_keyspace_update_announcement() calls db::schema_tables::make_create_keyspace_mutations(), with the parameter with_tables_and_types_and_functions defaulting to true. Not only it should have worked with false, but if it's true, then we are passing the types.

[cvybhu]

Narrowed it down to between 5.0 and 5.1

Works on scylla-5.0.13.0.20230423.a0ca8abe4
Fails on: scylla5.1.11-0.20230522.88eeab78380c

[cvybhu]

What do you mean by "version"? Aren't UDTs identified by name?

I don't really mean anything, I don't know how this part of the code works x.x

[avikivity]

A bisect can be the easiest way to identify the problem.

[cvybhu]

Bisecting...

[michoecho]

Culprit: 5852959

Fix:

diff --git a/data_dictionary/data_dictionary.cc b/data_dictionary/data_dictionary.cc
index a91e9b8769..55411bdf97 100644
--- a/data_dictionary/data_dictionary.cc
+++ b/data_dictionary/data_dictionary.cc
@@ -216,7 +216,7 @@ keyspace_metadata::keyspace_metadata(std::string_view name,
                         std::move(strategy_options),
                         durable_writes,
                         std::move(cf_defs),
-                        user_types_metadata{},
+                        std::move(user_types),
                         storage_options{}) { }

 keyspace_metadata::keyspace_metadata(std::string_view name,

[cvybhu]

@michoecho could you make a PR with your fix?

[avikivity]

Wow that is so sad.

[michoecho]

@michoecho could you make a PR with your fix?

Done: #14143

Also wrote a pytest which reproduces the issue (not sure if it's worth adding, but why not): #14144

[mykaul]

@michoecho could you make a PR with your fix?

Done: #14143

Also wrote a pytest which reproduces the issue (not sure if it's worth adding, but why not): #14144

It's worth adding, IMHO.

[raphaelsc]

Good catch.

[avikivity]

Backported to 5.1, 5.2, 5.3.