qos/raft_service_level_distributed_data_accessor: print correct error message when trying to modify a service level in recovery mode #18841
Conversation
When setting/dropping a service level using raft data accessor, the same validation steps are executed (this_shard_id = 0 and guard is present). To not duplicate the calls in both functions, they can be extracted to a helper function.
mode When a cluster goes into recovery mode and service levels were migrated to raft, service levels become temporarily read-only. This commit adds a proper error message in case a user tries to do any changes.
github-actions
bot
added
symptom/ux
| logging.info("Checking changes to service levels are forbidden during recovery") | ||
| with pytest.raises(InvalidRequest, match="The cluster is in recovery mode. Changes to service levels are not allowed."): | ||
| await cql.run_async(f"CREATE SERVICE LEVEL sl_{unique_name() }") |
There was a problem hiding this comment.
It would be nice to check other possible operations on service levels as well - altering, dropping, attaching, detaching.
There was a problem hiding this comment.
Added tried to alter and drop a service levele.
However attaching/detaching a service level is part of auth, which also prints the same ugly message in recovery
There was a problem hiding this comment.
However attaching/detaching a service level is part of auth, which also prints the same ugly message in recovery
Ok, no need to increase the scope of the PR to cover them, then. I'll open an issue about the ugly error messages in auth, this can be handled there.
"error message in recovery mode" doesn't really explain what your changes are supposed to do. Your PR improves error messages that are generated when trying to modify service levels in recovery mode. Please improve the title. |
|
In general, changes look good to me, but please make sure that you cover all kinds of operations on service levels and test them all. |
🟢 CI State: SUCCESS✅ - Build Build Details:
|
Jadw1
changed the title
|
Not sure what went wrong with the Jenkins job, it looks like Jenkins was being restarted around the time the job was scheduled and the job failed with a weird error. Retriggering. |
|
Jenkins crashed again retriggering |
🟢 CI State: SUCCESS✅ - Build Build Details:
|
|
Queued |
…r: print correct error message when trying to modify a service level in recovery mode' from ScyllaDB Raft service levels are read-only in recovery mode. This patch adds check and proper error message when a user tries to modify service levels in recovery mode. Fixes #18827 (cherry picked from commit 2b56158) (cherry picked from commit ee08d7f) (cherry picked from commit af0b6bc) Refs #18841 Closes #18913 * github.com:scylladb/scylladb: test/auth_cluster/test_raft_service_levels: try to create sl in recovery service/qos/raft_sl_dda: reject changes to service levels in recovery mode service/qos/raft_sl_dda: extract raft_sl_dda steps to common function
Raft service levels are read-only in recovery mode. This patch adds check and proper error message when a user tries to modify service levels in recovery mode.
Fixes #18827