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

Document LazyConnectionDataSourceProxy setup for routing datasource to act on transaction definition read-only flag #21415

Closed
spring-projects-issues opened this issue May 27, 2018 · 8 comments
Assignees
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) type: documentation A documentation task
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

Kaan Ozdokmeci opened SPR-16876 and commented

In org.springframework.transaction.support.AbstractPlatformTransactionManager#getTransactionmethod,

the synchronization point is created after the transaction is created and a connection is attached to it via the doBegin method.

Having the synchronization point created afterwards prevents the ability to utilize an AbstractRoutingDatasource that is able to pick between read-only and read-write datasources for the transaction by inspecting 

org.springframework.transaction.support.TransactionSynchronizationManager#isCurrentTransactionReadOnly

when determining the appropriate datasource.

 

Is there any reason not to move synchronization point creation before the doBegin method so that routing datasources can act on the transaction definition?

If not happy to send a PR.


No further details from SPR-16876

@spring-projects-issues spring-projects-issues added status: waiting-for-triage An issue we've not yet triaged or decided on in: data Issues in data modules (jdbc, orm, oxm, tx) type: enhancement A general enhancement and removed type: enhancement A general enhancement labels Jan 11, 2019
@OrangeDog
Copy link

@jhoeller as the old assignee, can this be triaged?

@kanakharaharsh
Copy link

I faced the same issue recently. I had a quick workaround until this issue not getting fixed.

@Component
@Slf4j
public class CustomTransactionManager extends DataSourceTransactionManager {

    private static final long serialVersionUID = 1L;

    public CustomTransactionManager(DataSource dataSource) {
        super(dataSource);
    }

    @Override
    protected void doBegin(Object transaction, TransactionDefinition definition) {
        if (definition.getName().contains("package_substring")) {
            log.debug(String.format("Init. Transaction for MyApp Service: %s : %s", definition.getName(),
                    definition));
            TransactionSynchronizationManager.setCurrentTransactionIsolationLevel(
                    definition.getIsolationLevel() != TransactionDefinition.ISOLATION_DEFAULT
                    ? definition.getIsolationLevel()
                            : null);
            TransactionSynchronizationManager.setCurrentTransactionReadOnly(definition.isReadOnly());
            TransactionSynchronizationManager.setCurrentTransactionName(definition.getName());
        }
        super.doBegin(transaction, definition);
    }
}

As we can see, I am extending DataSourceTransactionManager and updating TransactionSynchronizationManager with the new TransactionDefinition before acquiring a connection for the JDBC transaction. This way we will always get the updated value in AbstractRoutingDataSource.determineCurrentLookupKey()

@jhoeller jhoeller changed the title Move synchronization point creation just before transaction begins to enable routing datasources to act on transaction definition [SPR-16876] Enable routing datasources to act on transaction definition read-only flag [SPR-16876] Nov 24, 2023
@jhoeller jhoeller added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 24, 2023
@jhoeller jhoeller self-assigned this Nov 24, 2023
@jhoeller jhoeller added this to the 6.1.2 milestone Nov 24, 2023
@jhoeller
Copy link
Contributor

This is a close sibling to #19688, just with the initial case being the read-only flag. We mean to deal with these in combination for a JDBC theme in the 6.1.2 release.

As outlined on #19688, I am inclined to address the isolation level scenario here through an extension of SmartDataSource with a dedicated getConnection(int isolationLevel, boolean readOnly) method that DataSourceTransactionManager can call if the target DataSource implements it. IsolationLevelDataSourceRouter and a similar routing DataSource based on @Transactional(readOnly=...) can then take this into account directly when specified. This would allow us to preserve the semantics of our existing transaction synchronization arrangement which we cannot easily bend towards earlier exposure.

@jhoeller
Copy link
Contributor

jhoeller commented Dec 1, 2023

Experimenting with a few scenarios here, such an extension to SmartDataSource is rather involved in more complex setups, e.g. behind a JPA provider. In comparison to that, LazyConnectionDataSourceProxy is actually the simplest solution out: configuring both your transaction manager and your data access setup (JdbcTemplate or JPA setup) with a LazyConnectionDataSourceProxy conveniently allows for routing datasources to pick up a late-bound current isolation level within the transaction. And this comes with the extra benefit that connection contention is minimized and even avoided completely (with no connection ever fetched) for "empty" transactions which is quite common with JPA queries that can be answered from a cache.

As a consequence, I am going to turn this ticket into a documentation ticket for LazyConnectionDataSourceProxy. Maybe we'll even introduce a dedicated DataSourceRouter class for read-only vs read-write connections in the 6.1.2 timeframe since this is apparently as common a scenario as routing by isolation level.

@jhoeller jhoeller changed the title Enable routing datasources to act on transaction definition read-only flag [SPR-16876] Document LazyConnectionDataSourceProxy usage with routing datasource to act on transaction definition read-only flag Dec 1, 2023
@jhoeller jhoeller added type: documentation A documentation task and removed type: enhancement A general enhancement labels Dec 1, 2023
@jhoeller jhoeller changed the title Document LazyConnectionDataSourceProxy usage with routing datasource to act on transaction definition read-only flag Document LazyConnectionDataSourceProxy setup for routing datasource to act on transaction definition read-only flag Dec 1, 2023
@krebsba4
Copy link

Doesn't seem to be working as expected in Spring 6.1.4

Using an AbstractRoutingDatasource wrapped with LazyConnectionDataSourceProxy (and setting the read-only datasource) during the initial DataSourceTransactionManager.doGetTransaction the DataSourceTransactionObject object is not marked as read-only, only the TransactionDefinition is. When this delegates to the AbstractRoutingDatasource it returns the read-write DataSource as the current TransactionSynchronizationManager is still not marked as read-only. It will still make a connection to the read-write DataSource and THEN set the read-only flag in the TransactionSynchronizationManager.

To make things more confusing, it looks like after this the LazyConnectionDataSourceProxy is doing its routing and returning the correct DataSource.

I was under the assumption that I wouldn't see any connection made to the read-write as the transaction should have been marked in the TransactionSynchronizationManager as read-only on transaction start, but that looks to not be the case. Only when using the work around mentioned by kanakharaharsh above does the transaction NOT start a connection to the read-write and only operates on the read-only.

@jhoeller
Copy link
Contributor

The DataSourceTransactionManager interaction will still grab a Connection handle as usual, but with LazyConnectionDataSourceProxy inbetween, it should grab a lazy Connection from there, call a few methods to prepare that lazy Connection handle but not actually trigger a target Connection before an actual statement is being executed.

Also, as of 6.1.2, LazyConnectionDataSourceProxy has a setReadOnlyDataSource itself. There should not be a need for a separate routing DataSource for that standard use case anymore.

@krebsba4
Copy link

Ok, after removing my AbstractRoutingDatasource I can see the connection is wrapped with the LazyConnectionProxy but see in my debug logs that it looks like my writer is starting and grabbing a connection from the pool. I don't think it's doing any action on that connection but will try to verify.

com.zaxxer.hikari.HikariDataSource       : writerDatasourcePool - Starting...
com.zaxxer.hikari.pool.HikariPool        : writerDatasourcePool - Added connection org.postgresql.jdbc.PgConnection@4c484e88
com.zaxxer.hikari.HikariDataSource       : writerDatasourcePool - Start completed.
o.s.j.d.DataSourceTransactionManager     : Acquired Connection [Lazy Connection proxy for target DataSource [HikariDataSource (writerDatasourcePool)]] for JDBC transaction
o.s.jdbc.datasource.DataSourceUtils      : Setting JDBC Connection [Lazy Connection proxy for target DataSource [HikariDataSource (writerDatasourcePool)]] read-only
o.s.j.d.DataSourceTransactionManager     : Switching JDBC Connection [Lazy Connection proxy for target DataSource [HikariDataSource (writerDatasourcePool)]] to manual commit

@OrangeDog
Copy link

That's because those lines are logged before a connection is actually made, so the toString() just delegates to the default DataSource. Unfortunately, Hikari doesn't have any logging to tell you when a connection is borrowed, nor from which pool.

I can confirm, via use of the debugger, that HikariDataSource.getConnection() is only called on the read-only DataSource.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) type: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

5 participants