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

wmitros · 2024-12-24T13:54:08Z

We plan to disallow materialized views in tablet keyspaces, until the remaining issues with them are resolved. For that, we've introduced the "views-with-tablets" experimental feature, so that they can be enabled regardless, for example for testing. Currently, the feature has no effect, but we want to prepare the cluster tests to use it, so that they won't start failing when it starts actually having an effect.

While improving views with tablets, we should make sure that the existing tests keep passing, so we should run materialized view (and index) tests with tablets using the experimental feature.

The feature doesn't affect tests ran on vnodes. For tablets, the tests would fail without the feature due to unsupported configuration.

Considering that, we should enable the feature by default for all tests (with a special consideration to upgrade tests).

The change includes:

modifying the default experimental_features in defaults/test_default.yaml
updating json results for YAML tests accordingly to the new default
updating upgrade tests - for upgrades, the first version with this experimental feature will be 2025.1, so the upgrade tests are updated to remove this feature from the defaults and to set it when upgrading to 2025.1 or higher. 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.

wmitros · 2024-12-24T14:30:51Z

Looks like yaml tests aren't using the default configurations, fixed in rebase

wmitros · 2025-01-07T15:38:05Z

Refs scylladb/scylladb#21832

piodul · 2025-01-13T07:38:29Z

@wmitros why is this PR a draft? What is missing?

wmitros · 2025-01-13T10:40:20Z

@wmitros why is this PR a draft? What is missing?

I think it's ready, forgot to change the type

piodul · 2025-01-14T13:44:20Z

defaults/test_default.yaml

@@ -110,6 +110,8 @@ skip_download: false
 authenticator_user: ''
 authenticator_password: ''

+experimental_features: ['views-with-tablets']


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.

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.

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

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

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?

@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:

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

So, choose whatever fits your needs better.

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).

@piodul

there two place we can do it programmatically:

in proposed_scylla_yaml, where we read append_scylla_yaml and use it

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.

piodul · 2025-01-14T14:01:46Z

upgrade_test.py

+        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":
+            scylla_yaml_updates.update({"experimental_features": ["views-with-tablets"]})


Do we need something similar in _rollback_node, too?

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

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

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

…ablets We plan to disallow materialized views in tablet keyspaces, until the remaining issues with them are resolved. For that, we've introduced the "views-with-tablets" experimental feature, so that they can be enabled regardless, for example for testing. Currently, the feature has no effect, but we want to prepare the cluster tests to use it, so that they won't start failing when it starts actually having an effect. While improving views with tablets, we should make sure that the existing tests keep passing, so we should run materialized view (and index) tests with tablets using the experimental feature. The feature doesn't affect tests ran on vnodes. For tablets, the tests would fail without the feature due to unsupported configuration. Considering that, we should enable the feature by default for all tests (with a special consideration to upgrade tests). The change includes: * modifying the default experimental_features in defaults/test_default.yaml * updating upgrade tests - for upgrades, the first version with this experimental feature will be 2025.1, so the upgrade tests are updated to remove this feature from the defaults and to set it when upgrading to 2025.1 or higher. 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.

vponomaryov · 2025-01-14T10:51:41Z

upgrade_test.py

@@ -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":


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.

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

vponomaryov · 2025-01-16T18:11:33Z

defaults/test_default.yaml

@@ -110,6 +110,8 @@ skip_download: false
 authenticator_user: ''
 authenticator_password: ''

+experimental_features: ['views-with-tablets']


@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:

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

So, choose whatever fits your needs better.

vponomaryov · 2025-01-16T18:13:45Z

docs/configuration_options.md

@@ -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


6.3-dev won't exist

vponomaryov · 2025-01-16T18:37:39Z

upgrade_test.py

+        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":
+            scylla_yaml_updates.update({"experimental_features": ["views-with-tablets"]})


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

github-actions bot assigned wmitros Dec 24, 2024

wmitros added the backport/none Backport is not required label Dec 24, 2024

wmitros force-pushed the forbid-tablet-mv branch from 0beb533 to 2d59969 Compare December 24, 2024 14:30

wmitros force-pushed the forbid-tablet-mv branch from 2d59969 to 8380a4c Compare December 28, 2024 13:02

wmitros marked this pull request as ready for review January 13, 2025 10:39

wmitros force-pushed the forbid-tablet-mv branch from 8380a4c to 8e26385 Compare January 13, 2025 21:36

wmitros requested a review from fruch January 13, 2025 22:00

piodul reviewed Jan 14, 2025

View reviewed changes

wmitros force-pushed the forbid-tablet-mv branch 2 times, most recently from 8b5e8ef to 573509a Compare January 15, 2025 22:07

wmitros force-pushed the forbid-tablet-mv branch from 573509a to 5f575d9 Compare January 15, 2025 22:08

vponomaryov requested changes Jan 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

wmitros commented Dec 24, 2024 •

edited

Loading

wmitros commented Dec 24, 2024

wmitros commented Jan 7, 2025

piodul commented Jan 13, 2025

wmitros commented Jan 13, 2025

piodul Jan 14, 2025

piodul Jan 14, 2025

wmitros Jan 15, 2025

fruch Jan 16, 2025

piodul Jan 16, 2025

vponomaryov Jan 16, 2025

nyh Jan 16, 2025

fruch Jan 16, 2025

piodul Jan 14, 2025

wmitros Jan 15, 2025

fruch Jan 16, 2025

vponomaryov Jan 16, 2025

vponomaryov Jan 14, 2025

fruch Jan 16, 2025

vponomaryov Jan 16, 2025

vponomaryov Jan 16, 2025

vponomaryov Jan 16, 2025

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

Are you sure you want to change the base?

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

Conversation

wmitros commented Dec 24, 2024 • edited Loading

wmitros commented Dec 24, 2024

wmitros commented Jan 7, 2025

piodul commented Jan 13, 2025

wmitros commented Jan 13, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wmitros commented Dec 24, 2024 •

edited

Loading