-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
make enable_compacting_data_for_streaming_and_repair truly live-update #18705
base: master
Are you sure you want to change the base?
Conversation
🔴 CI State: FAILURE✅ - Build Failed Tests (198/600):
Build Details:
|
Could we just remove the configuration? it smells like we added it so it can be disabled if broken, not because there's a reason not to do it. The changelog doesn't say the same thing, but not too far either. commit 9e3987f
|
Though I guess, if you went to the trouble of fixing it, probably you did want to disable it. |
We have hit a bug in the compacting reader, causing false-positive validation errors (see #18667) and to work around it field wanted to disable compacting during streaming, and that ended up needing a RR. |
…g_and_repair This config item is propagated to the table object via table::config. Although the field in table::config, used to propagate the value, was utils::updateable_value<T>, it was assigned a constant and so the live-update chain was broken. This patch fixes this.
Allow injection points save values into the "value" parameter, which external code can then examine. This allows exfiltrating the values if internal variables, to be examined by tests, without exposing these variables via an "official" path.
Allow external code to obtain information about an error injection point, including whether it is enabled, and importantly, what its parameters are. Together with the `set_parameter()` added in the previous patch, this allows tests to read out the values of internal parameters, via a set_parameter() injection point.
The /v2/error_injection/{injection} endpoint now has a GET method too, expose this.
…ng_and_repair live-update Avoid this the live-update feature of this config item breaking silently.
a8dbe99
to
8f617b3
Compare
New in v2:
|
🔴 CI State: FAILURE✅ - Build Failed Tests (2/31706):Build Details:
|
🔴 CI State: FAILURE✅ - Build Failed Tests (1/30873):Build Details:
|
test_repair_compacts_data dtest is broken by this PR, looks like it silently relied on live-update being broken. I need to fix the dtest first. |
haha |
dtest fix PR: https://github.com/scylladb/scylla-dtest/pull/4322 |
This config item is propagated to the table object via table::config. Although the field in
table::config
, used to propagate the value, wasutils::updateable_value<T>
, it was assigned a constant and so the live-update chain was broken.This series fixes this and adds a test which fails before the patch and passes after. The test needed new test infrastructure, around the failure injection api, namely the ability to exfiltrate the value of internal variable. This infrastructure is also added in this series.
Fixes: #18674