storage/engine: don't decrement IntentCount on system intent abort#39427
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom Aug 8, 2019
Merged
storage/engine: don't decrement IntentCount on system intent abort#39427craig[bot] merged 1 commit intocockroachdb:masterfrom
craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
Fixes cockroachdb#39390. Fixes cockroachdb#39407. Fixes cockroachdb#39370. Fixes cockroachdb#39419. A bug introduced in cockroachdb#39251 was causing the IntentCount field in MVCCStats to get out of sync. I was expecting `TestMVCCStatsRandomized/sys/inline=false` to easily catch the bug, but it wasn't. It turns out that the test was updating the transaction's write timestamp but not its read timestamp, which was causing it to hit this error: https://github.com/cockroachdb/cockroach/blob/41cb28e85575292926790e79912c893d13e1054d/pkg/storage/engine/mvcc.go#L1349 This was preventing the randomized test from performing the series of steps we needed it to to hit the bug. This commit fixes the bug, fixes the randomized test so that it would have caught it, and adds a targeted regression test for this case. Release note: None
Member
dt
approved these changes
Aug 8, 2019
Contributor
Author
|
bors r+ |
craig bot
pushed a commit
that referenced
this pull request
Aug 8, 2019
39206: sql: add RevertTable wrapper for RevertRangeRequest r=dt a=dt (first commit is #39251) kv,batcheval: fix RevertRange to work with DistSender and ResumeSpans This patch includes several changes required to make RevertRange requests get along with DistSender: 1) add RevertRange to the explicit check for maxkey-supported requests. 2) makes RevertRangeResponse combinable, however combining assumes that at-most-one reply returned a ResumeSpan, so it also 3) sets NumKeys to MaxKeys in RevertRange when returning a ResumeSpan, to stop distsender from sending any further requests that might also hit a resume and then trip up combining. Together these make it possible to send batches of RevertRange requests via DistSender and get back resume spans. Release note: none. sql: add RevertTables helper for reverting a Table via RevertRange This adds a helper that can revert a collection of tables to a target timestamp using the RevertRange RPC. It checks that the tables do not overlap the key space of another table not being reverted (i.e. that all the reverting tables are interleaved, if at all, only with each other) and that the tables are offline. It then reverts the table spans using RevertRange, including handling the ResumeSpan pagination. Release note: none. 39427: storage/engine: don't decrement IntentCount on system intent abort r=nvanbenschoten a=nvanbenschoten Fixes #39390. Fixes #39407. Fixes #39370. Fixes #39419. A bug introduced in #39251 was causing the IntentCount field in MVCCStats to get out of sync. I was expecting `TestMVCCStatsRandomized/sys/inline=false` to easily catch the bug, but it wasn't. It turns out that the test was updating the transaction's write timestamp but not its read timestamp, which was causing it to hit this error: https://github.com/cockroachdb/cockroach/blob/41cb28e85575292926790e79912c893d13e1054d/pkg/storage/engine/mvcc.go#L1349 This was preventing the randomized test from performing the series of steps we needed it to hit the bug. This commit fixes the bug, fixes the randomized test so that it would have caught it, and adds a targeted regression test for this case. Release note: None Co-authored-by: David Taylor <tinystatemachine@gmail.com> Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Contributor
Build succeeded |
This was referenced Aug 28, 2019
This file contains hidden or 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
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.
Fixes #39390.
Fixes #39407.
Fixes #39370.
Fixes #39419.
A bug introduced in #39251 was causing the IntentCount field in MVCCStats to get out of sync.
I was expecting
TestMVCCStatsRandomized/sys/inline=falseto easily catch the bug, but it wasn't. It turns out that the test was updating the transaction's write timestamp but not its read timestamp, which was causing it to hit this error:cockroach/pkg/storage/engine/mvcc.go
Line 1349 in 41cb28e
This was preventing the randomized test from performing the series of steps we needed it to hit the bug.
This commit fixes the bug, fixes the randomized test so that it would have caught it, and adds a targeted regression test for this case.
Release note: None