Uploaded image for project: 'Flume'
  1. Flume
  2. FLUME-2997

Fix flaky junit test in SpillableMemoryChannel

    Details

    • Type: Test
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.7.0
    • Fix Version/s: 1.8.0
    • Component/s: None
    • Labels:
      None

      Description

      testParallelSingleSourceAndSink sometimes trigger an edge case scenario if sinks are slower than sources. In such situations the channel can get full thus uncaught ChannelFullException breaks the test. Since testCapacityWithOverflow was designed to cover such edge-case scenario already we can safely fix the test by increasing the channel capacity to make sure it won't get full.

      1. FLUME-2997.patch
        0.9 kB
        Attila Simon
      2. FLUME-2997-1.patch
        1 kB
        Attila Simon

        Activity

        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Jenkins build Flume-trunk-hbase-1 #222 (See https://builds.apache.org/job/Flume-trunk-hbase-1/222/)
        FLUME-2997. Fix flaky test in SpillableMemoryChannel (bessbd: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=7c4b2fd3e88f038dea3bdae01eb864547770ad48)

        • (edit) flume-ng-channels/flume-spillable-memory-channel/src/test/java/org/apache/flume/channel/TestSpillableMemoryChannel.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Jenkins build Flume-trunk-hbase-1 #222 (See https://builds.apache.org/job/Flume-trunk-hbase-1/222/ ) FLUME-2997 . Fix flaky test in SpillableMemoryChannel (bessbd: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=7c4b2fd3e88f038dea3bdae01eb864547770ad48 ) (edit) flume-ng-channels/flume-spillable-memory-channel/src/test/java/org/apache/flume/channel/TestSpillableMemoryChannel.java
        Hide
        bessbd Bessenyei Balázs Donát added a comment -

        Attila Simon: thank you for the patch!

        Show
        bessbd Bessenyei Balázs Donát added a comment - Attila Simon : thank you for the patch!
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 7c4b2fd3e88f038dea3bdae01eb864547770ad48 in flume's branch refs/heads/trunk from Attila Simon
        [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=7c4b2fd ]

        FLUME-2997. Fix flaky test in SpillableMemoryChannel

        When the mock sinks are slower than sources, testParallelSingleSourceAndSink sometimes fails.
        In such situations the channel can get full, thus uncaught ChannelFullException breaks the test.

        Since testCapacityWithOverflow was designed to cover such a scenario, we
        can safely fix the test by increasing the channel capacity to make sure it won't get full.

        This commit does the previously mentioned change.

        Reviewers: Bessenyei Balázs Donát

        (Attila Simon via Bessenyei Balázs Donát)

        Show
        jira-bot ASF subversion and git services added a comment - Commit 7c4b2fd3e88f038dea3bdae01eb864547770ad48 in flume's branch refs/heads/trunk from Attila Simon [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=7c4b2fd ] FLUME-2997 . Fix flaky test in SpillableMemoryChannel When the mock sinks are slower than sources, testParallelSingleSourceAndSink sometimes fails. In such situations the channel can get full, thus uncaught ChannelFullException breaks the test. Since testCapacityWithOverflow was designed to cover such a scenario, we can safely fix the test by increasing the channel capacity to make sure it won't get full. This commit does the previously mentioned change. Reviewers: Bessenyei Balázs Donát (Attila Simon via Bessenyei Balázs Donát)
        Hide
        bessbd Bessenyei Balázs Donát added a comment -

        I'm about to commit this

        Show
        bessbd Bessenyei Balázs Donát added a comment - I'm about to commit this
        Hide
        bessbd Bessenyei Balázs Donát added a comment -

        +1, LGTM

        Show
        bessbd Bessenyei Balázs Donát added a comment - +1, LGTM
        Hide
        sati Attila Simon added a comment -

        I think I missed some description of this change so let me provide it now: Originally this test case aimed to verify behaviour about having multiple mocked sources putting events on the same SpillableMemoryChannel while multiple mocked sinks drains those events. Given the mock implementation didn't do retry if sources are faster than the sinks the channel will get full. Since full channel doesn't accept new events the mocked sources fail their task by raising a runtime exception. This edge case should be avoided thus the increased capacity (which makes sure that channel cannot be full). The "channel is full" edge case is already covered by another test.

        Those System.out.println-s were replaced by asserts to make the function to became a test. It wasn't testing anything before. It only ran to the end without checking anything. (The flakiness was caused by a ChannelFullException). I guess the original author used those System.out.println-s for manual testing (purely looking at the numbers). This change also made that manual step automatic.

        Show
        sati Attila Simon added a comment - I think I missed some description of this change so let me provide it now: Originally this test case aimed to verify behaviour about having multiple mocked sources putting events on the same SpillableMemoryChannel while multiple mocked sinks drains those events. Given the mock implementation didn't do retry if sources are faster than the sinks the channel will get full. Since full channel doesn't accept new events the mocked sources fail their task by raising a runtime exception. This edge case should be avoided thus the increased capacity (which makes sure that channel cannot be full). The "channel is full" edge case is already covered by another test. Those System.out.println-s were replaced by asserts to make the function to became a test. It wasn't testing anything before. It only ran to the end without checking anything. (The flakiness was caused by a ChannelFullException). I guess the original author used those System.out.println-s for manual testing (purely looking at the numbers). This change also made that manual step automatic.
        Hide
        bessbd Bessenyei Balázs Donát added a comment -

        Attila Simon: thank you for the patch!

        Can you please explain why you think it's better to remove the System.out.println-s we had in the test?

        Show
        bessbd Bessenyei Balázs Donát added a comment - Attila Simon : thank you for the patch! Can you please explain why you think it's better to remove the System.out.println-s we had in the test?
        Hide
        sati Attila Simon added a comment -

        missed the asserts from prev version, mvn clean install builds the project, unit test passes for the module

        Show
        sati Attila Simon added a comment - missed the asserts from prev version, mvn clean install builds the project, unit test passes for the module
        Hide
        sati Attila Simon added a comment -

        Code change in the attached patch sets up a channel in the test with a capacity enough to store the produced events.

        Show
        sati Attila Simon added a comment - Code change in the attached patch sets up a channel in the test with a capacity enough to store the produced events.

          People

          • Assignee:
            sati Attila Simon
            Reporter:
            sati Attila Simon
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development