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

Optimize JdbcIndexedSessionRepository for most updates that don't actually need to modify the session and avoid establishing a transaction/connection #3329

Open
ryanrupp opened this issue Jan 21, 2025 · 0 comments
Labels
status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement

Comments

@ryanrupp
Copy link

ryanrupp commented Jan 21, 2025

Expected Behavior

I would like to minimize the amount of transaction starts/connection acquisitions that occur for the JDBC based repository. I have a use case where essentially the session data doesn't get updated often, really the only thing that changed on it is "last access time" and even for that I have implemented a custom FindByIndexNameSessionRepository extension where it overrides Session.setLastAccessTime so that this only gets updated if the access time being set is within a new minute or in other words I just have minute granularity on this timestamp. This is likely against the http session spec but is a sufficient tradeoff for our use case where we only need to expire sessions after <X> minutes and avoids having to perform many updates to the session row i.e. if the session performs 1000 reads in a minute, only one of those is actually updating last access time (could technically be more during concurrent access but that's the general idea).

The performance behavior that I would like to solve is that in this use case because the session really is only changing at most once per minute, I would like to minimize the overhead of "noops", specifically the update code block here it establishes a new transaction which subsequently will acquire a DB connection from the pool (we have this setup as the default of eager although possibly moving this to lazy could help alleviate?). Then the entire code block effectively no-ops because this.changed is false and we have no other modifications to session attributes.

While I think the minute granularity custom setup I have is niche, there are some other use cases here where I think this could be optimized i.e. I think the session update logic actually gets called at least twice, once in request pre-commit and post-commit (I remember finding a thread explaining why this is to handle post commit added attributes). I would like to essentially move the "delta" logic so that it happens before and then the transaction is only established if there is a delta. I think this can be done in a way that doesn't affect existing usage. I'm willing to implement this if that approach is approved i.e. something like:

interface DeltaAction {

      void apply() throws SQLException;
}

private void save() {
    // update impl

    // get delta changes first
    List<DeltaAction> deltaActions = new ArrayList<>();

    // move the delta logic up here and add any applicable actions based on the in memory delta

    // if actions, execute them now in a transaction
    if (!deltaActions.isEmpty()) {
        JdbcIndexedSessionRepository.this.transactionOperations.executeWithoutResult((status) -> {
            for (DeltaAction action : deltaActions) {
                action.apply();
            }
        }
    }
    
}

Current Behavior
The current behavior establish a transaction (and subsequently a DB connection from the pool) even though there's no actual updates taking place to the session. This can happen also in pre vs post commit I believe i.e. checks this more than once per request.

Context

How has this issue affected you?
What are you trying to accomplish?
Improved performance by avoiding having to acquire a database connection / start a transaction when it's not needed in most cases

What other alternatives have you considered?
I tried using the support in Spring Session to disable transaction management but this doesn't work correctly then if the primary transaction is read-only.

Are you aware of any workarounds?
I didn't try this yet but possibly moving the connection acquisition to lazy as described here but this is likely a wider scoped change (affecting the rest of my application) vs just the session handling piece unless possibly I could wrap the datasource just for the JDBC session repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

1 participant