kvdb/postgres: remove global application level lock #8644
Conversation
Roasbeef
added
database
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
saubyk
requested review from
a team,
bhandras and
yyforyongyu
and removed request for
a team
|
@bhandras: review reminder |
|
Gonna close then re-open so CI can run again. |
|
would be nice to see this rebased since the nativesql tests run correctly now as well |
Roasbeef
force-pushed
the
remove-sql-mutex-part-deux
branch
from
8394e13
to
c7975c7
Compare
|
Rebased! |
|
LGTM too! |
|
Rebasing. I don't think the failures are flakes (last I looked at the logs, wallet related tests we're failing), we'll see with the next CI run. |
In this commit, we remove the global application level lock from the postgres backend. This lock prevents multiple write transactions from happening at the same time, and will also block a writer if a read is on going. Since this lock was added, we know always open DB connections with the strongest level of concurrency control available: `LevelSerializable`. In concert with the new auto retry logic, we ensure that if db transactions conflict (writing the same key/row in this case), then the tx is retried automatically. Removing this lock should increase perf for the postgres backend, as now concurrent write transactions can proceed, being serialized as needed. Rather then trying to handle concurrency at the application level, we'll set postgres do its job, with the application only needing to retry as necessary.
Roasbeef
force-pushed
the
remove-sql-mutex-part-deux
branch
from
c7975c7
to
43a1ca4
Compare
|
Kicked CI to re-run....yuge W here if this passes! |
|
aaaand we're ✅ |
Unfortunately because of the missing |
In this commit, we remove the global application level lock from the postgres backend. This lock prevents multiple write transactions from happening at the same time, and will also block a writer if a read is on going. Since this lock was added, we know always open DB connections with the strongest level of concurrency control available:
LevelSerializable. In concert with the new auto retry logic, we ensure that if db transactions conflict (writing the same key/row in this case), then the tx is retried automatically.Removing this lock should increase perf for the postgres backend, as now concurrent write transactions can proceed, being serialized as needed. Rather then trying to handle concurrency at the application level, we'll set postgres do its job, with the application only needing to retry as necessary.
Remake of #7992