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.

Sign up for GitHub

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

feature(views-with-tablets): enable testing materialized views with tablets #9615

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feature(views-with-tablets): enable testing materialized views with tablets #9615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions defaults/test_default.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ skip_download: false
authenticator_user: ''
authenticator_password: ''

experimental_features: ['views-with-tablets']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Materialized views are currently not allowed in tablet keyspaces, due to some
pending issues. However, they can be enabled using the "views-with-tablets" experimental feature.

This is not technically true, they can be created in tablet keyspaces right now but we want to forbid this. We introduced an experimental feature first, now we want to adjust the test and, finally, we will forbid creating materialized views without the experimental feature being enabled.

Just clarifying so that other reviewers won't get confused why tests that use MVs are still working. I recommend updating the description.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When testing upgrades to future versions, we'll need to
keep the experimental_feature also for base version, with an exception for sequential upgrades,
(currently we only have 2021.1->2022.1->2023.1 test in test_custom_profile_sequential_rolling_upgrade)
where the base version of the sequence might not have this experimental feature, so we'll need
to adjust it accordingly.

It would be great if the feature were enabled programmatically for the base version as well, but I'm not familiar with SCT enough to tell you how to do that. @fruch perhaps you will know?

If it's not feasible, alternatively you could add a comment near the experimental_features key which explains when the views-with-tablets feature needs to be provided and when not.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Materialized views are currently not allowed in tablet keyspaces, due to some
pending issues. However, they can be enabled using the "views-with-tablets" experimental feature.

This is not technically true, they can be created in tablet keyspaces right now but we want to forbid this.

I updated the description for the current state

It would be great if the feature were enabled programmatically for the base version as well, but I'm not familiar with SCT enough to tell you how to do that. @fruch perhaps you will know?

I'm not sure there is such a way in general for all tests. I saw that when tablets were introduced, we added a enable_tablets_on_upgrade config option that we used on specific test pipelines - we could probably do something like that here as well, but currently no tests would use it, as we don't have tests for upgrading to 2025.1, and the developers that were adding these tests would have to remember about it.

If it's not feasible, alternatively you could add a comment near the experimental_features key which explains when the views-with-tablets feature needs to be provided and when not.

For now I added a comment for the experimental_features configuration option

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only downside of using this in default for everything, that is gonna block people from using older versions

And it's gonna waste lots of people's time,

We are using older releases during development

I think this is different from the regular extra configuration in scylla yaml that wasn't there before, and existence of it does not break anything

I think moving it into the with the same version logic you used in upgrade tests

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Older versions of scylla which do not support this experimental feature will not recognize it, and will fail to start. If we are using SCT with older versions of scylla then it sounds like it will be a problem.

@fruch is there a way to adjust the configuration programmatically?

I found an old PR which removed the experimental "tablets" feature from SCT (#7512). It looks like the approach there was to modify Jenkinsfiles (configurations/raft/enable_raft_experimental.yaml). Would it be a viable approach here instead of adding "views-with-tablets" to the list of default features?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@piodul
I agree with @fruch that defaults must not limit us to only one scylla version.
For example, I use various scylla versions with the master SCT branch all the time.

Yes, additional small file in the configurations is good solution.

Also, note that each CI job can be provided with the updated SCT config option in the jenkins pipeline extra_environment_variables parameter.
One of the recent examples:
Screenshot from 2025-01-16 20-08-09

In this case it is not needed to update SCT configs.

So, choose whatever fits your needs better.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, today I was hit with the same problem in test/cqlpy/run's "--release" feature which is supposed to allow running old versions of Scylla, but the newly added views-with-tablets doesn't work on old versions. My fix is scylladb/scylladb#22350. Basically test/cqlpy/run.py has a rather-ugly list of configuration "overrides", specific options that need to be added or removed from specific past versions. This list is ugly, but isn't long or hard, and I'm rather please with the power it gives me to run tests against old versions (e.g., just today in scylladb/scylladb#22354 I wrote a test and then ran it against 10 different versions of Scylla).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@piodul

there two place we can do it programmatically:

  1. in proposed_scylla_yaml, where we read append_scylla_yaml and use it
  2. in sct_config.py after sct_config.get_version_based_on_conf() is called, and we have the version information

the first is the simplest, but it's buried in not so clear place
the 2nd might be better and clear place for more changes per release we need to do in SCT configuration.


# gemini defaults
n_test_oracle_db_nodes: 1
gemini_seed: 0
Expand Down
3 changes: 2 additions & 1 deletion docs/configuration_options.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,8 @@ Set type of ssh library to use. Could be 'fabric' (default) or 'libssh2'

## **experimental_features** / SCT_EXPERIMENTAL_FEATURES

unlock specified experimental features
Unlock specified experimental features. The 'views-with-tablets` feature is enabled in the default configuration, but it should not be enabled when testing versions older than 2025.1-dev or 6.3-dev
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6.3-dev won't exist



**default:** N/A

Expand Down
2 changes: 2 additions & 0 deletions test-cases/performance/perf-regression-latency-650gb-upgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ append_scylla_args: '--blocked-reactor-notify-ms 5 --abort-on-lsa-bad-alloc 1 --
backtrace_decoding: false
print_kernel_callstack: true

experimental_features: []

store_perf_results: true
email_recipients: ["scylla-perf-results@scylladb.com"]
use_prepared_loaders: true
Expand Down
2 changes: 2 additions & 0 deletions test-cases/upgrades/customer-profile/rolling-upgrade-custom-d1.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ append_scylla_args: '--blocked-reactor-notify-ms 500 --abort-on-lsa-bad-alloc 1

use_mgmt: false

experimental_features: []

use_prepared_loaders: true

append_scylla_yaml:
Expand Down
2 changes: 2 additions & 0 deletions test-cases/upgrades/generic-rolling-upgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ internode_compression: 'all'

use_mgmt: false

experimental_features: []

use_preinstalled_scylla: false

# those are needed to be give by the job, via environment variable
Expand Down
2 changes: 2 additions & 0 deletions test-cases/upgrades/multi-dc-rolling-upgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ authenticator_password: 'cassandra'

internode_compression: 'all'

experimental_features: []

use_mgmt: false

# those are needed to be give by the job, via environment variable
Expand Down
2 changes: 2 additions & 0 deletions test-cases/upgrades/rolling-upgrade-latency-regression.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ nemesis_class_name: 'NoOpMonkey'
nemesis_during_prepare: false
use_mgmt: false

experimental_features: []

user_prefix: 'rolling-upgrade-ltncy-rgrssn'

server_encrypt: true
Expand Down
2 changes: 1 addition & 1 deletion unit_tests/test_data/test_scylla_yaml_builders/PR-provision-test.result.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
]
}
],
"experimental_features": [],
"experimental_features": ["views-with-tablets"],
"prometheus_address": "0.0.0.0",
"alternator_enforce_authorization": false,
"auto_bootstrap": true,
Expand Down
2 changes: 1 addition & 1 deletion unit_tests/test_data/test_scylla_yaml_builders/functional.result.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
]
}
],
"experimental_features": [],
"experimental_features": ["views-with-tablets"],
"prometheus_address": "0.0.0.0",
"alternator_enforce_authorization": false,
"auto_bootstrap": true,
Expand Down
2 changes: 1 addition & 1 deletion unit_tests/test_data/test_scylla_yaml_builders/jepsen.result.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
]
}
],
"experimental_features": [],
"experimental_features": ["views-with-tablets"],
"prometheus_address": "0.0.0.0",
"alternator_enforce_authorization": false,
"auto_bootstrap": true,
Expand Down
2 changes: 1 addition & 1 deletion unit_tests/test_data/test_scylla_yaml_builders/longevity-10gb-3h.result.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
}
],
"broadcast_rpc_address": "__NODE_IPV6_ADDRESS__",
"experimental_features": [],
"experimental_features": ["views-with-tablets"],
"prometheus_address": "__NODE_IPV6_ADDRESS__",
"enable_ipv6_dns_lookup": true,
"alternator_enforce_authorization": false,
Expand Down
2 changes: 1 addition & 1 deletion unit_tests/test_data/test_scylla_yaml_builders/manager-backup-1TB-gce.result.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
]
}
],
"experimental_features": [],
"experimental_features": ["views-with-tablets"],
"prometheus_address": "0.0.0.0",
"alternator_enforce_authorization": false,
"auto_bootstrap": true,
Expand Down
2 changes: 1 addition & 1 deletion .../test_scylla_yaml_builders/multi_network_interfaces/cdc-replication-longevity.result.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
}
],
"broadcast_rpc_address": "__NODE_PRIVATE_ADDRESS_2__",
"experimental_features": [],
"experimental_features": ["views-with-tablets"],
"prometheus_address": "0.0.0.0",
"alternator_enforce_authorization": false,
"auto_bootstrap": true,
Expand Down
2 changes: 1 addition & 1 deletion ...est_data/test_scylla_yaml_builders/multi_network_interfaces/longevity-10gb-3h.result.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
}
],
"broadcast_rpc_address": "__NODE_PRIVATE_ADDRESS_2__",
"experimental_features": [],
"experimental_features": ["views-with-tablets"],
"prometheus_address": "0.0.0.0",
"enable_ipv6_dns_lookup": false,
"alternator_enforce_authorization": false,
Expand Down
2 changes: 1 addition & 1 deletion ...tests/test_data/test_scylla_yaml_builders/perf-regression.100threads.30M-keys.result.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
]
}
],
"experimental_features": [],
"experimental_features": ["views-with-tablets"],
"prometheus_address": "0.0.0.0",
"alternator_enforce_authorization": false,
"auto_bootstrap": true,
Expand Down
2 changes: 2 additions & 0 deletions unit_tests/test_data/test_scylla_yaml_builders/rolling-upgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ authenticator_password: 'cassandra'

use_mgmt: false

experimental_features: []

gemini_cmd: "gemini -d --duration 2h \
-c 10 -m write -f --non-interactive --cql-features normal \
--max-mutation-retries 5 --max-mutation-retries-backoff 500ms \
Expand Down
2 changes: 2 additions & 0 deletions unit_tests/test_scylla_yaml.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def test_scylla_yaml(self):
parameters=[{'seeds': ['1.1.1.1', '2.2.2.2']}]),
],
'force_schema_commit_log': True,
'experimental_features': None,
},
expected_without_defaults={
'background_writer_scheduling_quota': 1.0,
Expand All @@ -151,6 +152,7 @@ def test_scylla_yaml(self):
'prometheus_prefix': 'someprefix',
'log_to_stdout': True,
'force_schema_commit_log': True,
'experimental_features': None,
},
expected_with_defaults={
'abort_on_ebadf': None,
Expand Down
5 changes: 4 additions & 1 deletion upgrade_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
from sdcm.utils.user_profile import get_profile_content
from sdcm.utils.version_utils import (
get_node_supported_sstable_versions,
ComparableScyllaVersion,
is_enterprise,
get_node_enabled_sstable_version
)
Expand Down Expand Up @@ -221,7 +222,9 @@ def _upgrade_node(self, node, upgrade_sstables=True, new_scylla_repo=None, new_v

if self.params.get("enable_tablets_on_upgrade"):
scylla_yaml_updates.update({"enable_tablets": True})

version = ComparableScyllaVersion(self.scylla_version)
if self.is_enterprise and version >= "2025.1.0~dev" or not self.is_enterprise and version >= "6.3.0~dev":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the upcoming versioning changes, the6.3 is not planned to appear.
So, the 2025.1.0~dev must be the only version to be checked here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6.3.0-dev does exists, it never gonna be released
since today master is 2025.1.0-dev, so we can drop it now

scylla_yaml_updates.update({"experimental_features": ["views-with-tablets"]})
Comment on lines +225 to +227
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need something similar in _rollback_node, too?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed we reused the original scylla.yaml after rollback because we don't revert other updates (like enable_tablets) to it either, but I don't see it explicitly so @scylladb/qa-maintainers please confirm or deny

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, someone need to cross check it in the code

Anyhow I would recommend appending to the flags, now the code is override anything that any might have put in there before

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We restore original scylla.yaml file. See following:
/~https://github.com/scylladb/scylla-cluster-tests/blame/c22fba4d/upgrade_test.py#L434

if self.params.get('test_sst3'):
scylla_yaml_updates.update({"enable_sstables_mc_format": True})

Expand Down
Loading