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

Exclusive DB access #15029

Merged
merged 11 commits into from
Oct 30, 2023
Merged

Conversation

vient
Copy link
Contributor

@vient vient commented Oct 29, 2023

Changelog: Feature: Allow only one simultaneous database connection.
Docs: omit

Implementation of a proposed solution for "database is locked" errors.

Fixes #13924 - I did not really check that this change fixes the error but it should

@vient
Copy link
Contributor Author

vient commented Oct 30, 2023

Checked with parallel=100 (I have ~80 packages in my project), adding exclusive lock does not affect performance

@memsharded memsharded added this to the 2.0.14 milestone Oct 30, 2023
@vient
Copy link
Contributor Author

vient commented Oct 30, 2023

In the last jenkins run Linux integration tests slowed down by a minute 🤔
Functional tests also slowed down

@memsharded
Copy link
Member

In the last jenkins run Linux integration tests slowed down by a minute 🤔
Functional tests also slowed down

Interesting. It is possible that the class instance is common to the whole running suite and that is creating the slow down? I think tests are run in parallel in CI. This would have a similar impact on users using the Conan Python API to automate things that run in parallel even if they are using different DBs. Might be worth trying if a lock per DB improves this (using the self.filename)

@vient
Copy link
Contributor Author

vient commented Oct 30, 2023

Makes sense. We'll need a dict with its own lock then (defaultdict with threading.Lock may be thread safe in current CPython but noGIL cpython is in the progress..)

@vient
Copy link
Contributor Author

vient commented Oct 30, 2023

OK, let's try dict without locking first

@vient
Copy link
Contributor Author

vient commented Oct 30, 2023

Well, I pushed and immediately understood that slowdown occurred after I added timeout to lock, before this running times were fine. While jenkins is still running, I'll hypothesize that slowdown may be about heavy usage of time functions when timeout is defined, and we won't see any improvement in this version.

@vient
Copy link
Contributor Author

vient commented Oct 30, 2023

Huh, it seems that jenkins runs are not so precise. In the last commit I commented out all changes and it still works slower than in the first build. What is more weird is that it looks like runs slowed down after third one - first three are similar, and in 4-9 results are also consistent with changes, but run 9 is slower than 1 even if there are no locks at all at 9 while there were per-instance locks in 1.

I'll return per-filename locks then.

@vient
Copy link
Contributor Author

vient commented Oct 30, 2023

And now runs in 10 take ~same time as in 1 😵‍💫
I'll call it "does not noticeably affect performance" then.

@memsharded memsharded requested a review from czoido October 30, 2023 10:38
Co-authored-by: Carlos Zoido <mrgalleta@gmail.com>
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.

3 participants