kv/kvserver: use proper formatting when debug printing intents#69809
kv/kvserver: use proper formatting when debug printing intents#69809craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
|
Thanks Alex! CI is red, I think we want an update to |
tbg
left a comment
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @erikgrinaker and @sumeerbhola)
sumeerbhola
left a comment
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status:
pkg/storage/enginepb/mvcc.go, line 323 at r1 (raw file):
txnDidNotUpdateMeta = *meta.TxnDidNotUpdateMeta } fmt.Fprintf(buf, " mergeTs=%s txnDidNotUpdateMeta=%t", meta.MergeTimestamp, txnDidNotUpdateMeta)
will this print the LegacyTimestamp contents if MergeTimestamp is non-nil?
AlexTalks
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/storage/enginepb/mvcc.go, line 323 at r1 (raw file):
Previously, sumeerbhola wrote…
will this print the
LegacyTimestampcontents ifMergeTimestampis non-nil?
Both the Timestamp and the MergeTimestamp fields are of type util.hlc.LegacyTimestamp, did you mean the first one? If so, that's already printed out, yes.
035f328 to
a94f82f
Compare
AlexTalks
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/storage/enginepb/mvcc.go, line 323 at r1 (raw file):
Previously, AlexTalks (Alex Sarkesian) wrote…
Both the
Timestampand theMergeTimestampfields are of typeutil.hlc.LegacyTimestamp, did you mean the first one? If so, that's already printed out, yes.
I'm not sure in what cases MergeTimestamp is non-nil, but if it's unnecessary in the output I can easily do one of:
- skip printing if nil,
- only print if
expand=true(I.e. if printed likefmt.Printf("%+v", meta)) - or just leave it as is
knz
left a comment
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1.
Reviewable status:
pkg/storage/enginepb/mvcc.go, line 279 at r2 (raw file):
// String implements the fmt.Stringer interface. func (meta *MVCCMetadata) String() string {
FWIW, this code and the methods below ought to be replaced by a SafeFormat method and associated wrappers.
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
a94f82f to
a278eee
Compare
|
bors r+ |
|
Build succeeded: |
This commit changes the formatting used when printing intents via the
CLI debug command from the default generated Protobuf formatter to our
custom
MVCCMetadataformatter implementation. Additionally, theMergeTimestampandTxnDidNotUpdateMetadatafields have been added tothe output. This changes the debug formatting from the former
example:
to the following example:
Related to #69414
Release justification: Bug fix
Release note: None