[
https://issues.apache.org/jira/browse/CASSANDRA-18042?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17636888#comment-17636888
]
Stefan Miklosovic edited comment on CASSANDRA-18042 at 11/21/22 9:23 PM:
-------------------------------------------------------------------------
PR: [https://github.com/apache/cassandra/pull/2025/files]
j11 pre-commit:
[https://app.circleci.com/pipelines/github/instaclustr/cassandra/1598/workflows/8be819a3-57a8-41ea-a99e-ac44ea145548]
j8 pre-commit:
[https://app.circleci.com/pipelines/github/instaclustr/cassandra/1598/workflows/5068bade-d090-45c1-bda4-1f5a6cb188e9]
CI run includes also 500x on added and affected tests.
The solution presented here modifies EnableFlag in such a way that:
0) this flag is enabled by default
1) when flag is disabled and such configuration combination is found upon
CREATE or ALTER TABLE commands, CQL statement will FAIL
2) when flag is enabled and such configuration combination is found, user will
be WARNED
3) nothing happens when such configuration combination is not found
4) warnings can not be turned off
5) It will warn ordinary users only, it will not warn when a user executing
that CQL statement is super-user / system user
6) system keyspaces are excluded from this guardrail completely
The reasoning behind always warning is that I do not think this is actually
something which should be configurable. It does not make sense to me to create
two properties in cassandra.yaml for this functionality.
Bottom line is that we _can_ have two properties if we _really_ want, the
solution presented here is prepared for that so we would have one property for
warning and the other failure so even warnings for normal users could be turned
off but I really do not find it appropriate.
Examples how it looks:
guardrail disabled (turned to "false"):
{code:java}
stefan@cqlsh> CREATE TABLE ks1.tb44 ( id text primary key ) WITH
default_time_to_live = 0 AND compaction = {'class':
'TimeWindowCompactionStrategy', 'enabled': true };
InvalidRequest: Error from server: code=2200 [Invalid query] message="Guardrail
zero_default_ttl_on_time_window_compaction_strategy_enabled violated: 0
default_time_to_live on a table with TimeWindowCompactionStrategy compaction
strategy is not allowed. It is suspicious to use default_time_to_live set to 0
with such compaction strategy. Please keep in mind that data will not start to
automatically expire after they are older than a respective compaction window
unit of a certain size."
{code}
guardrail enabled (set to "true")
{code:java}
stefan@cqlsh> CREATE TABLE ks1.tb44 ( id text primary key ) WITH
default_time_to_live = 0 AND compaction = {'class':
'TimeWindowCompactionStrategy', 'enabled': true };
Warnings :
Guardrail zero_default_ttl_on_time_window_compaction_strategy_enabled violated:
0 default_time_to_live on a table with TimeWindowCompactionStrategy compaction
strategy. It is suspicious to use default_time_to_live set to 0 with such
compaction strategy. Please keep in mind that data will not start to
automatically expire after they are older than a respective compaction window
unit of a certain size.
{code}
was (Author: smiklosovic):
PR: [https://github.com/apache/cassandra/pull/2025/files]
j11 pre-commit:
[https://app.circleci.com/pipelines/github/instaclustr/cassandra/1598/workflows/8be819a3-57a8-41ea-a99e-ac44ea145548]
j8 pre-commit:
[https://app.circleci.com/pipelines/github/instaclustr/cassandra/1598/workflows/5068bade-d090-45c1-bda4-1f5a6cb188e9]
CI run includes also 500x on added and affected tests.
The solution presented here modifies EnableFlag in such a way that:
0) this flag is enabled by default
1) when flag is disabled and such configuration combination is found upon
CREATE or ALTER TABLE commands, CQL statement will FAIL
2) when flag is enabled and such configuration combination is found, user will
be WARNED
3) nothing happens when such configuration combination is not found
4) warnings can not be turned off
5) It will warn ordinary users only, it will not warn when a user executing
that CQL statement is super-user / system user
The reasoning behind always warning is that I do not think this is actually
something which should be configurable. It does not make sense to me to create
two properties in cassandra.yaml for this functionality.
Bottom line is that we _can_ have two properties if we _really_ want, the
solution presented here is prepared for that so we would have one property for
warning and the other failure so even warnings for normal users could be turned
off but I really do not find it appropriate.
Examples how it looks:
guardrail disabled (turned to "false"):
{code:java}
stefan@cqlsh> CREATE TABLE ks1.tb44 ( id text primary key ) WITH
default_time_to_live = 0 AND compaction = {'class':
'TimeWindowCompactionStrategy', 'enabled': true };
InvalidRequest: Error from server: code=2200 [Invalid query] message="Guardrail
zero_default_ttl_on_time_window_compaction_strategy_enabled violated: 0
default_time_to_live on a table with TimeWindowCompactionStrategy compaction
strategy is not allowed. It is suspicious to use default_time_to_live set to 0
with such compaction strategy. Please keep in mind that data will not start to
automatically expire after they are older than a respective compaction window
unit of a certain size."
{code}
guardrail enabled (set to "true")
{code:java}
stefan@cqlsh> CREATE TABLE ks1.tb44 ( id text primary key ) WITH
default_time_to_live = 0 AND compaction = {'class':
'TimeWindowCompactionStrategy', 'enabled': true };
Warnings :
Guardrail zero_default_ttl_on_time_window_compaction_strategy_enabled violated:
0 default_time_to_live on a table with TimeWindowCompactionStrategy compaction
strategy. It is suspicious to use default_time_to_live set to 0 with such
compaction strategy. Please keep in mind that data will not start to
automatically expire after they are older than a respective compaction window
unit of a certain size.
{code}
> Implement a guardrail for not having zero default ttl on tables with TWCS
> -------------------------------------------------------------------------
>
> Key: CASSANDRA-18042
> URL: https://issues.apache.org/jira/browse/CASSANDRA-18042
> Project: Cassandra
> Issue Type: Improvement
> Components: Feature/Guardrails, Legacy/Core
> Reporter: Stefan Miklosovic
> Assignee: Stefan Miklosovic
> Priority: Normal
> Fix For: 4.x
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> A user was surprised that his data have not started to expire after 90 days
> on his TWCS, he noticed that default_time_to_live on the table was set to 0
> (by accident from his side) and inserts were using TTL = 0 too.
> It is questionable why it it possible to create a table with TWCS and enable
> a user to specify default_time_to_live to be zero.
> On the other hand, I would argue that having default_time_to_live set to 0 on
> TWCS does not necessarily mean that such combination is illegal. It is about
> people just using that with advantage very often so tables are compacted away
> nicely. However, that does not have to mean that they could not use it with
> 0. But I yet have to see a use-case where TWCS was used and default ttl was
> set to 0 on purpose. Merely looking into Cassandra codebase, there are only
> cases when this parameter is not 0.
> There are three approaches:
> 1) just reject such statements (for CreateTable and AlterTable statements)
> where default_time_to_live = 0
> 2) Implement a guardrail for 1) so it can be enabled / disabled on demand
> 3) Leave possibility to set default_time_to_live to 0 on a table but make a
> guardrail for UpdateStatement so it might reject queries for tables with
> default_time_to_live is zero and for which its TTL (on that update statement)
> is set to 0 too.
> I would be careful about making the current configuration illegal because of
> backward compatibility. For that reason 2) makes the most sense to me.
> Maybe implementing 3) would make sense as well. There might be a table which
> has default ttl set to 0 as it expects a user to supply TTL every time.
> However, as it is not currently enforced anywhere, a client might still
> insert TTLs to be set to 0 even by accident.
> POC for 2) is here
> https://github.com/instaclustr/cassandra/commit/0b4dcc3d3deeffa393c02a3b80e27482007f9579
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
