Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
63309: cliccl/debug_backup.go: refactor backup inspection tool r=pbardea a=Elliebababa
Previously, cli backup inspection tool was incorporated
in cliccl/load.go. Both the command and syntax are
confusing in terms of backup inspection.
This patch refactors the naming and syntax of the tool
and hang the tool under `cockroach debug`, so that
it makes more sense when using it to debug backups.
Release note (cli change): Backup inspection used to be done via
`cockroach load show ..`, which was confusing to users with
ambiguous verbs in the command chain. We refactor the syntax of
tool and make the syntax more clear and indicative for users to
debug backups. The changes are:
`load show summary <backup_url>` ->
`debug backup show <backup_url>`
```
$ ./cockroach debug backup show $AWS_BACKUP_PATH
{
"StartTime": "1970-01-01T00:00:00Z",
"EndTime": "2021-04-05T22:02:25Z",
"DataSize": "110 KiB",
"Rows": 224,
"IndexEntries": 324,
"FormatVersion": 1,
"ClusterID": "7c1b9ceb-d05a-4879-b4a4-886ace456953",
"NodeID": 0,
"BuildInfo": "CockroachDB CCL v21.1.0-alpha.3-1790-g25dc628337-dirty (x86_64-apple-darwin19.6.0, built 2021/03/29 18:50:47, go1.15.7)",
"Files": [
{
"Path": "647462079350243329.sst",
"Span": "/Table/4/{1-2}",
"DataSize": "99 B",
"IndexEntries": 0,
"Rows": 2
},
],
"Spans": "[/Table/4/{1-2}]",
"DatabaseDescriptors": {
"1": "system"
},
"TableDescriptors": {
"53": "demodb.public.demo",
"54": "demodb.public.foo"
},
"TypeDescriptors": {},
"SchemaDescriptors": {
"29": "public"
}
}
```
`load show incremental <backup_url>` ->
`debug backup list-incremental <backup_url>`
```
$ ./cockroach debug backup list-incremental $AWS_BACKUP_PATH
path | start time | end time
-------------------------------+----------------------+-----------------------
/mybackup | - | 2021-04-09T20:22:59Z
/mybackup/20210409/202303.20 | 2021-04-09T20:22:59Z | 2021-04-09T20:23:03Z
/mybackup/20210409/202305.97 | 2021-04-09T20:23:03Z | 2021-04-09T20:23:05Z
(3 rows)
```
`load show backups <collection_url>` ->
`debug backup list-backups <collection_url>`
```
$ ./cockroach debug backup list-backups $AWS_COLLECTION_PATH
path
--------------------------
./2021/04/09-202455.98
./2021/04/09-202522.59
./2021/04/09-202526.93
(3 rows)
```
`load show data <table_name> <backup_url> `->
`debug backup export <backup_url> --table=<table_name> [--destination=</path/to/export>]`
```
$ ./cockroach debug backup export $AWS_BACKUP_PATH --table=demodb.public.demo
1,null,true
2,'ellie',null
```
64028: kv: don't clear raftRequestQueue of right-hand side of Range split r=nvanbenschoten a=nvanbenschoten
This commit fixes a test flake of `TestLeaderAfterSplit` I observed in CI and
which we've seen at least once in #43564 (comment).
I bisected the flake back to a591707, but that wasn't the real source of the
flakiness - the move from `multiTestContext` to `TestCluster` just changed
transport mechanism between replicas and revealed an existing bug.
The real issue here was that, upon applying a split, any previously established
`raftRequestQueue` to the RHS replica was discarded. The effect of this is that
we could see the following series of events:
```
1. r1 is created from a split
2. r1 campaigns to establish a leader for the new range
3. r1 sends MsgPreVote msgs to r2 and r3
4. s2 and s3 both receive the messages for the uninitialized r2 and r3, respectively.
5. raftRequestQueues are established for r2 and r3, and the MsgPreVotes are added
6. the split triggers to create r2 and r3 finally fire
7. the raftRequestQueues for r2 and r3 are discarded
8. the election stalls indefinitely, because the test sets RaftElectionTimeoutTicks=1000000
```
Of course, in real deployments, `RaftElectionTimeoutTicks` will never be set so
high, so a new election will be called again after about 3 seconds. Still, this
could cause unavailability immediately after a split for about 3s even in real
deployments, so it seems worthwhile to fix.
This commit fixes the issue by removing the logic to discard an uninitialized
replica's `raftRequestQueue` upon applying a split that initializes the replica.
That logic looks quite intentional, but if we look back at when it was added, we
see that it wasn't entirely deliberate. The code was added in d3b0e73, which
extracted everything except the call to `s.mu.replicas.Delete(int64(rangeID))`
from `unlinkReplicaByRangeIDLocked`. So the change wasn't intentionally
discarding the queue, it was just trying not to change the existing behavior.
This change is safe and does not risk leaking the `raftRequestQueue` because
we are removing from `s.mu.uninitReplicas` but will immediately call into
`addReplicaInternalLocked` to add an initialized replica.
Release notes (bug fix): Fix a rare race that could lead to a 3 second stall
before a Raft leader was elected on a Range immediately after it was split off
from its left-hand neighbor.
64032: kv: don't allow node liveness to regress in Gossip network r=nvanbenschoten a=nvanbenschoten
In #64028, we fixed a long-standing flake in `TestLeaderAfterSplit`. However,
the test had actually gotten more flaky recently, which I bisected back to
df826cd. The problem we occasionally see with the test is that all three
replicas of a post-split Range call an election, resulting in a hung vote. Since
the test is configured with RaftElectionTimeoutTicks=1000000, a follow-up
election is never called, so the test times out.
After some debugging, I found that the range would occasionally split while the
non-leaseholder nodes (n2 and n3) thought that the leaseholder node (n1) was not
live. This meant that their call to `shouldCampaignOnWake` in the split trigger
considered the RHS's epoch-based lease to be invalid (state = ERROR). So all
three replicas would call an election and the test would get stuck.
The offending commit introduced this new flake because of this change:
df826cd#diff-488a090afc4b6eaf56cd6d13b347bac67cb3313ce11c49df9ee8cd95fd73b3e8R454
Now that the call to `MaybeGossipNodeLiveness` is asynchronous on the
node-liveness range, it was possible for two calls to `MaybeGossipNodeLiveness`
to race, one asynchronously triggered by `leasePostApplyLocked` and one
synchronously triggered by `handleReadWriteLocalEvalResult` due to a node
liveness update. This allowed for the following ordering of events:
```
- async call reads liveness(nid:1 epo:0 exp:0,0)
- sync call writes and then reads liveness(nid:1 epo:1 exp:1619645671.921265300,0)
- sync call adds liveness(nid:1 epo:1 exp:1619645671.921265300,0) to gossip
- async call adds liveness(nid:1 epo:0 exp:0,0) to gossip
```
One this had occurred, n2 and n3 never again considered n1 live. Gossip never
recovered from this state because the liveness record was never heartbeated
again, due to the test's configuration of `RaftElectionTimeoutTicks=1000000`.
This commit fixes the bug by ensuring that all calls to MaybeGossipNodeLiveness
and MaybeGossipSystemConfig hold the raft mutex. This provides the necessary
serialization to avoid data races, which was actually already documented on
MaybeGossipSystemConfig.
64070: build: retry register.cockrachdb.com requests r=rail a=stevendanna
Retry transient problems getting a developer license. We may want to
avoid the retries here as a forcing function to make that service more
reliable, but given that we might see timeouts from non-service
related local networking problems, I'd prefer to not fail the test run
because of a single curl failure.
Release note: None
Co-authored-by: elliebababa <ellie24.huang@gmail.com>
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Co-authored-by: Steven Danna <danna@cockroachlabs.com>- Loading branch information
