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
fix: added GROUP_STARTUP_PARAMETERS boolean property to determine whether or not to group startup parameters in a transaction or not fixes Issue 2423 pgbouncer cannot deal with transactions in statement pooling mode #2425
Conversation
…ther or not to group startup parameters in a transaction or not fixes Issue 2423 pgbouncer cannot deal with transactions in statement pooling mode
I think it should be the other way around - e.g. Executing statements assume auto-commit per statement unless an explicit transaction is started. If "statement pooling" doesn't respect that - a transaction can be started on one connection and each of the "body" statement executed on another (or more) plus finally the commit on a third connection - then users need to explicit set this property to Connection pools looks at the |
Hmmm the idea is that your PR is a "new" behaviour and would have to be requested. Good question for discussion though |
I understand that. However, "statement pooling" assumes auto-commit ALWAYS and SELECT ONLY as ANY transaction semantics is disabled. This is one of the reasons why @pgagroal won't support this mode, as people need to really understand the underlying constraints. Once you code to those constraints transaction pooling will give you the same performance and architecture. |
hmmm... interesting |
From a user level. Having Sure , in JDBC you have to do
and so on, but in practice a transaction manager will handle that for you transparently. This PR is only to group the startup statements, not to change the overall driver behaviour - which would be dangerous. |
So you are referring to user level transactions, not "actual transactions" |
If this PR is merged with |
Thank you @davecramer for the quick PR. (I'd be happy with any of the defaults. I agree that Furthermore, I have still not understood what the original PR is actually supposed to deliver. In a transaction pool, the effects of Thanks again for all of your ongoing efforts in this project! |
@hannibal218bc we've been a little distracted by the latest security advisory. @jesperpedersen care to expound on the use case. My understanding is that it ensures that the startup parameters are all applied to the same connection ? |
@davecramer may I again ask if there is anything we can help to get this issue resolved? The issue is still present in the latest |
I think the PR is good to go, except I would add a relevant notice to the changelog like «revert to … behaviour, do not group startup parameters in a transaction, …» |
Alternatively we bump to 42.4.0 ? |
Sure. Let's merge it, bump the version and proceed with the release. |
fixes Issue #2423