Solr
  1. Solr
  2. SOLR-4992

Solr queries don't propagate Java OutOfMemoryError back to the JVM

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.3.1
    • Fix Version/s: 4.7, 6.0
    • Component/s: search, SolrCloud, update
    • Labels:
      None

      Description

      Solr (specifically SolrDispatchFilter.doFilter() but there might be other places) handle generic java.lang.Throwable errors but that "hides" OutOfMemoryError scenarios.

      IndexWriter does this too but that has a specific exclusion for OOM scenarios and handles them explicitly (stops committing and just logs to the transaction log).

      Example Stack trace:
      2013-06-26 19:31:33,801 [qtp632640515-62] ERROR
      solr.servlet.SolrDispatchFilter Q:22 -
      null:java.lang.RuntimeException: java.lang.OutOfMemoryError: Java heap
      space
      at org.apache.solr.servlet.SolrDispatchFilter.sendError(SolrDispatchFilter.java:670)
      at org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:380)
      at org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:155)
      at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1423)
      at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:450)
      at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:138)
      at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:564)
      at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:213)
      at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1083)
      at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:379)
      at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:175)
      at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1017)
      at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:136)
      at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:258)
      at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:109)
      at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97)
      at org.eclipse.jetty.server.Server.handle(Server.java:445)
      at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:260)
      at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:225)
      at org.eclipse.jetty.io.AbstractConnection$ReadCallback.run(AbstractConnection.java:358)
      at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:596)
      at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:527)
      at java.lang.Thread.run(Thread.java:722)
      Caused by: java.lang.OutOfMemoryError: Java heap space
      
      
      1. SOLR-4992.patch
        62 kB
        Mark Miller
      2. SOLR-4992.patch
        61 kB
        Mark Miller
      3. SOLR-4992.patch
        41 kB
        Mark Miller

        Issue Links

          Activity

          Hide
          Steve Rowe added a comment -

          Bulk move 4.4 issues to 4.5 and 5.0

          Show
          Steve Rowe added a comment - Bulk move 4.4 issues to 4.5 and 5.0
          Hide
          Mark Miller added a comment -

          We like to catch those throwables because of things like assertion errors - we still want to close resources, etc.

          One idea is to always do:

               } catch (Throwable t) {
                  if (t instanceof OutOfMemoryError) {
                    throw (OutOfMemoryError)t;
                  }
          

          But that is difficult to enforce over time.

          Show
          Mark Miller added a comment - We like to catch those throwables because of things like assertion errors - we still want to close resources, etc. One idea is to always do: } catch (Throwable t) { if (t instanceof OutOfMemoryError) { throw (OutOfMemoryError)t; } But that is difficult to enforce over time.
          Hide
          Ramkumar Aiyengar added a comment -

          Doesn't `finally` offer a cleaner way to do that?

          Show
          Ramkumar Aiyengar added a comment - Doesn't `finally` offer a cleaner way to do that?
          Hide
          Mark Miller added a comment -

          I think for some cases it does.

          It's a little painful for closing collections of items or if you need to close a lot of items serially, nesting one finally after another.

          It does seem like we should probably stop catching throwable in most cases anyway. In some cases, you might still catch it, log something and then throw if instanceOf Error I think, but in general, we need OOM's to bubble up. It seems like the best way to ensure that is to try and minimize use of catch (Throwable...

          Show
          Mark Miller added a comment - I think for some cases it does. It's a little painful for closing collections of items or if you need to close a lot of items serially, nesting one finally after another. It does seem like we should probably stop catching throwable in most cases anyway. In some cases, you might still catch it, log something and then throw if instanceOf Error I think, but in general, we need OOM's to bubble up. It seems like the best way to ensure that is to try and minimize use of catch (Throwable...
          Hide
          Mark Miller added a comment -

          Here is a first (somewhat conservative) pass at improving the situation.

          Show
          Mark Miller added a comment - Here is a first (somewhat conservative) pass at improving the situation.
          Hide
          Mark Miller added a comment -

          Here is a more complete patch. I'd like to commit shortly and iterate if their are any concerns - this covers a lot of classes and will be outdated very quickly.

          Show
          Mark Miller added a comment - Here is a more complete patch. I'd like to commit shortly and iterate if their are any concerns - this covers a lot of classes and will be outdated very quickly.
          Hide
          Yonik Seeley added a comment -

          Are there response format implications to this?
          For example, on certain errors (like OOM) will I still get a JSON response format back (assuming that's the writer I'm using)?

          Show
          Yonik Seeley added a comment - Are there response format implications to this? For example, on certain errors (like OOM) will I still get a JSON response format back (assuming that's the writer I'm using)?
          Hide
          Mark Miller added a comment -

          In most cases I still try and log or send the same response before propagating the Throwable. Just fixed some stuff around the SolrDispatcher here on a closer review.

          Show
          Mark Miller added a comment - In most cases I still try and log or send the same response before propagating the Throwable. Just fixed some stuff around the SolrDispatcher here on a closer review.
          Hide
          David Smiley added a comment -

          FYI I thought I'd point out some utilities Google Guava has in this regard:
          http://code.google.com/p/guava-libraries/wiki/ClosingResourcesExplained "Closer", as well as various utility methods on the "Throwables" class.

          Show
          David Smiley added a comment - FYI I thought I'd point out some utilities Google Guava has in this regard: http://code.google.com/p/guava-libraries/wiki/ClosingResourcesExplained "Closer", as well as various utility methods on the "Throwables" class.
          Hide
          Mark Miller added a comment -

          Nice David.

          Show
          Mark Miller added a comment - Nice David.
          Hide
          Mark Miller added a comment -

          I'm going to commit my current progress. I think it's all okay and we can make improvements on it over time.

          Show
          Mark Miller added a comment - I'm going to commit my current progress. I think it's all okay and we can make improvements on it over time.
          Hide
          ASF subversion and git services added a comment -

          Commit 1557778 from Mark Miller in branch 'dev/trunk'
          [ https://svn.apache.org/r1557778 ]

          SOLR-4992: Solr eats OutOfMemoryError exceptions in many cases.

          Show
          ASF subversion and git services added a comment - Commit 1557778 from Mark Miller in branch 'dev/trunk' [ https://svn.apache.org/r1557778 ] SOLR-4992 : Solr eats OutOfMemoryError exceptions in many cases.
          Hide
          ASF subversion and git services added a comment -

          Commit 1557783 from Mark Miller in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1557783 ]

          SOLR-4992: Solr eats OutOfMemoryError exceptions in many cases.

          Show
          ASF subversion and git services added a comment - Commit 1557783 from Mark Miller in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1557783 ] SOLR-4992 : Solr eats OutOfMemoryError exceptions in many cases.
          Hide
          Mark Miller added a comment -

          Thanks Daniel - let's do further specific work in new issues.

          Show
          Mark Miller added a comment - Thanks Daniel - let's do further specific work in new issues.

            People

            • Assignee:
              Mark Miller
              Reporter:
              Daniel Collins
            • Votes:
              1 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development