Skip to content

Eliminate datastore threadpool executor#18270

Open
anodos325 wants to merge 1 commit intomasterfrom
mt-datastore
Open

Eliminate datastore threadpool executor#18270
anodos325 wants to merge 1 commit intomasterfrom
mt-datastore

Conversation

@anodos325
Copy link
Contributor

This commit removes the datastore plugin threadpool executor that was serializing all database reads / writes inside a single thread. Instead it is replaced with per-thread SQL connection objects with generation tracking (for noting when we need to reopen the database due to config file upload). Reads are MT-safe, writes are serialized behind a threading lock primarily for the purposes of enterpise HA and cross-node database consistency.

This commit removes the datastore plugin threadpool executor
that was serializing all database reads / writes inside a single
thread. Instead it is replaced with per-thread SQL connection
objects with generation tracking (for noting when we need to
reopen the database due to config file upload). Reads are
MT-safe, writes are serialized behind a threading lock primarily
for the purposes of enterpise HA and cross-node database
consistency.
Copy link
Contributor

@themylogin themylogin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there any performance bottleneck somewhere or is there any other reason to do this?

Copy link
Contributor

@themylogin themylogin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to achieve concurrent read and write, PRAGMA journal_mode=WAL; must also be enabled.

@anodos325
Copy link
Contributor Author

anodos325 commented Feb 23, 2026

Was there any performance bottleneck somewhere or is there any other reason to do this?

I haven't robustly checked for performance issues, but I'm concerned that a slow writer (if we're having to replicate changes) can block reads significantly. I'll enable WAL. I was also thinking of plumbing in asyncio taskgroups to concurrently gather render context for etc file generation and testing. It's kind of pointless though if entire datastore layer is single-threaded.

@themylogin
Copy link
Contributor

Sorry, but I am not sure the problem even exists. Our database is 1MB tops, it fits entirely into RAM, how can there be slow readers or writers?

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.

2 participants