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.

Sign up for GitHub

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

Jump to bottom

backupccl: redact backup statement in backup schedule logger #99145

Closed
msbutler opened this issue Mar 21, 2023 · 2 comments
Closed

backupccl: redact backup statement in backup schedule logger #99145

msbutler opened this issue Mar 21, 2023 · 2 comments
Labels
A-disaster-recovery C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-disaster-recovery

Comments

@msbutler
Copy link
Collaborator

msbutler commented Mar 21, 2023 •
edited
Loading

We currently log the whole backup statement as a raw string in the backup schedule executor here, causing unsanitized uris and secret keys to appear in the unredacted logs. We ought to scrub this info. This bug was discovered in this support issue https://github.com/cockroachlabs/support/issues/2170

Jira issue: CRDB-25725
@msbutler msbutler added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. T-disaster-recovery branch-release-22.1 Used to mark GA and release blockers, technical advisories, and bugs for 22.1 labels Mar 21, 2023
@msbutler msbutler self-assigned this Mar 21, 2023
@blathers-crl
Copy link

blathers-crl bot commented Mar 21, 2023

cc @cockroachdb/disaster-recovery

@msbutler msbutler mentioned this issue Mar 21, 2023
Merged
msbutler added a commit to msbutler/cockroach that referenced this issue Mar 21, 2023
@msbutler
backupccl: stop logging unsanitized backup stmt in schedule executor
7a92f58 
Informs cockroachdb#99145

Release note: None
msbutler added a commit to msbutler/cockroach that referenced this issue Mar 21, 2023
@msbutler
backupccl: stop logging unsanitized backup stmt in schedule executor
46e82d0 
Informs cockroachdb#99145

Release note: None
@msbutler msbutler mentioned this issue Mar 21, 2023
Merged
@msbutler msbutler removed release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. branch-release-22.1 Used to mark GA and release blockers, technical advisories, and bugs for 22.1 labels Mar 22, 2023
@msbutler msbutler mentioned this issue Mar 22, 2023
Merged
msbutler added a commit to msbutler/cockroach that referenced this issue Mar 22, 2023
@msbutler
backupccl: scrub backup stmt from return errors
b6d2a86 
This patch ensures that no unsanitized uris or secret keys get written to the
jobs table if the backup fails.

Informs cockroachdb#99145

Release note: None
msbutler added a commit to msbutler/cockroach that referenced this issue Mar 22, 2023
@msbutler
backupccl: scrub backup stmt from return errors
97243f6 
This patch ensures that no unsanitized uris or secret keys get written to the
jobs table if the backup fails.

Informs cockroachdb#99145

Release note: None
@msbutler msbutler mentioned this issue Mar 22, 2023
Open
craig bot pushed a commit that referenced this issue Mar 22, 2023
@cockroach-teamcity @adityamaru
@tbg @miraradeva @msbutler
Merge #98741 #98878 #99099 #99126 #99150
c2460f1 
98741: ci: update bazel builder image r=rickystewart a=cockroach-teamcity

Release note: None
Epic: None


98878: backupccl: fix occassional TestRestoreErrorPropagates flake r=stevendanna a=adityamaru

Very rarely under stress race another automatic job would race with the restore and increment the error count. This would result in the count being greater than our expected value of 1. This disables all the automatic jobs eliminating the chance of this race.

Fixes: #98037

Release note: None

99099: kvserver: deflake TestReplicaTombstone r=andrewbaptist a=tbg

Like many other tests, this test could flake because we'd sometimes
catch a "cannot remove learner while snapshot is in flight" error.

I think the root cause is that sometimes there are errant Raft snapshots
in the system[^1] and these get mistaken for LEARNERs that are still
being caught up by the replicate queue. I tried to address this general
class of issues by making the check for in-flight learner snapshots not
care about *raft* snapshots.

I was able to stress TestReplicaTombstone for 30+ minutes without a
failure using that approach, whereas previously it usually failed within
a few minutes.

```
./dev test --stress pkg/kv/kvserver/ --filter TestReplicaTombstone 2>&1 | tee stress.log
[...]
2461 runs so far, 0 failures, over 35m45s
```

[^1]: #87553

Fixes #98883.

Epic: none
Release note: None


99126: kv: return error on locking request in LeafTxn r=nvanbenschoten a=miraradeva

Previously, as noted in #94290, it was possible for a LeafTxn to issue locking requests as part of SELECT FOR UPDATE. This behavior was unexpected and the RootTxn wasn't properly cleaning up the locks, resulting in others waiting for those locks to be released. The issue was resolved, in #94399, by ensuring non-default locking strength transactions don't use the streamer API and always run as RootTxn.

This patch adds an assertion on the kv side to prevent other existing or future attempts of LeafTxn issuing locking requests. We don't expect that there are such existing cases, so we don't expect this assertion to fail, but will keep an eye on the nightly tests to make sure.

Fixes: #97817
Release note: None

99150: backupccl: stop logging unsanitized backup stmt in schedule executor r=stevendanna a=msbutler

Informs #99145

Release note: None

Co-authored-by: cockroach-teamcity <[email protected]>
Co-authored-by: adityamaru <[email protected]>
Co-authored-by: Tobias Grieger <[email protected]>
Co-authored-by: Mira Radeva <[email protected]>
Co-authored-by: Michael Butler <[email protected]>
blathers-crl bot pushed a commit that referenced this issue Mar 22, 2023
@msbutler
backupccl: stop logging unsanitized backup stmt in schedule executor
ad47ce5 
Informs #99145

Release note: None
@blathers-crl blathers-crl bot mentioned this issue Mar 22, 2023
Closed
blathers-crl bot pushed a commit that referenced this issue Mar 22, 2023
@msbutler
backupccl: scrub backup stmt from return errors
8fc0a0a 
This patch ensures that no unsanitized uris or secret keys get written to the
jobs table if the backup fails.

Informs #99145

Release note: None
blathers-crl bot pushed a commit that referenced this issue Mar 22, 2023
@msbutler
backupccl: stop logging unsanitized backup stmt in schedule executor
d0af5f3 
Informs #99145

Release note: None
@blathers-crl blathers-crl bot mentioned this issue Mar 22, 2023
Merged
blathers-crl bot pushed a commit that referenced this issue Mar 22, 2023
@msbutler
backupccl: scrub backup stmt from return errors
f6613a4 
This patch ensures that no unsanitized uris or secret keys get written to the
jobs table if the backup fails.

Informs #99145

Release note: None
blathers-crl bot pushed a commit that referenced this issue Mar 22, 2023
@msbutler
backupccl: stop logging unsanitized backup stmt in schedule executor
9afda26 
Informs #99145

Release note: None
@blathers-crl blathers-crl bot mentioned this issue Mar 22, 2023
Merged
blathers-crl bot pushed a commit that referenced this issue Mar 22, 2023
@msbutler
backupccl: scrub backup stmt from return errors
44e66d0 
This patch ensures that no unsanitized uris or secret keys get written to the
jobs table if the backup fails.

Informs #99145

Release note: None
@msbutler
Copy link
Collaborator Author

all release branch prs have merged

@msbutler msbutler removed their assignment Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-disaster-recovery C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-disaster-recovery
Projects
No open projects
Disaster Recovery Backlog
Archived in project
Milestone
No milestone
Development

No branches or pull requests
1 participant
@msbutler