Skip to content
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

Roachtest Repro #69646

Closed
wants to merge 7 commits into from
Closed

Roachtest Repro #69646

wants to merge 7 commits into from

Conversation

AlexTalks
Copy link
Contributor

Repro working on #69414

Release justification: n/a
Release note (<category, see below>): n/a
@AlexTalks AlexTalks added the do-not-merge bors won't merge a PR with this label. label Aug 31, 2021
@cockroach-teamcity
Copy link
Member

This change is Reviewable

AlexTalks and others added 5 commits September 1, 2021 11:19
Previously the CLI debug command only printed `MVCCKey`s, resulting in
the debug printer to error on key decoding whenever a `LockTableKey` was
encountered.  By switching to the more generalized `EngineKey` (and
utilizing the existing MVCC key formatting whenever the key has an MVCC
version), we can increase visibility into our debug logs while testing.

Release justification: Non-production bug fix
Release note: None
This commit changes the formatting used when printing intents via the
CLI debug command from the default generated Protobuf formatter to our
custom `MVCCMetadata` formatter implementation.  Additionally, the
`MergeTimestamp` and `TxnDidNotUpdateMetadata` fields have been added to
the output.  This changes the debug formatting from the former
example:
```
0,0 /Local/RangeID/203/r/RangePriorReadSummary (0x0169f6cb727270727300): {Txn:<nil> Timestamp:0,0 Deleted:false KeyBytes:0 ValBytes:0 RawBytes:[230 123 85 161 3 10 12 10 10 8 146 229 195 204 139 135 186 208 22 18 12 10 10 8 146 229 195 204 139 135 186 208 22] IntentHistory:[] Me
rgeTimestamp:<nil> TxnDidNotUpdateMeta:<nil>}
/Local/Lock/Intent/Table/56/1/1319/6/3055/0 0361fea07d3f0d40ba8f44dc4ee46cbdc2 (0x017a6b12c089f705278ef70bef880001000361fea07d3f0d40ba8f44dc4ee46cbdc212): 1630559399.176502568,0 {Txn:id=61fea07d key=/Table/57/1/1319/6/0 pri=0.01718258 epo=0 ts=1630559399.176502568,0 min=1630559399.176502568,0 seq=4 Timestamp:1630559399.176502568,0 Deleted:false KeyBytes:12 ValBytes:5 RawBytes:[] IntentHistory:[] MergeTimestamp:<nil> TxnDidNotUpdateMeta:0xc0016059b0}
```
to the following example:
```
0,0 /Local/RangeID/203/r/RangePriorReadSummary (0x0169f6cb727270727300): txn={<nil>} ts=0,0 del=false klen=0 vlen=0 raw=/BYTES/0x0a0c0a0a0892e5c3cc8b87bad016120c0a0a0892e5c3cc8b87bad016 mergeTs=<nil> txnDidNotUpdateMeta=false
/Local/Lock/Intent/Table/56/1/1319/6/3055/0 0361fea07d3f0d40ba8f44dc4ee46cbdc2 (0x017a6b12c089f705278ef70bef880001000361fea07d3f0d40ba8f44dc4ee46cbdc212): 1630559399.176502568,0 txn={id=61fea07d key=/Table/57/1/1319/6/0 pri=0.01718258 epo=0 ts=1630559399.176502568,0 min=1630559399.176502568,0 seq=4} ts=1630559399.176502568,0 del=false klen=12 vlen=5 mergeTs=<nil> txnDidNotUpdateMeta=true
```

Release justification: Bug fix
Release note: None
They are affected by formatting changes in preceding commits.
Fixes cockroachdb#69414.

Only use SingleDel if

- the meta tracking allows it (i.e. the sole previous condition), and
- epoch zero, and
- no ignored txn seqno ranges.

These three together imply that the engine history of the metadata key
is a single `Set`, so we can safely use `SingleDel`.

This is a below-Raft change, but no mixed-version problems are expected.
This is because using a Del vs using a SingleDel (if it does not cause
an anomaly) is not visible at the replica state level. In particular,
a deleted key looks the same to the consistency checker as a
single-deleted key.

Release justification: fix for a release blocker
Release note: None
@AlexTalks AlexTalks closed this Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge bors won't merge a PR with this label.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants