storage: reproposals after snapshot may swallow necessary ambiguous result

[tbg]

See #30792 (comment) for full details.

When we call refreshProposalsLocked, we pass in a refreshReason. If this reason indicates that a snapshot was applied, we return ambiguous results to clients waiting on those proposals (since they might've applied through a snapshot; we don't know).

Unfortunately, we sometimes override this refreshReason at the caller in handleRaftReady, so that in principle ambiguous results could be omitted when they are necessary.

Jira issue: CRDB-4769

Epic CRDB-39898

[andreimatei]

Tobi I tried to do something but I don't quite understand the code that does the dubious overwrite. Mind if I assign you here?
This came back to my mind because I'm doing the related #33381 and I was about to file it again.

[tbg]

Sure, I'll take a look.

[nvanbenschoten]

This still seems real, right? We still appear to prioritize reasonNewLeader and reasonNewLeaderOrConfigChange above reasonSnapshotApplied in handleRaftReadyRaftMuLocked.

[tbg]

Yes, still a problem.

[tbg]

I poked at this and it'd be really easy to fix but then hard to test, so at that point we'll want to pull out the reason state changes into something that can stomach unit testing. But that takes a bit of work.

[github-actions]

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!