sql: byte monitor panic when a hard(ungraceful) shutdown catches up with a graceful drain

[knz]

Describe the problem

While investigating #100989 I created the following scenario:

1. SQL graceful shutdown starts. This causes in the background the SQL statement stats provider to start persisting txn/stmt stats.
2. while the above is ongoing, the test triggers a hard stop, by quiescing the stopper + canceling the graceful drain context. We can confirm in logs that it causes the ongoing stmt/txn persistence queries to be canceled.
3. when all stopper tasks have quiesced, this finalizer in stats provider is called by stopper (from sql > sqlstats > persistedsqlstats > provider.go > - (*PersistedSQLStats).Start):

  stopper.AddCloser(stop.CloserFn(func() {
    s.cfg.InternalExecutorMonitor.Stop(ctx)
  }))

And this panics:

*
* ERROR: a panic has occurred!
* internal SQL executor: unexpected 122880 leftover bytes
* (1) attached stack trace
*   -- stack trace:
*   | runtime.gopanic
*   |   GOROOT/src/runtime/panic.go:884
*   | [...repeated from below...]
* Wraps: (2) attached stack trace
*   -- stack trace:
*   | github.com/cockroachdb/cockroach/pkg/util/log/logcrash.ReportOrPanic
*   |   github.com/cockroachdb/cockroach/pkg/util/log/logcrash/crash_reporting.go:390
*   | github.com/cockroachdb/cockroach/pkg/util/mon.(*BytesMonitor).doStop
*   |   github.com/cockroachdb/cockroach/pkg/util/mon/bytes_usage.go:592
*   | github.com/cockroachdb/cockroach/pkg/util/mon.(*BytesMonitor).Stop
*   |   github.com/cockroachdb/cockroach/pkg/util/mon/bytes_usage.go:565
*   | github.com/cockroachdb/cockroach/pkg/sql/sqlstats/persistedsqlstats.(*PersistedSQLStats).Start.func1
*   |   github.com/cockroachdb/cockroach/pkg/sql/sqlstats/persistedsqlstats/pkg/sql/sqlstats/persistedsqlstats/provider.go:112
*   | github.com/cockroachdb/cockroach/pkg/util/stop.CloserFn.Close
*   |   github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:107
*   | github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).Stop
*   |   github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:532

This bug is relatively hard to trigger because in most cases, a server is shut down either only using a graceful drain, or only using a hard stop, but not both.

TestServerStartStop happens to do this and this is where this problem was caught.

I am to work around this by changing mon.Stop to mon.EmergencyStop in that specific case. This papers over the issue and prevents the panic from occurring. However, this is not a good long term fix because it disables the leak detection code, which we still would like to use.

To Reproduce

Take the code from #101296. Remove the last commit which replaces mon.Stop by mon.EmergencyStop in sql > sqlstats > persistedsqlstats > provider.go > - (*PersistedSQLStats).Start. Then stress test TestServerStartStop. Recommended flags:

dev test //pkg/ccl/serverccl -f TestServerStartStop --verbose --stress --test-args='-vmodule=bytes_usage=2'

Expected behavior

The abrupt termination of ongoing SQL stats queries during shutdown should still result in an orderly cleanup of the memory monitors.

Jira issue: CRDB-26896

[knz]

I wonder if this is related to #100761 ?

[knz]

cc @mgartner for triage