66

Important note: this has been accepted as a Spring issue with a target fix version of 4.1.2.

My goal is to achieve O(1) space complexity when generating an HTTP response from Hibernate's ScrollableResults. I want to keep the standard mechanism where a MessageConverter is dispatched to handle an object returned from a @Controller. 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).

Additionally, I need a database which supports that kind of holdability. PostgreSQL is such a database and I have no trouble with this.

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.

I have managed to work around this with a bad hack: I had to override the offending method with a method which is effectively a copy-paste of the original except for the removed disconnect() call, but it must resort to reflection to access private API.

public class NoDisconnectHibernateTransactionManager extends HibernateTransactionManager
{
  private static final Logger logger = LoggerFactory.getLogger(NoDisconnectHibernateTransactionManager.class);

  public NoDisconnectHibernateTransactionManager(SessionFactory sf) { super(sf); }

  @Override
  protected void doCleanupAfterCompletion(Object transaction) {
    final JdbcTransactionObjectSupport txObject = (JdbcTransactionObjectSupport) transaction;
    final Class<?> c = txObject.getClass();
    try {
      // Remove the session holder from the thread.
      if ((Boolean)jailBreak(c.getMethod("isNewSessionHolder")).invoke(txObject))
        TransactionSynchronizationManager.unbindResource(getSessionFactory());

      // Remove the JDBC connection holder from the thread, if exposed.
      if (getDataSource() != null)
        TransactionSynchronizationManager.unbindResource(getDataSource());

      final SessionHolder sessionHolder = (SessionHolder)jailBreak(c.getMethod("getSessionHolder")).invoke(txObject);
      final Session session = sessionHolder.getSession();
      if ((Boolean)jailBreak(HibernateTransactionManager.class.getDeclaredField("prepareConnection")).get(this)
          && session.isConnected() && isSameConnectionForEntireSession(session))
      {
        // We're running with connection release mode "on_close": We're able to reset
        // the isolation level and/or read-only flag of the JDBC Connection here.
        // Else, we need to rely on the connection pool to perform proper cleanup.
        try {
          final Connection con = ((SessionImplementor) session).connection();
          DataSourceUtils.resetConnectionAfterTransaction(con, txObject.getPreviousIsolationLevel());
        }
        catch (HibernateException ex) {
          logger.debug("Could not access JDBC Connection of Hibernate Session", ex);
        }
      }
      if ((Boolean)jailBreak(c.getMethod("isNewSession")).invoke(txObject)) {
        logger.debug("Closing Hibernate Session [{}] after transaction",  session);
        SessionFactoryUtils.closeSession(session);
      }
      else {
        logger.debug("Not closing pre-bound Hibernate Session [{}] after transaction", session);
        if (sessionHolder.getPreviousFlushMode() != null)
          session.setFlushMode(sessionHolder.getPreviousFlushMode());
      }
      sessionHolder.clear();
    }
    catch (ReflectiveOperationException e) { throw new RuntimeException(e); }
  }

  static <T extends AccessibleObject> T jailBreak(T o) { o.setAccessible(true); return o; }
}

Since I regard my approach as the "right way" to generate ResultSet-backed responses, and since the Streams API makes this approach very convenient, I would like to solve this in a supported way.

Is there a way to get the same behavior without my hack? If not, would this be a good thing to request via Spring's Jira?

Community
  • 1
  • 1
Marko Topolnik
  • 179,046
  • 25
  • 276
  • 399
  • 1
    out of curiosity, what stops you from hacking the source and submitting a patch? Personally I have no troubles going into the source, keep the patches and always build from source any open source project. On an unrelated note: having scrollable result sets moves the space complexity into the database instead of the client, if you have more clients that can concurrently execute similar requests the solution to keep the data on the client is more tempting. – bestsss Oct 21 '14 at 12:12
  • What stops me is the fact that I have currently no idea how that improvement should look like. It's very complicated maze of parameters and I do not want to impose any simplistic, narrow-sighted solutions. – Marko Topolnik Oct 21 '14 at 12:15
  • About moving the space complexity... yes, I am indeed aware of that concern, but it is contingent on the assumption that the transfer to the client will be much slower than the transfer from DB to the REST service. If the transfer to the client is comparably fast, then the DB is pretty much in the same position either way. And do note that my approach has an inherent speed benefit: elimination of latency while copying from DB to the middle tier. – Marko Topolnik Oct 21 '14 at 12:17
  • Thinking further, the DB can reclaim the space as soon as the cursor moves over a record. Therefore as the results are being served, occupancy goes down linearly (or at least has the potential to do so). This does not happen with data structures typically used in Java. The database could also implement a copy-on-write strategy, which would mean that in the uncontended case there is no storage overhead. – Marko Topolnik Oct 21 '14 at 12:23
  • 1
    About local patches... that would create really hard-core requirements on the whole build system. From just having Maven dependencies, to a completely custom Maven repository with custom artifact ids, etc. Not worth it by a long shot. – Marko Topolnik Oct 21 '14 at 12:27
  • *This does not happen with data structures typically used in Java.* It's easily implementable with iterator over an ArrayList and calling `set(null)` after `next()`. Still the memory footprint is pretty much the same. CoW is a fair assessment but I wonder if anyone would actually do it as it'd require to explicitly keep the row version in the resulset. Later I will look into the problem to see if I can come up with anything better. – bestsss Oct 21 '14 at 15:11
  • You are right that it is easily implementable *in principle*, but in practice it isn't implemented that way and it is not simple to extend `ArrayList` such that it has this behavior. Its iterator is a private class and has access to private state. It is clearly designed against extension. So getting this in practice would require quite a lot of cumbersome work (although I guess a naive Iterator implementation with slightly less efficiency would be easy). – Marko Topolnik Oct 21 '14 at 15:17
  • Few notes (1st observation) `HibernateTransactionObject` should be a static class - very reasonable request imo. Then it should be protected not private and `HibernateTransactionManager` should use a protected factory method to create the instance. Very reasonable improvements, and trivial - I guess you should be able to push it. – bestsss Oct 21 '14 at 15:30
  • I see, you would go for enough extensiblility to make my posted method implementable without reflection. However, I would prefer not to have to repeat the complete logic of that method with just the small delta. That logic is subject to change in future versions, so my custom subclass would have to reflect those changes. An approach could be to introduce a new method in `HibernateTransactionObject` which would isolate this concern. Then I would just override your suggested factory to return my slight modification of `HibernateTransactionObject`. – Marko Topolnik Oct 21 '14 at 15:43
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/63428/discussion-between-bestsss-and-marko-topolnik). – bestsss Oct 21 '14 at 16:29
  • If you are not limited to use `Hibernate`, then a little extension of Spring's `JdbcTemplate` would be easy. Let me know if you want to knwo more detail. – Wins Dec 29 '15 at 02:47
  • @wins At the time I was limited to Hibernate, but AFAIR I suggested to the Spring team that the same approach would be useful with plain JDBC as well. Or just switch to jOOQ :) However, if you haven't actually tried it, you may find out it's not doable without changes to the transaction manager. – Marko Topolnik Dec 29 '15 at 06:33
  • @MarkoTopolnik It is possible without changes on transaction manager. I had done it in my previous company. The change was on `JdbcTemplate` or `NamedParameterJdbcTemplate` depending on which API you wanted to have `stream`. So far Spring doesn't support that in their `JdbcTemplate` or its inheritance – Wins Dec 29 '15 at 07:14
  • @wins What you have sounds interesting and useful, but unfortunately off-topic on this page. I suggest you write a self-answered question with your technique. – Marko Topolnik Dec 29 '15 at 07:42
  • @MarkoTopolnik I agree, it is off-topic. That's the reason I said if you're not limited to Hibernate :) – Wins Dec 29 '15 at 07:43
  • 1
    @MarkoTopolnik Has this issue been fixed? If so, I'd suggest posting & accepting it as answer. :) – Seth Mar 16 '16 at 13:19

1 Answers1

1

Cleaning up. As Marko Topolnik had said here

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.

Marsha
  • 187
  • 1
  • 8
  • 2
    I'm not sure about your point, but Spring *did* make changes to support my requirement. The comment you quote is about a very technical detail of how `allowResultSetAccessAfterCompletion` is implemented, and contributes almost nothing useful to the user. It resulted from a misunderstanding I had when reviewing the implementation for the first time. – Marko Topolnik May 18 '15 at 09:16