fix(LoaderUtilsMixin): table_enabled was allways True as this feature is allways enabled

[jsmolar]

Enabling/disabling tablets was changed. Now we have to check scylla yaml or keystore schema.
For more info see

• Allow explicitly enabling or disabling tablets when creating a new keyspace scylladb#21451
• conf/scylla.yaml: update documentation for enable_tablets scylladb#21614

Testing

• https://jenkins.scylladb.com/view/staging/job/scylla-staging/job/jsmolar/job/longevity-counters-3h-test/9/

PR pre-checks (self review)

• I added the relevant backport labels
• I didn't leave commented-out/debugging code

Reminders

• Add New configuration option and document them (in sdcm/sct_config.py)
• Add unit tests to cover my changes (under unit-test/ folder)
• Update the Readme/doc folder relevant to this change (if needed)

[cezarmoise]

Is the case where tablets are enabled globally but disabled on the test keyspace something to be considered?

[jsmolar]

Is the case where tablets are enabled globally but disabled on the test keyspace something to be considered?

Afaik, tablets are enabled by default from 6.2 (or so). When enable_tablets is false for append_scylla_yaml, they are disabled for the specific keyspace: CREATE KEYSPACE keyspace1 WITH replication = {'class': 'org.apache.cassandra.locator.NetworkTopologyStrategy', 'us-east': '3'} AND durable_writes = true AND tablets = {'enabled': false};
This behavior in our tests is inconsistent across the test suite, but I belive, it works as described.

[soyacz]

@jsmolar there are plenty of unrelated formatting changes which blur the review process. Please try not doing that if not necessary.

[soyacz]

after reading more details in referenced PR's my understanding is:

1. we still have to query if tablets are enabled for 6.1/2 and 2024.1/2 - we didn't backport config param meaning there. I don't see the point of changing current implementation regards that.
2. enable_tablets config meaning changed only for dev/2025.1 - for this we should have a method/function to determine if tablets are enabled by default for new keyspaces (if we need it anywhere). So we should have new function: is_tablet_default.
3. for versions >= 2025.1 is_tablets_feature_enabled can just return True and skip verifying features.

[pehala]

fter reading more details in referenced PR's my understanding is:

1. we still have to query if tablets are enabled for 6.1/2 and 2024.1/2 - we didn't backport config param meaning there. I don't see the point of changing current implementation regards that.

2. `enable_tablets` config meaning changed only for dev/2025.1 - for this we should have a method/function to determine if tablets are enabled by default for new keyspaces (if we need it anywhere). So we should have new function: `is_tablet_default`.

3. for versions >= 2025.1 `is_tablets_feature_enabled` can just return `True` and skip verifying features.

@soyacz I agree with all those suggestions, but we need a hotfix first for testing 2025.1, as currently some nemesis are skipped even on Vnode runs because this check is now outdated. So I propose merging this (If it works ofc) and then immediately start working on a much better and comprehensive solution.

[soyacz]

@soyacz I agree with all those suggestions, but we need a hotfix first for testing 2025.1, as currently some nemesis are skipped even on Vnode runs because this check is now outdated. So I propose merging this (If it works ofc) and then immediately start working on a much better and comprehensive solution.

Ok.

@pehala please create issue that will take above considerations.
Actually, problem is deeper if we want to test both vnodes and tablets in one test. Basically, we should have 3 methods: is_tablets_feature_enabled (soon to be obsolete), is_tablets_default and is_tablets_used (still test may use tablets despite of tablets default set to false).

@jsmolar please let us know if the most recent push was tested

[Lakshmipathi]

but we need a hotfix first for testing 2025.1, as currently some nemesis are skipped even on Vnode runs because this check is now outdated.

Ok, now I understand why this run fails https://jenkins.scylladb.com/job/scylla-staging/job/LakshmipathiGanapathi/job/longevity-counters-3h-test/14/console with message Tablets feature is enabled, skipping counter stress even though tablets are not enabled.

[fruch]

LGTM

[soyacz]

@pehala please create issue that will take above considerations.
Actually, problem is deeper if we want to test both vnodes and tablets in one test. Basically, we should have 3 methods: is_tablets_feature_enabled (soon to be obsolete), is_tablets_default and is_tablets_used (still test may use tablets despite of tablets default set to false).

@pehala @jsmolar ^^ so we don't forget

[jsmolar]

@pehala please create issue that will take above considerations.
Actually, problem is deeper if we want to test both vnodes and tablets in one test. Basically, we should have 3 methods: is_tablets_feature_enabled (soon to be obsolete), is_tablets_default and is_tablets_used (still test may use tablets despite of tablets default set to false).

@pehala @jsmolar ^^ so we don't forget

I created issue for this: #10168