Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: v1.4.0
    • Fix Version/s: v1.5.0
    • Component/s: Sinks+Sources
    • Labels:
      None

      Description

      Lines 251-252 in the DatasetSink are a potential deadlock: if the transaction throws an exception, then the writer lock is not released, but the same thread tries to lock in the error handling.

      While the simplest solution is to move those two lines inside the try/finally statement, I think we can actually remove the lock completely by reverting to the original version that rolled the files in the process() method. The original concern about that design was that there needs to be some guarantee that all files will be rolled. Because the SinkRunner has a max backoff, there is a guaranteed maximum amount of time between calls to process.

      1. 0002-FLUME-2320-Fix-deadlock-by-removing-writer-lock.patch
        11 kB
        Ryan Blue
      2. FLUME-2320-1.patch
        11 kB
        Ryan Blue
      3. FLUME-2320-2.patch
        11 kB
        Ryan Blue

        Activity

        Hide
        rdblue Ryan Blue added a comment -

        This patch implements the proposed solution: it removes the roll thread and writer lock, and moves the rolling logic into the process method. It also adds a note in the documentation that file rolling may be slightly delayed, in some cases up to 5 seconds.

        Show
        rdblue Ryan Blue added a comment - This patch implements the proposed solution: it removes the roll thread and writer lock, and moves the rolling logic into the process method. It also adds a note in the documentation that file rolling may be slightly delayed, in some cases up to 5 seconds.
        Hide
        hshreedharan Hari Shreedharan added a comment -

        Ryan Blue - This looks good. Do you mind adding an @VisibleForTesting annotation to the roll() method?

        Show
        hshreedharan Hari Shreedharan added a comment - Ryan Blue - This looks good. Do you mind adding an @VisibleForTesting annotation to the roll() method?
        Hide
        rdblue Ryan Blue added a comment -

        Corrected patch with @VisibleForTesting.

        Show
        rdblue Ryan Blue added a comment - Corrected patch with @VisibleForTesting.
        Hide
        rdblue Ryan Blue added a comment -

        The real patch – I uploaded the wrong one. They're identical, but this one is named correctly.

        Show
        rdblue Ryan Blue added a comment - The real patch – I uploaded the wrong one. They're identical, but this one is named correctly.
        Hide
        hshreedharan Hari Shreedharan added a comment -

        +1.

        Show
        hshreedharan Hari Shreedharan added a comment - +1.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit ec40929596b06b4ec608a8199f6be2f807a865c6 in flume's branch refs/heads/trunk from Hari Shreedharan
        [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=ec40929 ]

        FLUME-2320. Fixed Deadlock in DatasetSink

        (Ryan Blue via Hari Shreedharan)

        Show
        jira-bot ASF subversion and git services added a comment - Commit ec40929596b06b4ec608a8199f6be2f807a865c6 in flume's branch refs/heads/trunk from Hari Shreedharan [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=ec40929 ] FLUME-2320 . Fixed Deadlock in DatasetSink (Ryan Blue via Hari Shreedharan)
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 2fc69f54b45588a40fca06942d4530eb17ce51a0 in flume's branch refs/heads/flume-1.5 from Hari Shreedharan
        [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=2fc69f5 ]

        FLUME-2320. Fixed Deadlock in DatasetSink

        (Ryan Blue via Hari Shreedharan)

        Show
        jira-bot ASF subversion and git services added a comment - Commit 2fc69f54b45588a40fca06942d4530eb17ce51a0 in flume's branch refs/heads/flume-1.5 from Hari Shreedharan [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=2fc69f5 ] FLUME-2320 . Fixed Deadlock in DatasetSink (Ryan Blue via Hari Shreedharan)
        Hide
        hshreedharan Hari Shreedharan added a comment -

        Committed!Thanks Ryan!

        Show
        hshreedharan Hari Shreedharan added a comment - Committed!Thanks Ryan!
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in flume-trunk #552 (See https://builds.apache.org/job/flume-trunk/552/)
        FLUME-2320. Fixed Deadlock in DatasetSink (hshreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo/?p=flume.git&a=commit&h=ec40929596b06b4ec608a8199f6be2f807a865c6)

        • flume-ng-sinks/flume-dataset-sink/src/test/java/org/apache/flume/sink/kite/TestDatasetSink.java
        • flume-ng-sinks/flume-dataset-sink/src/main/java/org/apache/flume/sink/kite/DatasetSinkConstants.java
        • flume-ng-sinks/flume-dataset-sink/src/main/java/org/apache/flume/sink/kite/DatasetSink.java
        • flume-ng-doc/sphinx/FlumeUserGuide.rst
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in flume-trunk #552 (See https://builds.apache.org/job/flume-trunk/552/ ) FLUME-2320 . Fixed Deadlock in DatasetSink (hshreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo/?p=flume.git&a=commit&h=ec40929596b06b4ec608a8199f6be2f807a865c6 ) flume-ng-sinks/flume-dataset-sink/src/test/java/org/apache/flume/sink/kite/TestDatasetSink.java flume-ng-sinks/flume-dataset-sink/src/main/java/org/apache/flume/sink/kite/DatasetSinkConstants.java flume-ng-sinks/flume-dataset-sink/src/main/java/org/apache/flume/sink/kite/DatasetSink.java flume-ng-doc/sphinx/FlumeUserGuide.rst

          People

          • Assignee:
            rdblue Ryan Blue
            Reporter:
            rdblue Ryan Blue
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development