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

High Contention due to inroduced call to DriverManager.println #984

Closed
yuvalp-k2view opened this issue Oct 1, 2023 · 7 comments
Closed
Labels
enhancement released Issue has been released

Comments

@yuvalp-k2view
Copy link
Contributor

Describe the bug
NativeDB_exec and NativeDB.prepare() call DriverManager.println()
Unfortunately this method has a global lock creating a huge bottleneck in multithread (connection per thread) environments.
I suggest removing these calls.

To Reproduce
Prepare statements on multiple threads

Expected behavior
No contention between threads working on separate connections.

Additional context
println imp for your reference :-(

public static void println(String message) {
    synchronized (logSync) {
        if (logWriter != null) {
            logWriter.println(message);

            // automatic flushing is never enabled, so we must do it ourselves
            logWriter.flush();
        }
    }
}
@gotson
Copy link
Collaborator

gotson commented Oct 1, 2023

Unfortunately this method has a global lock creating a huge bottleneck in multithread (connection per thread) environments.

Would you be able to share more on this?

@gotson
Copy link
Collaborator

gotson commented Oct 1, 2023

Related to #802

@yuvalp-k2view
Copy link
Contributor Author

I've attached the code from DriverManager. You see that they are synchronizing logSync which is defined:
private static final Object logSync = new Object();

This means that even if there is no logging, there is a contention point that all threads creating a prepared statement go through. I think this method should be avoided, especially in success scenarios where high throughput is required.

@gotson
Copy link
Collaborator

gotson commented Oct 1, 2023

Yes I saw that, but did you do some tests or benchmarks with and without? Or is that just an observation of the code above and that's it?

@yuvalp-k2view
Copy link
Contributor Author

This came up in real life. The complaint was we could not scale vertically on a larger machine (128 vcpu).
We could not go beyond 20% cpu usage. Once we identified that our threads were mostly blocked, I found this. Downgrading the version resolved the issue and our threads were no longer blocked.

@gotson
Copy link
Collaborator

gotson commented Oct 2, 2023

Thanks, that is useful context that could have been provided in the first post.

@gotson gotson added enhancement and removed triage labels Oct 2, 2023
@gotson gotson closed this as completed in 75ce563 Oct 3, 2023
@github-actions
Copy link
Contributor

🎉 This issue has been resolved in 3.43.2.0 (Release Notes)

@github-actions github-actions bot added the released Issue has been released label Oct 13, 2023
gotson added a commit to gotson/sqlite-jdbc that referenced this issue Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement released Issue has been released
Projects
None yet
Development

No branches or pull requests

2 participants