storage: re-enable the merge queue by default

[benesch]

This reverts commit a1cc4c5.

The results from last night's stress run on were extremely promising. All the failures were existing flaky tests. You can see for yourself here:

https://teamcity.cockroachdb.com/viewType.html?buildTypeId=Cockroach_Nightlies_Stress&tab=buildTypeStatusDiv&branch_Cockroach_Nightlies=refs%2Fheads%2Fmerge-default&state=failed

[cockroach-teamcity]

This change is 

[cockroach-teamcity]

This change is 

[benesch]

The results from last night's stress run on were extremely promising. All the failures were existing flaky tests. You can see for yourself here:

Scratch that. There is a longstanding bug in our stress test configuration which results in them always getting run against master. Thanks, TeamCity.

[benesch]

The results from last night's stress run on were extremely promising. All the failures were existing flaky tests. You can see for yourself here:

Scratch that. There is a longstanding bug in our stress test configuration which results in them always getting run against master. Thanks, TeamCity.

[benesch]

Ok, stress build triggered for real: https://teamcity.cockroachdb.com/viewType.html?buildTypeId=Cockroach_Nightlies_Stress&tab=buildTypeHistoryList&branch_Cockroach_Nightlies=merge-default&state=failed

[benesch]

Ok, stress build triggered for real: https://teamcity.cockroachdb.com/viewType.html?buildTypeId=Cockroach_Nightlies_Stress&tab=buildTypeHistoryList&branch_Cockroach_Nightlies=merge-default&state=failed

[benesch]

Alright. There are only two stress failures that are not addressed in this PR.

The first failure is #31062, which is a scary consistency failure. I've run that test 100+ times with roachprod-stress (it's really slow; 100 runs took 45m) and haven't been able to reproduce. It's entirely possible that the bug was fixed by #30986, which wasn't included in the build that I triggered the stress run on.

The second stress failure is #31059. This is an ambiguous result caused by a replica removal. I don't think this is related to merges, actually. I'm willing to bet that merges exacerbate this issue—they cause a lot of replica removals—but whatever the fix is here, it's going to be tricky. I'm inclined to leave this one alone for now. Like #31062, I haven't been able to reproduce it, even with roachprod-stress.

Any objections to merging as-is? We're coming down to the wire. Introducing two rarely-flaky tests in exchange for getting merges on by default seems worth it to me. /cc @bdarnell @tschottdorf

[benesch]

Alright. There are only two stress failures that are not addressed in this PR.

The first failure is #31062, which is a scary consistency failure. I've run that test 100+ times with roachprod-stress (it's really slow; 100 runs took 45m) and haven't been able to reproduce. It's entirely possible that the bug was fixed by #30986, which wasn't included in the build that I triggered the stress run on.

The second stress failure is #31059. This is an ambiguous result caused by a replica removal. I don't think this is related to merges, actually. I'm willing to bet that merges exacerbate this issue—they cause a lot of replica removals—but whatever the fix is here, it's going to be tricky. I'm inclined to leave this one alone for now. Like #31062, I haven't been able to reproduce it, even with roachprod-stress.

Any objections to merging as-is? We're coming down to the wire. Introducing two rarely-flaky tests in exchange for getting merges on by default seems worth it to me. /cc @bdarnell @tschottdorf

[tschottdorf]

The first failure is #31062, which is a scary consistency failure

Have you tried to repro on the same sha? If you manage that and don't manage right after #30986 lands, this looks fine.

Any objections to merging as-is? We're coming down to the wire. Introducing two rarely-flaky tests in exchange for getting merges on by default seems worth it to me. /cc @bdarnell @tschottdorf

I'd like to figure the consistency failure out, but the other one seems OK to deal with in isolation.

[tschottdorf]

The first failure is #31062, which is a scary consistency failure

Have you tried to repro on the same sha? If you manage that and don't manage right after #30986 lands, this looks fine.

Any objections to merging as-is? We're coming down to the wire. Introducing two rarely-flaky tests in exchange for getting merges on by default seems worth it to me. /cc @bdarnell @tschottdorf

I'd like to figure the consistency failure out, but the other one seems OK to deal with in isolation.

[benesch]

Have you tried to repro on the same sha? If you manage that and don't manage right after #30986 lands, this looks fine.

Welp, yep, the consistency failure repro'd straight away without #30986. I'll try to repro once more for good measure, but feeling pretty good that the bug fixed in #30986 was actually to blame.

[benesch]

Have you tried to repro on the same sha? If you manage that and don't manage right after #30986 lands, this looks fine.

Welp, yep, the consistency failure repro'd straight away without #30986. I'll try to repro once more for good measure, but feeling pretty good that the bug fixed in #30986 was actually to blame.

[benesch]

Yep, I ran this twice more without #30986 and twice more with #30986. I repro'd the consistency failure both times without it and never with it. I'm going to tentatively call this case closed and merge!

bors r=tschottdorf

[benesch]

Yep, I ran this twice more without #30986 and twice more with #30986. I repro'd the consistency failure both times without it and never with it. I'm going to tentatively call this case closed and merge!

bors r=tschottdorf

[craig]

Build failed (retrying...)

• GitHub CI (Cockroach)

[craig]

Build failed (retrying...)

• GitHub CI (Cockroach)

[benesch]

And somehow TC manages to be much better than I am about reproducing flakes. Gah!

bors r-

[benesch]

And somehow TC manages to be much better than I am about reproducing flakes. Gah!

bors r-

[craig]

Canceled

[craig]

Canceled

[tschottdorf]

When you roachprod-stress{,race}, are you using identical machines?

roachprod create peter-stress -n 20 --gce-machine-type=n1-standard-8 --local-ssd=false

Of course the difference is that CI runs the whole test suite at once (so multiple packages are running concurrently).

[tschottdorf]

When you roachprod-stress{,race}, are you using identical machines?

roachprod create peter-stress -n 20 --gce-machine-type=n1-standard-8 --local-ssd=false

Of course the difference is that CI runs the whole test suite at once (so multiple packages are running concurrently).

[benesch]

Yep, using identical machines. I think the problem is that this failure was a timeout, and since I didn't run roachprod-stress for 25+ minutes I never hit the timeout. Maybe I can repro this locally.

[benesch]

Yep, using identical machines. I think the problem is that this failure was a timeout, and since I didn't run roachprod-stress for 25+ minutes I never hit the timeout. Maybe I can repro this locally.

[tschottdorf]

Reviewed 3 of 3 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, 1 of 1 files at r5, 2 of 2 files at r6.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)

[benesch]

Ok, 18th time is the charm. 🍿

[benesch]

Ok, 18th time is the charm. 🍿

[tschottdorf]

Reviewed 3 of 3 files at r7, 2 of 2 files at r8.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)

[benesch]

whew!

bors r=tschottdorf

[benesch]

whew!

bors r=tschottdorf

[craig]

Build failed

• GitHub CI (Cockroach)

[craig]

Build failed

• GitHub CI (Cockroach)

[tschottdorf]

bors r+

flaked on #31287

[tschottdorf]

bors r+

flaked on #31287

[craig]

Build succeeded

• GitHub CI (Cockroach)

[craig]

Build succeeded

• GitHub CI (Cockroach)

[benesch]

whoa, thanks, I'd just assumed that was my fault!

[benesch]

whoa, thanks, I'd just assumed that was my fault!