release-21.2: stats: fix autostats panic with minStaleRows == 0

[msirek]

Fixes #89323

This fixes a panic in autostats code when cluster setting
sql.stats.automatic_collection.min_stale_rows is 0.

Release note (bug fix): This patch fixes errors that may
occur in autostats collection when cluster setting
sql.stats.automatic_collection.min_stale_rows is set to 0.

Release justification: high-severity bug in existing functionality;
autostats collection should never panic.

[blathers-crl]

Thanks for opening a backport.

Please check the backport criteria before merging:

• Patches should only be created for serious issues or test-only changes.
• Patches should not break backwards-compatibility.
• Patches should change as little code as possible.
• Patches should not change on-disk formats or node communication protocols.
• Patches should not add new functionality.
• Patches must not add, edit, or otherwise modify cluster versions; or add version gates.

If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.

• 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.
• The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

• What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
• Will this work in a cluster of mixed patch versions? Did we test that?
• If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

[cockroach-teamcity]

This change is 

[msirek]

This is backporting some code from #78110.

[cucaroach]

Does is make sense to allow setting minStaleRows to zero? What does that mean? Recreate stats on any update/insert/delete? Wouldn't 1 be the same? Maybe it should be an error to set it to 0? Or maybe it should mean never recreate stats (ie turn off auto stats)?

Where's the PR for this to be fixed on master? Did I miss it or does this bug not exist on master?

The release justification is just a restatement of the description, it should be something like one of the 4 categories here: https://cockroachlabs.atlassian.net/wiki/spaces/ENG/pages/168656965/Guidelines+for+Release+Stability+Period

[msirek]

I hadn't questioned the valid range of min_stale_rows before. It's had a min value of zero for a couple years. The description says:

Target minimum number of stale rows per table that will trigger a statistics refresh

And there's an example in docs:

Suppose the clusters settings sql.stats.automatic_collection.fraction_stale_rows and sql.stats.automatic_collection.min_stale_rows have the default values .2 and 500 as shown in the preceding table.

If a table has 100 rows and 20 became stale, a re-collection would not be triggered because, even though 20% of the rows are stale, they do not meet the 500 row minimum.

So, I think a min_stale_rows setting of zero would mean basically the thing as a setting of 1. The number of mutations must be greater than or equal to this value to trigger auto stats. In the case that rowCount starts off low, like a value of 1, a min_stale_rows setting of zero would trigger stats refresh on the next mutation (if fraction_stale_rows is less than 1). This seems like OK behavior. Once the rowCount start growing, rowCount * fraction_stale_rows would be above zero and we'd start randomly triggering autostats (instead of on every mutation).

Where's the PR for this to be fixed on master? Did I miss it or does this bug not exist on master?

This code was part of #78110, which I'm backporting to fix this issue.

I've updated the release justification to category 3.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @cucaroach and @michae2)

[msirek]

I suppose users could get creative and set both fraction_stale_rows and min_stale_rows to zero to force autostats collection on every mutation. That could be by design, but I'm not sure. Copying @rytaft

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @cucaroach and @michae2)

[michae2]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @cucaroach)

[msirek]

TFTR!

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @cucaroach)