prevent db deadlocks while updating feature tags [MRXN23-316]

[hotzevzl]

https://vizzuality.atlassian.net/browse/MRXN23-316

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
marxan	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 15, 2023 4:30pm

[hotzevzl]

@KevSanchez so, I've been unable to consistently reproduce the db transaction festival in my local dev environment, but I'm actually able to reproduce this almost always on marxan23 with the current develop code, whereas when checking out this WIP branch (still on m23), I have not been able to trigger the deadlock ever.

Not that I've actually spent hours doing this, but combining my initial guess of where the issue may have been, with the changes I'm suggesting in this PR as well as the behaviour just noted above about reproducing this (or not) on marxan23, I'd be inclined to vaguely trust that the mix of in-transaction and out-of-transaction db ops in the current implementation may be at least part of the observed behaviour.

Given how jumpy the behaviour is, I am not sure how to test that the proposed fix is indeed a fix and that the observed problem is indeed a problem: I think the problem manifests itself depending on timing of sequential operations, so even writing a test that for example creates 100 features and then tries to bulk edit their tags all at once may occasionally still pass without the changes in this PR depending on... things (sequence of requests, load on CPU and IO for the node where the tests are running, etc.).

Maybe still worth it through brute force? I am not sure. Especially at this stage I'd be inclined to be satisfied with a cross-check of the changes from your end.

[KevSanchez]

I think it's best to do as you suggest, as the nature of the error was really erratic for me too; I could bulk edit a couple of tags several times in a row without issue, and other times, even right after spinning the API up.
I noticed that the previous code, by not really doing the operations inside the transaction block might explain this. In any case, it makes much more sense the way you're suggesting now.

[hotzevzl]

I think it's best to do as you suggest, as the nature of the error was really erratic for me too; I could bulk edit a couple of tags several times in a row without issue, and other times, even right after spinning the API up

Thanks @KevSanchez. So I suggested a "brute-force" approach to testing this, here: 9fde377

Green on the current PR's branch with the "fix" applied: https://github.com/marxan-cloud/marxan-cloud/actions/runs/6880148837/job/18713710344

Red in a separate branch where I only included the test added on top of develop, without the commit with the "fix": https://github.com/marxan-cloud/marxan-cloud/actions/runs/6880117803/job/18713605077#step:3:18460

As I would expect, this test fails because of timeout, as the trouble is at db level.

For this reason, I'm not too convinced that this is a wise thing to do, but I'd say, unless you hate it, let's include the test in this PR and if it turns out to be a fickle test in practice (it surely is in principle), then we do something about it.