Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Internal schema change CQL queries should not be used for distributed tables #6700

Closed
psarna opened this issue Jun 23, 2020 · 4 comments
Closed

Comments

@psarna
Copy link
Contributor

psarna commented Jun 23, 2020

As @nyh suggested in #6513, I'm creating a separate issue for a thing that came up in the discussion.

Internal CQL queries are generally expected to be used for local tables, not regular, distributed ones. One of the reasons is that any schema changes issued by internal CQL queries are only propagated locally, which means that calling ALTER TABLE on a regular, distributed table will result in not propagating this change to other nodes, which leads to schema inconsistencies. There should be an assertion/warning somewhere that fires when somebody tries to call an internal query on a regular, distributed table. Such misuse (of mine) was the root cause of #6513.

Example internal query:

    sstring req = format("ALTER TABLE \"{}\".\"{}\" WITH {} = {}",
            schema->ks_name(), schema->cf_name(), tags_extension::NAME, serialized_tags_str);
    return db::execute_cql(std::move(req)).discard_result();
@psarna psarna self-assigned this Jun 24, 2020
avikivity pushed a commit that referenced this issue Jun 24, 2020
Local altering, which does not propagate the change to other nodes,
should not be allowed for a non-local table.

Refs #6700
Message-Id: <34a2b191c0e827f296e6d720dc31bf8bda0fd160.1592990796.git.sarna@scylladb.com>
@psarna psarna removed their assignment Jun 25, 2020
@psarna
Copy link
Contributor Author

psarna commented Jun 25, 2020

Let's reopen - altering a schema is already guarded, but there are more statements that probably shouldn't be called from within the database - like CREATE TABLE, CREATE MATERIALIZED VIEW, etc.

@psarna psarna reopened this Jun 25, 2020
@slivne
Copy link
Contributor

slivne commented Jul 2, 2020

@psarna are you working on a fix ?

@tgrabiec any comments on this ?

@psarna
Copy link
Contributor Author

psarna commented Jul 2, 2020

I wasn't actively working on it, but it's a simple thing, so I'll squeeze it in.

psarna added a commit to psarna/scylla that referenced this issue Jul 2, 2020
Changing the schemas via internal calls to CQL is dangerous,
since the changes are not propagated to other nodes. Thus, it should
never be used for regular distributed tables.
The guarding code was already added for ALTER TABLE statement
and it's now expanded to cover all schema altering statements.

Tests: unit(dev)
Fixes scylladb#6700
nyh added a commit that referenced this issue Jul 7, 2020
Merged patch set from Piotr Sarna:

This series addresses issue #6700 again (it was reopened),
by forbidding all non-local schema changes to be performed
from within the database via CQL interface. These changes
are dangerous since they are not directly propagated to other
nodes.

Tests: unit(dev)
Fixes #6700

Piotr Sarna (4):
  test: make schema changes in query_processor_test global
  cql3: refuse to change schema internally for distributed tables
  test: expand testing internal schema changes
  cql3: add explanatory comments to execute_internal

 cql3/query_processor.hh                      | 13 ++++++++++++-
 cql3/statements/alter_table_statement.cc     |  6 ------
 cql3/statements/schema_altering_statement.cc | 15 +++++++++++++++
 test/boost/cql_query_test.cc                 |  8 ++++++--
 test/boost/query_processor_test.cc           | 16 ++++++++--------
 5 files changed, 41 insertions(+), 17 deletions(-)
avikivity pushed a commit that referenced this issue Jul 7, 2020
Merged patch set from Piotr Sarna:

This series addresses issue #6700 again (it was reopened),
by forbidding all non-local schema changes to be performed
from within the database via CQL interface. These changes
are dangerous since they are not directly propagated to other
nodes.

Tests: unit(dev)
Fixes #6700

Piotr Sarna (4):
  test: make schema changes in query_processor_test global
  cql3: refuse to change schema internally for distributed tables
  test: expand testing internal schema changes
  cql3: add explanatory comments to execute_internal

 cql3/query_processor.hh                      | 13 ++++++++++++-
 cql3/statements/alter_table_statement.cc     |  6 ------
 cql3/statements/schema_altering_statement.cc | 15 +++++++++++++++
 test/boost/cql_query_test.cc                 |  8 ++++++--
 test/boost/query_processor_test.cc           | 16 ++++++++--------
 5 files changed, 41 insertions(+), 17 deletions(-)
@avikivity
Copy link
Member

Not a bug, not backporting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants