Apache Drill
  1. Apache Drill
  2. DRILL-640

Limit operator leaks memory when used with exchanges

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.4.0
    • Component/s: None
    • Labels:
      None

      Description

      Limit operator currently invokes kill() when it reaches the desired limit of records. Current semantic of kill() does not stop upstream operators if they are across exchanges. So its the responsibility of the operator to drain out all the record batches and clear the memory, otherwise we will hit memory leaks.

        Activity

        Mehant Baid created issue -
        Show
        Mehant Baid added a comment - github link: https://github.com/mehant/incubator-drill/tree/limit
        Mehant Baid made changes -
        Field Original Value New Value
        Attachment DRILL-640.patch [ 12643443 ]
        Mehant Baid made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Jake Farrell made changes -
        Workflow no-reopen-closed, patch-avail [ 12859865 ] no-reopen-closed, patch-avail, testing [ 12860204 ]
        Mehant Baid made changes -
        Attachment DRILL-640.patch [ 12643443 ]
        Hide
        Mehant Baid added a comment -

        Rebased on latest master.

        Show
        Mehant Baid added a comment - Rebased on latest master.
        Mehant Baid made changes -
        Attachment DRILL-640.patch [ 12643834 ]
        Hide
        Jacques Nadeau added a comment -

        merged in f2ff2c9

        Show
        Jacques Nadeau added a comment - merged in f2ff2c9
        Jacques Nadeau made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Steven Phillips added a comment -

        This solution seems problematic to me. Won't we end up with queries that continue to run needlessly after the requested records have been returned? On large tables, this could result in a lot of disk and network IO.

        I think what we really need to do is solve the problem of terminating a query. Maybe it should happen like this:

        Limit reaches the end, return IterOutcome.NONE. It does not call incoming.next() to drain incoming buffers.
        After the result has been sent to the client, the foreman informs the leaf fragments that they should stop execution if they haven't already. The foreman will inform all intermediate fragments, and finally close the root fragment.

        Part of closing each of these fragment will be ensuring that any batch in the IncomingBuffers queue get released. And we shouldn't have to worry about any batches that arrive after the fragment has closed, because there will no longer be a socket connection to receive them.

        Show
        Steven Phillips added a comment - This solution seems problematic to me. Won't we end up with queries that continue to run needlessly after the requested records have been returned? On large tables, this could result in a lot of disk and network IO. I think what we really need to do is solve the problem of terminating a query. Maybe it should happen like this: Limit reaches the end, return IterOutcome.NONE. It does not call incoming.next() to drain incoming buffers. After the result has been sent to the client, the foreman informs the leaf fragments that they should stop execution if they haven't already. The foreman will inform all intermediate fragments, and finally close the root fragment. Part of closing each of these fragment will be ensuring that any batch in the IncomingBuffers queue get released. And we shouldn't have to worry about any batches that arrive after the fragment has closed, because there will no longer be a socket connection to receive them.
        Jacques Nadeau made changes -
        Fix Version/s 0.4.0 [ 12324963 ]
        Tony Stevenson made changes -
        Workflow no-reopen-closed, patch-avail, testing [ 12860204 ] Drill workflow [ 12933872 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        1m 25s 1 Mehant Baid 05/May/14 23:26
        Patch Available Patch Available Resolved Resolved
        2d 4h 41m 1 Jacques Nadeau 08/May/14 04:07

          People

          • Assignee:
            Mehant Baid
            Reporter:
            Mehant Baid
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development