batcheval: add setting to reject non-MVCC AddSSTable
#76934
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
|
I'm expecting this to break all sorts of tests, but I'll let CI find out which. |
|
I think CTAS and materialized views are for sure going to beak since I didn’t change their flag to bulk adder yet. I wonder if we should just change the default in bulk adder options. |
erikgrinaker
force-pushed
the
addsstable-reject-nonmvcc
branch
from
43d1fce to
c1e0850
Compare
I'm starting to wonder if this whole thing is even a good idea. Callers can't use the new |
erikgrinaker
force-pushed
the
addsstable-reject-nonmvcc
branch
from
c1e0850 to
6f69250
Compare
erikgrinaker
force-pushed
the
addsstable-reject-nonmvcc
branch
from
6f69250 to
5b1db19
Compare
erikgrinaker
changed the title
AddSSTable without SSTTimestampToRequestTimestampAddSSTable
|
I've updated this to be a default-off setting as we discussed, PTAL. |
This patch adds a cluster setting `kv.bulk_io_write.sst_require_at_request_timestamp.enabled` (default `false`), which will reject all non-MVCC `AddSSTable` requests (i.e. without `SSTTimestampToRequestTimestamp` set). This can be used to verify that all callers send MVCC-compliant `AddSSTable` requests, for example when testing streaming replication or in preparation for 22.2. It is not enabled by default, since that would require multiple migrations to ensure all callers were in fact using `SSTTimestampToRequestTimestamp` (which is new in this release), and would prevent falling back to the old index backfiller in case bugs are discovered. Release note: None
erikgrinaker
force-pushed
the
addsstable-reject-nonmvcc
branch
from
5b1db19 to
3c557ca
Compare
dt
approved these changes
Mar 21, 2022
|
bors r=dt |
|
bors r=dt |
|
Build failed (retrying...): |
craig bot
pushed a commit
that referenced
this pull request
Mar 21, 2022
76934: batcheval: add setting to reject non-MVCC `AddSSTable` r=dt a=erikgrinaker This patch adds a cluster setting `kv.bulk_io_write.sst_require_at_request_timestamp.enabled` (default `false`), which will reject all non-MVCC `AddSSTable` requests (i.e. without `SSTTimestampToRequestTimestamp` set). This can be used to verify that all callers send MVCC-compliant `AddSSTable` requests, for example when testing streaming replication or in preparation for 22.2. It is not enabled by default, since that would require multiple migrations to ensure all callers were in fact using `SSTTimestampToRequestTimestamp` (which is new in this release), and would prevent falling back to the old index backfiller in case bugs are discovered. Release note: None 77965: ccl/sqlproxyccl: rename RequestTransfer to TransferConnection, and make it sync r=JeffSwenson a=jaylim-crl Fixes a test flake in #77909. This commit fixes a test flake as described in the issue above. At the same time, we rename RequestTransfer to TransferConnection, and make the API synchronous instead. The transferer should invoke TransferConnection within a goroutine. Release justification: sqlproxy only change. Transfer API isn't used anywhere besides tests as well. Release note: None 78094: ui/tracez: load new snapshots automatically r=andreimatei a=andreimatei Before this patch taking a new snapshot had no apparent visual feedback. This patch makes it so that the new snapshot is loaded automatically. Release note: None 78196: ci: upgrade to Ubuntu 20.04 for TC agents r=rail a=rickystewart Release note: None 78198: release: use REL Jira project to track releases r=rail a=rail Previously, we used RE Jira project to track releases. The REL project was intended to be used from the beginning, but it was missing some configuration tweaks. This patch changes the Jira project and removes the in-line release checklist in favour of Jira checklist. Release note: None 78201: Revert "kvstreamer: re-enable streamer by default" r=yuzefovich a=yuzefovich This reverts commit b72c109. Informs: #78145. Co-authored-by: Erik Grinaker <[email protected]> Co-authored-by: Jay <[email protected]> Co-authored-by: Andrei Matei <[email protected]> Co-authored-by: Ricky Stewart <[email protected]> Co-authored-by: Rail Aliiev <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]>
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds a cluster setting
kv.bulk_io_write.sst_require_at_request_timestamp.enabled(defaultfalse), which will reject all non-MVCCAddSSTablerequests (i.e.without
SSTTimestampToRequestTimestampset).This can be used to verify that all callers send MVCC-compliant
AddSSTablerequests, for example when testing streaming replication orin preparation for 22.2. It is not enabled by default, since that would
require multiple migrations to ensure all callers were in fact using
SSTTimestampToRequestTimestamp(which is new in this release), andwould prevent falling back to the old index backfiller in case bugs are
discovered.
Release note: None