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

DBCP-592: Support request boundaries #324

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

meedbek
Copy link

@meedbek meedbek commented Nov 21, 2023

JDBC 4.3 introduced two new methods, beginRequest and endRequest, which connection pools can use to mark request boundaries.
According to JavaDoc, these methods signal to the driver that a connection is active, the driver's response is specific to the vendor and it may not do anything.
Goal is to make DBCP invoke beginRequest when a connection is borrowed and endRequest when the connection is returned to the pool.

@garydgregory
Copy link
Member

Hello @meedbek

  • How will this not clash for apps that call commit() and rollback()?
  • Since we do not call commit() and rollback() on behalf of call sites, why should we call these two APIs?

@meedbek
Copy link
Author

meedbek commented Nov 29, 2023

  • APIs?

Hi @garydgregory,

  1. Based on the JavaDoc if a request was ended then endRequest is a no-op so calling it should not clash for apps that call commit or rollback.
  2. These two methods do not begin or end transactions, they are just a hint to the driver that a request has began or ended. Using them can improve the driver performance and the user experience. Though if there is an active transaction when the connection is released, calling endRequest may throw an exception depending on the implementation.

@psteitz
Copy link
Contributor

psteitz commented Jan 19, 2024

I think adding these calls would be a bad idea. The spec is vague and at least some implementations do things that DBCP already does in activation/passivation and may cause problems for some applications. See, e.g https://learn.microsoft.com/en-us/sql/connect/jdbc/jdbc-4-3-compliance-for-the-jdbc-driver?view=sql-server-2017. The strange wording in the spec may also mean that DBCP should not send the calls, as we do work with connection handles.

@garydgregory
Copy link
Member

I agree with @psteitz, this feels like a won't do for now. It's not clear to me there's a site to place these calls for a generic library. This feels more like an app-level call, not something called from the guts of DBCP.

@ecki
Copy link
Contributor

ecki commented Jan 22, 2024

They can be very powerful, not only can they reset internal state with minimum round-trips but also ether are used for things like licenses check and reconnect in some drivers. So it would be good if it optionally can be switched on. But yes I agree it requires in depth Integration tests and also maybe changes. So far that maybe it needs a whole own connection pool logic in regards to caching.

@psteitz
Copy link
Contributor

psteitz commented Jan 22, 2024

Thanks for weighing in, Bernd. Do you by chance have some links to what drivers are actually doing here? I looked at Postgres and Mysql and could not find any implementations. I would like to understand better what the drivers actually do. The spec makes it look like, as Gary says, more an app concern, except they expect pool managers to send the messages which does not make sense to me. I must be missing something. When you say "caching" I am not sure what you mean. The whole pool is essentially a cache.

From what I have gleaned so far, it seems to me that the better place to put these calls might be in activate and passivate. That led me to think that allowing PoolableConnectionFactory to be pluggable or the lambdas for the lifecycle methods would allow users to set this up however they wanted.

@ecki
Copy link
Contributor

ecki commented Jan 23, 2024

I was curious on those begin/end but could not find much of documentation. So I snooped a bit around in the source code of some drivers. Besides the resetting of the client side state (the typical fields inside a SQL Connection object) there is also the possibility of server side session state resets, where the drivers would only send demarcation messages. I think mssql doesnt do that (but it has a reset flag in the protocol). In the MSSQL-JDBC driver I found some code to reconnect broken connection. I thought it was linked to the request state, but cant find it anymore.

I think I have also seen some logic for different timeout handlings of sessions outside the request block and associated diagnostics in oracle documents (but cant find it again at the moment).

So - it might get promising for some drivers in the future to use it. Maybe an app listener hook can also use it, but then the pool would still do the reset/aliveness dance which it could skip for qualified drivers.

MSSQL Resets:
https://github.com/microsoft/mssql-jdbc/blob/bb76a7832ecf1cd24238c75cf7c2c4706d922435/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java#L7561
Session recovery
https://github.com/microsoft/mssql-jdbc/blob/bb76a7832ecf1cd24238c75cf7c2c4706d922435/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java#L4327
https://github.com/microsoft/mssql-jdbc/blob/bb76a7832ecf1cd24238c75cf7c2c4706d922435/src/main/java/com/microsoft/sqlserver/jdbc/IdleConnectionResiliency.java#L16

@garydgregory
Copy link
Member

All of this hints to me that it is too early for us to integrate these calls into DBCP. Once more FOSS drivers implement these APIs in a meaningful manner will be able to create tests to make sure that (1) we do not break existing clients and (2) properly use the APIs.

WDYT?

@garydgregory
Copy link
Member

Oh, also when we do decide to use the APIs we should NOT use reflection. We can either bump the platform requirement or create a new Maven module with that requirement in a subclass or as some kind of plugin/hook.

@ecki
Copy link
Contributor

ecki commented Jan 23, 2024

Yes its early, maybe wait for concrete requests with a desireable driver behavior.

Having said that, calling endRequest to reset the vendor specific connection attributes seems like a win for us (i.e. we dont have to track them).

@ecki
Copy link
Contributor

ecki commented Feb 5, 2024

Here is btw oracles doc for they replay driver, it uses the request boundaries in this way: https://docs.oracle.com/en/database/oracle/oracle-database/23/jjdbc/application-continuity.html#GUID-AAC6F9B7-9B4C-4098-B0D5-312BF9A13928

@jeandelavarene
Copy link

With JDBC 4.2, the driver doesn't know when a connection is borrowed from connection cache or when a connection is returned to the cache. It is, however, a very useful piece of information. For example, when a connection is returned, the driver may decide to flush temporary resources or let the database know so that it can compute statistics, or even replace the connection with another one during a planned maintenance event of the Database. When a connection is borrowed from the cache, the driver may start recording calls in its replay queue in case of an unplanned outage so that it can transparently replay a request. This is the reason why these APIs beginRequest and endRequest were added to the JDBC 4.3 specification.

There are endless possibilities.

But this relies on connection pools such as DBCP to call these APIs. The features that rely on request boundaries can only be activated with connection pools that make these calls. This is why we (at Oracle) think that this is a very desirable feature to add to DBCP.

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.

5 participants