Backport 1/1 commits from #135861 on behalf of @yuzefovich.
/cc @cockroachdb/release
This commit addresses two problems with TestCreateStatsUsingExtremesProgress:
previously, it was possible for the main goroutine to get stuck when retrieving the stats job progress because the progress was nil. My guess is that this was due to us sending on the channel that we block scans on only twice total before attempting to retrieve the progress. In the store filter we receive twice from the channel, so it seems like we could have a race between the first scanned row being processed and progress being written in one goroutine against running out of SucceedsSoon duration of 45s in another. We now send on the channel ten times before retrieving the progress for the first time which should prevent this race. (This is similar to what we do in TestCreateStatsProgress. I also tried sending only three times, but the problem was still present.)
if we hit the succeeds soon error as described above, we would previously incorrectly shut down the test. This was the case because the test server would be stuck in the store filter we installed, and our attempt at closing the necessary channel was deferred before we defer the call to stop.Stopper.Stop which blocks until all tasks exit, but the server couldn't exit because it had one goroutine stuck on the channel. We fix this by deferring the closure of the channel after deferring the Stop call. The same problem was also present in TestCreateStatsProgress which is also now fixed.
Please check the backport criteria before merging:
[ ] Backports should only be created for serious
issues or test-only changes.
[ ] Backports should not break backwards-compatibility.
[ ] Backports should change as little code as possible.
[ ] Backports should not change on-disk formats or node communication protocols.
[ ] Backports should not add new functionality (except as defined
here).
[ ] Backports must not add, edit, or otherwise modify cluster versions; or add version gates.
[ ] All backports must be reviewed by the owning areas TL. For more information as to how that review should be conducted, please consult the backport
policy.
If your backport adds new functionality, please ensure that the
following additional criteria are satisfied:
- [ ] There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
- [ ] The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
- [ ] New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters. State changes must be further protected such that nodes running old binaries will not be negatively impacted by the new state (with a mixed version test added).
- [ ] The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.
- [ ] Your backport must be accompanied by a post to the appropriate Slack
channel (#db-backports-point-releases or #db-backports-XX-X-release) for awareness and discussion.
Also, please add a brief release justification to the body of your PR to justify this
backport.
Backport 1/1 commits from #135861 on behalf of @yuzefovich.
/cc @cockroachdb/release
This commit addresses two problems with
TestCreateStatsUsingExtremesProgress
:TestCreateStatsProgress
. I also tried sending only three times, but the problem was still present.)stop.Stopper.Stop
which blocks until all tasks exit, but the server couldn't exit because it had one goroutine stuck on the channel. We fix this by deferring the closure of the channel after deferring theStop
call. The same problem was also present inTestCreateStatsProgress
which is also now fixed.Fixes: #135733.
Release note: None
Release justification: test-only change.