Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(memdb): waaay too slow, at least use rwlock #4329

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

dpc
Copy link
Contributor

@dpc dpc commented Feb 15, 2024

After a lot of debuging I discovered that some flakes are due to all the slowness of MemDatabase.

On an even ligthly filled test database, coping all the data start taking considerable amount of time (single digit milliseconds), which combined with number of database transactions being opened and commited, can lead to quite long delays in processing in random places (as ordering of locking is somewhat arbitrary).

It's possible that something is also running db transactions in a tight loop somwhere.

For time being using an RwLock helps considerably, as all db transactions at least need to start, and we do two copied of the database content there, so being able to do it in parallel, significantly improves the throughput.

Having said that, current implementation is not practical for any production use, I'm afraid.

After a lot of debuging I discovered that some flakes are
due to all the slowness of `MemDatabase`.

On an even ligthly filled test database, coping all the data
start taking considerable amount of time (single digit milliseconds),
which combined with number of database transactions
being opened and commited, can lead to quite long delays in
processing in random places (as ordering of locking is somewhat
arbitrary).

It's possible that something is also running db transactions
in a tight loop somwhere.

For time being using an `RwLock` helps considerably, as
all db transactions at least need to start, and we do two
copied of the database content there, so being able to do it in parallel,
significantly improves the throughput.
@dpc
Copy link
Contributor Author

dpc commented Feb 15, 2024

#4335 even better, I hope.

@dpc dpc added this pull request to the merge queue Feb 15, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 15, 2024
@maan2003 maan2003 added this pull request to the merge queue Feb 15, 2024
github-merge-queue bot pushed a commit that referenced this pull request Feb 15, 2024
fix(memdb): waaay too slow, at least use rwlock
@m1sterc001guy
Copy link
Contributor

Having said that, current implementation is not practical for any production use, I'm afraid.

Perhaps we should re-prioritize looking into SurrealDB as a replacement https://github.com/surrealdb/surrealkv

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Feb 15, 2024
@dpc dpc added this pull request to the merge queue Feb 15, 2024
Merged via the queue into fedimint:master with commit 0ea55cb Feb 15, 2024
22 checks passed
@dpc dpc deleted the 24-02-14-memdb-rwlock branch February 15, 2024 16:35
dpc added a commit to dpc/fedimint that referenced this pull request Feb 15, 2024
@dpc dpc mentioned this pull request Feb 15, 2024
@elsirion
Copy link
Contributor

Longer-term we might want a copy-on-write map type?

@dpc
Copy link
Contributor Author

dpc commented Feb 20, 2024

#4335 @elsirion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants