Do not take snapshots after shutdown was requested

[pihme]

Description

The change sets the db to null right away, so that any concurrent calls thereafter have no access to the database while it is closing.

It is not completely safe yet. The database could be set to 'nullbetween checking fornull` in line 187 and using the reference in line 194.

Related issues

related to #7188

Definition of Done

Code changes:

• The changes are backwards compatibility with previous versions
• If it fixes a bug then PRs are created to backport the fix to the last two minor versions. You can trigger a backport by assigning labels (e.g. backport stable/0.25) to the PR, in case that fails you need to create backports manually.

Testing:

• There are unit/integration tests that verify all acceptance criterias of the issue
• New tests are written to ensure backwards compatibility with further versions
• The behavior is tested manually
• The change has been verified by a QA run
• The impact of the changes is verified by a benchmark

Documentation:

• The documentation is updated (e.g. BPMN reference, configuration, examples, get-started guides, etc.)
• New content is added to the release announcement

[npepinpe]

As you pointed out, this reduces the rate of incidence of the bug but doesn't completely fix it. Since taking a snapshot may happen on a different thread, it could even be that db is already null when the check is done on line 187, but the value read is outdated. IIRC, the state controller is called from the AsyncSnapshotDirector and from the ZeebePartition, both of which are actors, and as such executed at times on different threads. It could be that we should make the state controller an actor as well, though that would widen the scope of this PR.

❓ Do you think that the race condition you highlighted is not likely enough/important enough to warrant spending more time, or do you see value in immediately mitigating it with the idea that we'll come back and fix it properly later? If it's the latter, then we shouldn't close the issue by merging this PR, as it's not really fixed. I personally would opt to fix it properly, but I'd like to hear your thoughts.

[pihme]

question Do you think that the race condition you highlighted is not likely enough/important enough to warrant spending more time, or do you see value in immediately mitigating it with the idea that we'll come back and fix it properly later?

I think it is an improvement over status quo. I also hope that with the other work we are doing and that Chris is doing this error won't be able to occur anymore, because we have better control over the shutdown sequence.

But I am also fine with leaving the issue open and revisiting it later.

[npepinpe]

👍

Let's merge this for now, but remove the closes from the PR description as I wouldn't close the issue yet.

[pihme]

bors r+

[ghost]

Build succeeded:

• continuous-integration/jenkins/branch

[github-actions]

Backport failed for stable/1.0, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin stable/1.0
git worktree add -d .worktree/backport-7571-to-stable/1.0 origin/stable/1.0
cd .worktree/backport-7571-to-stable/1.0
git checkout -b backport-7571-to-stable/1.0
ancref=$(git merge-base be2bfabcd49de3185e0faa952f4bc7a227259148 c8fe46c88a785bbc52fdb6b54cec4fa1bcafc193)
git cherry-pick -x $ancref..c8fe46c88a785bbc52fdb6b54cec4fa1bcafc193

[github-actions]

Backport failed for stable/1.1, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin stable/1.1
git worktree add -d .worktree/backport-7571-to-stable/1.1 origin/stable/1.1
cd .worktree/backport-7571-to-stable/1.1
git checkout -b backport-7571-to-stable/1.1
ancref=$(git merge-base be2bfabcd49de3185e0faa952f4bc7a227259148 c8fe46c88a785bbc52fdb6b54cec4fa1bcafc193)
git cherry-pick -x $ancref..c8fe46c88a785bbc52fdb6b54cec4fa1bcafc193

[npepinpe]

It looks like we never backported this in the end? /cc @pihme

Or at least I don't see any linked PRs 🤔

[pihme]

Seems like it slipped through, yes.