storage: return TransactionRetryError from QueryIntent on pushed intent #27307
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained
pkg/roachpb/api.proto, line 773 at r1 (raw file):
// response. DO_NOTHING = 0; // Return an IntentMissingError.
Stale
pkg/storage/replica_test.go, line 5852 at r1 (raw file):
} for _, baTxn := range []*roachpb.Transaction{nil, txn} {
Throw another txn in here that's not the own txn? I don't think we'd ever call this command in that case, but it would still be useful to validate that the code is correct. Or, even better, error out in that case and verify that.
There was a problem hiding this comment.
Reviewable status:
This change adjusts how QueryIntent handles pushed intents. First, it changes how pushed intents interact with SNAPSHOT transactions. Next, it makes sure to update the response transaction in the case of pushed intents. Finally, it returns a RETRY_SERIALIZABLE TransactionRetryError for SERIALIZABLE transactions who observe a pushed intent with the RETURN_ERROR behavior. Release note: None
nvanbenschoten
force-pushed
the
nvanbenschoten/queryIntentSnapshot
branch
from
59fb4fe to
6dad7b6
Compare
There was a problem hiding this comment.
TFTR!
Reviewable status:
pkg/roachpb/api.proto, line 773 at r1 (raw file):
Previously, tschottdorf (Tobias Schottdorf) wrote…
Stale
Done.
pkg/storage/replica_test.go, line 5852 at r1 (raw file):
Previously, tschottdorf (Tobias Schottdorf) wrote…
Throw another txn in here that's not the own txn? I don't think we'd ever call this command in that case, but it would still be useful to validate that the code is correct. Or, even better, error out in that case and verify that.
Done.
There was a problem hiding this comment.
Reviewed 1 of 4 files at r1, 3 of 3 files at r2.
Reviewable status:
pkg/storage/replica_test.go, line 5794 at r2 (raw file):
defer leaktest.AfterTest(t)() for _, iso := range []enginepb.IsolationType{enginepb.SNAPSHOT, enginepb.SERIALIZABLE} {
Note that snapshot isolation will be inaccessible in 2.1, so we don't really need to add new tests or behavior for it (except to the extent necessary to prevent things from breaking during the rollout of 2.1)
|
bors r+ |
27262: ui: require yarn >= 1.7.0 r=benesch,vilterp a=tschottdorf Builds before the upgrade failed mysteriously. @benesch (cc) suspects a bug in older versions of yarn. ``` $ make build Running make with -j8 GOPATH set to /Users/tschottdorf/go build/node-run.sh -C pkg/ui yarn install --offline (echo "// Code generated by goyacc. DO NOT EDIT."; \ echo "// GENERATED FILE DO NOT EDIT"; \ cat pkg/sql/parser/gen/sql.go.tmp | \ sed -E 's/^const ([A-Z][_A-Z0-9]*) =.*$/const \1 = lex.\1/g') > pkg/sql/parser/sql.go.tmp || rm pkg/sql/parser/sql.go.tmp (echo "// Code generated by make. DO NOT EDIT."; \ echo "// GENERATED FILE DO NOT EDIT"; \ echo; \ echo "package lex"; \ echo; \ grep '^const [A-Z][_A-Z0-9]* ' pkg/sql/parser/gen/sql.go.tmp) > pkg/sql/lex/tokens.go.tmp || rm pkg/sql/lex/tokens.go.tmp mv -f pkg/sql/lex/tokens.go.tmp pkg/sql/lex/tokens.go mv -f pkg/sql/parser/sql.go.tmp pkg/sql/parser/sql.go yarn install v1.6.0 (node:50064) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead. [1/5] Validating package.json... go install -v docgen bin/prereqs ./pkg/cmd/docgen > bin/docgen.d.tmp [2/5] Resolving packages... mv -f bin/docgen.d.tmp bin/docgen.d [3/5] Fetching packages... build/node-run.sh pkg/ui/bin/gen-protobuf-cli-deps.js > pkg/ui/node_modules/protobufjs/cli/package.json bash: pkg/ui/node_modules/protobufjs/cli/package.json: No such file or directory make: *** [pkg/ui/yarn.installed] Error 1 make: *** Waiting for unfinished jobs.... ``` Release note (general change): the build now requires yarn at version 1.7.0 or above, to work around yarnpkg/yarn#5761. 27307: storage: return TransactionRetryError from QueryIntent on pushed intent r=nvanbenschoten a=nvanbenschoten Extracted from #26599. This change adjusts how QueryIntent handles pushed intents. First, it changes how pushed intents interact with SNAPSHOT transactions. Next, it makes sure to update the response transaction in the case of pushed intents. Finally, it returns a RETRY_SERIALIZABLE TransactionRetryError for SERIALIZABLE transactions who observe a pushed intent with the RETURN_ERROR behavior. Release note: None 27329: log: use fake stacktrace when test-exiting r=benesch a=tschottdorf Having stack traces littered across tests has been a long-standing pet peeve of mine (and presumably others). We search through logs more often than we would like to admit, and having to skip a number of intentional stack traces is annoying. Make it so that whenever the logging exit func is mocked, the stack trace indicates that. Release note: None 27348: engine: fix RocksDBBatchReader example comment r=nvanbenschoten a=nvanbenschoten Release note: None Co-authored-by: Tobias Schottdorf <[email protected]> Co-authored-by: Nathan VanBenschoten <[email protected]>
Build succeeded |
Extracted from #26599.
This change adjusts how QueryIntent handles pushed intents. First, it
changes how pushed intents interact with SNAPSHOT transactions. Next,
it makes sure to update the response transaction in the case of pushed
intents. Finally, it returns a RETRY_SERIALIZABLE TransactionRetryError
for SERIALIZABLE transactions who observe a pushed intent with the
RETURN_ERROR behavior.
Release note: None