Open jtb93 opened 7 months ago
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, Sujan
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 SFSession#open
code may not appear super long or complex on its own, performing any I/O within synchronized functions or blocks results in “pinning” (i.e. the small, fixed-size thread-pool of carrier threads, which Virtual Threads run on, gets blocked, effectively preventing the app from being able to serve concurrent requests).
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 -Djdk.tracePinnedThreads=short
).
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 ReentrantLock
for synchronized
. :slightly_smiling_face:
Thanks!
the overhead of acquiring the lock is not likely to be a significant factor in overall performance 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.
the overhead of acquiring the lock is not likely to be a significant factor in overall performance
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. Could you please provide us the logs from Spring boot MVC with virtual thread enabled when run w/ option -Djdk.tracePinnedThreads=short.
Regards, Sujan
@sfc-gh-sghosh @sfc-gh-snow-drivers-warsaw-dl
LMK if there's any else needed from my end!
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?