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

HibernateTransactionManager should allow holdability of ResultSet into the View layer [SPR-12349] #16954

Closed
spring-projects-issues opened this issue Oct 20, 2014 · 10 comments
Assignees
Labels
in: data type: enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Oct 20, 2014

Marko Topolnik opened SPR-12349 and commented

I would like an old topic to be reconsidered in the light of new developments. One of the issues dealing with the topic was #6715 and it was resolved such that the Hibernate session releases its JDBC connection upon the completion of a transaction.

In the context of a REST Web Service, I want to use the standard MVC mechanism where a MessageConverter is dispatched to handle an object returned from a @Controller, but I want that mechanism to support Java 8 Streams and let the MesageConverter consume the stream while producing the response. The stream source would be Hibernate's ScrollableResults. Note the following advantages:

  • O(1) space complexity instead of O(N);
  • transaction semantics stay intact: the transaction is committed when exiting the service layer;
  • all the business logic stays within the service layer. The returned Stream wraps the logic needed to transform the SQL results into response DTOs;
  • the transformation can be automatically parallelized with no thread safety issues on the JDBC level (Spliterator's programming model is single-threaded).

I have set up the following:

  1. MappingJackson2HttpMessageConverter enriched with a JsonSerializer which handles a Java 8 Stream;
  2. a custom ScrollableResultSpliterator needed to wrap ScrollableResults into a Stream;
  3. OpenSessionInViewInterceptor needed to keep the Hibernate session open within the MessageConverter;
  4. set hibernate.connection.release_mode to ON_CLOSE;
  5. ensure that the JDBC connection has the necessary ResultSet holdability: con.setHoldability(ResultSet.HOLD_CURSORS_OVER_COMMIT)

The final stumbling point I have encountered is the policy used by HibernateTransactionManager on transaction commit: unless the underlying session is "Hibernate-managed", it will disconnect() it, closing my cursor along with everything else. Such a policy is useful in some special scenarios, specifically "conversation-scoped sessions", which are far removed from my requirements.

Since I regard my approach as "the right way" to generate ResultSet-backed REST responses, and since the Streams API makes this approach very convenient, I would like to have support for it from Spring.


Affects: 4.1.1

Reference URL: http://stackoverflow.com/questions/26324112/trouble-using-scrollableresults-backed-stream-as-return-type-in-spring-mvc

Issue Links:

  • #13659 Hibernate4 version of SpringSessionContext.currentSession() does not create a session if TransactionSynchronizationManager does not contain one

Referenced from: commits 49f3a6b

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 22, 2014

Marko Topolnik commented

After some discussion with user bestsss on Stack Overflow, an idea came up that it would be enough support from Spring if it just allowed more customizability of HibernateTransactionManager code. Specifically HibernateTransactionObject should be made protected, its instantiation in doGetTransaction done via a protected factory method, and finally the session disconnection concern should be handled by a new strategy method in HibernateTransactionObject.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 23, 2014

Juergen Hoeller commented

Aside from factoring out the disconnect-on-completion logic into a protected template method (which makes sense in any case), we could support your scenario with a dedicated "allowResultSetAccessAfterCompletion" bean property, setting both the holdability on the prepared JDBC Connection and skipping the disconnect on completion. Would this make sense for your purposes?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 23, 2014

Marko Topolnik commented

Actually that would be great, and more than I asked for initially. It would relieve me from having to set holdability myself for each transaction.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 23, 2014

Marko Topolnik commented

If there shall be any interest, I can contribute my plumbing code (the JsonSerializer and ScrollableResultSpliterator).

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 23, 2014

Juergen Hoeller commented

I've introduced this as "allowResultAccessAfterCompletion" bean property on our HibernateTransactionManager variant for Hibernate 4. This will be available in the upcoming 4.1.2 snapshot... Please give it a try!

As for the ScrollableResults adapters, sure, feel free to create a separate issue for it, attaching those files (or adding a pull request). We intend to research some general Stream support for Spring Framework 4.2 (#16789), and this seems like a good fit for that timeframe...

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 24, 2014

Marko Topolnik commented

I gave it a try, it works perfectly for my production use case.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Nov 5, 2014

Marko Topolnik commented

I have further considered some performance issues with holdable cursors. It turns out that regarding them the mainstream databases pretty much cover the entire spectrum of possibilities:

  • Oracle doesn't even support non-holdable cursors;
  • MS SQL Server supports non-holdability, but prefers holdability;
  • Postgresql implements holdability by copying the entire result set into a temporary area;
  • Postgresql JDBC driver, however, doesn't make use of that: it creates a non-holdable cursor and eagerly prefetches the entire result set into JVM heap (so-called "client-side" cursor);
  • MySQL flat-out does not support them.

Given the above, applications working with Postgresql will be interested not to pay the cost of holdabilty when they don't need it. This makes me conclude that setting result set holdability should be moved to the OSIV interceptor:

  • these can be selectively applied just to the URL patterns where they are needed;
  • more to the point, it is really the OSIV which needs the setting so it makes sense to put it there.

Theoretically, this could go even further: the transaction manager could prehaps find out whether OSIV behavior is enabled for the current transaction and set holdability according to that. If this were possible, the explicit setting on the transaction manager bean would very rarely need to be used.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Nov 5, 2014

Marko Topolnik commented

I'll also add one caveat I have learned about: the method setHoldability may throw an exception if the driver does not support the requested setting. For example, Oracle JDBC driver throws an exception if non-holdable result sets are requested; there is an outstanding issue on MySQL whose fix would make its driver throw an exception if holdable result sets are requested.

Since OSIV may also be legitimately used when holdable cursors are not needed (Hibernate's lazy-fetched collections accessed in the view layer), it should not be eager to fail due to the inability to set holdability. This leads to the need to catch the exception and just log it (at DEBUG level because otherwise the logs would be full of these warnings).

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Nov 6, 2014

Juergen Hoeller commented

Thanks for the summary of holdability support in common databases here... I wasn't aware of these limitations!

As for HibernateTransactionManager actually applying the "allowResultSetAccessAfterCompletion" setting, we are checking for a non-new Session already and only call setHoldability in that case... This is basically checking for an OSIV scenario since otherwise transactions will always have new Sessions going into them.

And when using OSIV without holdable cursors, you wouldn't set "allowResultSetAccessAfterCompletion" to "true" in the first place, I suppose? I don't really see a scenario where you would want holdable ResultSets if possible and silently ignore their non-availability... since an application either relies on them or not.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Nov 7, 2014

Marko Topolnik commented

Yes, I missed this part that the holdability setting is only applied when encountering a pre-existing session. This means that my "idea" how it could be done is already the way it is done. It also means that my comment about failures doesn't apply: you either want holdability and skipping the session disconnection --- or you don't need either. So if you can't get holdability, there is no reason not to disconnect the session at commit, therefore there's no reason to activate the "allowResultSetAccessAfterCompletion" in that case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants