-
Notifications
You must be signed in to change notification settings - Fork 171
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
SNOW-1335472: Support Virtual Threads by using ReentrantLock
instead of synchronized
for SFSession#open
#1729
Comments
ReentrantLock
instead of synchronized
for SFSession#open
ReentrantLock
instead of synchronized
for SFSession#open
Hello @jtb93 , Thanks for raising the issue, Could you please let us know how its impacting currently, please explain the scenario where and how it will improve the performance. Please note: While ReentrantLock might provide better performance in some scenarios, it also incurs some overhead compared to the intrinsic locking mechanism provided by synchronized. And also the performance will vary as per scenarios. The example which you quoted for "SFSession.open" , may not result in significant performance improvements. The reason is that the critical section of code protected by the lock (openLock) is not particularly long or complex. In this case, the critical section of code is relatively short, so the overhead of acquiring the lock is not likely to be a significant factor in overall performance. will keep this thread posted. Regards, |
Hi @sfc-gh-sghosh. The team behind the MariaDB JDBC driver team actually put out a pretty thorough write-up that does a good job of explaining the benefits. And while the My understanding is that pinning that either only occurs very infrequently (e.g. just at app start-up) or does not involve I/O is typically okay. But when leveraging something like Spring Boot MVC w/ Virtual threads enabled + Snowflake JDBC, pinning happens repeatedly (as can be demonstrated by the numerous log messages that occur when the app is run w/ option LMK if I’m off-base conceptually, if something like a print-out of the log messages I am seeing would be helpful, or you have any advice on if there are any work-around approaches we could use to avoid and/or mitigate pinning other than swapping-in Thanks! |
Compare the performance difference between the Maria DB + MySQL JDBC drivers. I agree that any difference between |
And, for clarity, the overhead of acquiring the lock is not my concern. It's the pinning of the Virtual thread carrier threads that is the issue. Compare the performance difference between the Maria DB + MySQL JDBC drivers. I agree that any difference between ReentrantLock and synchronized in a vacuum is likely to be negligible. But when considering how Virtual Threads operate there is a substantial difference. (Sorry for the double-post. Don't seem to be able to edit posts) |
Hi @sfc-gh-sghosh , been about a week since I last heard from you. Any update? My team is looking at making our decision whether or not to use Snowflake JDBC this week, so knowing if the library intends to support Virtual Threads or not would be really helpful :-) |
Hi @sfc-gh-sghosh, it's been a couple weeks since I last heard from you. Any update yet or an approximate ETA for when status-triaging will be completed? Thanks! |
Hello @jtb93 , Sorry for the delay, and thanks for the update; we are checking internally with the team and will update. Regards, |
@sfc-gh-sghosh @sfc-gh-snow-drivers-warsaw-dl Virtual Thread pinning using Spring Boot MVC w/ virtual threads enabledUsing
|
Hi @sfc-gh-snow-drivers-warsaw-dl, it's been a few weeks since I last received an update, so just checking-in. LMK if there's anything I can do to help expedite things. |
hi @jtb93 appreciate your interest in the matter. Your enhancement request is with the relevant team who will consider it for future plans, alongside with all the other enhancement requests, but we cannot commit to any timeline at this point. If this capability is very important for your use-case, the best next step is to contact your Snowflake account team (your Sales rep. or the SE) and express the importance of the enhancement to them. They can help product management to reprioritize this request. Alternatively of course if that's within your capabilities, you can also submit a PR with the implementation (I saw that mentioned somewhere in this issue?) which contribution is very much appreciated. |
What is the current behavior?
The
SFSession#open
function issynchronized
, so when using Snowflake JDBC w/ Virtual Threads, pinning frequently occurs.What is the desired behavior?
The same functionality can be implemented using
ReentrantLock
and Virtual Thread pinning will be avoided. Happy to open a PR if there isn't a particular reason whysynchronized
still needs to be used.How would this improve
snowflake-jdbc
?Better performance and reliability of apps leveraging
snowflake-jdbc
References, Other Background
Proposed implementation:
What is your Snowflake account identifier, if any?
The text was updated successfully, but these errors were encountered: