Flume
  1. Flume
  2. FLUME-1793

Unit test TestElasticSearchLogStashEventSerializer fails with IBM JDK

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: v1.3.0
    • Fix Version/s: v1.6.0
    • Component/s: Sinks+Sources, Test
    • Labels:
    • Environment:

      IBM Java 6 SR 12
      RHEL 6.3

      Description

      TestElasticSearchLogStashEventSerializer fails with IBM JDK:
      testRoundTrip(org.apache.flume.sink.elasticsearch.TestElasticSearchLogStashEventSerializer) Time elapsed: 0.201 sec <<< FAILURE!
      org.junit.ComparisonFailure:expected:<...p/test","@fields":{"[timestamp":"1355151725225","src_path":"/tmp/test","host":"test@localhost","headerNameTwo":"headerValueTwo","source":"flume_tail_src","headerNameOne":"headerValueOne","type":"sometyp]e"}}@@@@@@@@@@@@@@@@...> but was:<...p/test","@fields":{"[src_path":"/tmp/test","headerNameTwo":"headerValueTwo","timestamp":"1355151725225","host":"test@localhost","type":"sometype","source":"flume_tail_src","headerNameOne":"headerValueOn]e"}}@@@@@@@@@@@@@@@@...>

      The values on the "@fields" element are the same as expected, but the order is different. This test case works with SUN Java though - it seems like HashMap on IBM Java and SUN Java return values in different orders, and the test case is counting on order. Order shouldn't matter when using a hashmap though.

      1. FLUME-1793-1.patch
        6 kB
        li xiang
      2. FLUME-1793.patch
        3 kB
        Aline Guedes Pinto

        Issue Links

          Activity

          Aline Guedes Pinto created issue -
          Hide
          Aline Guedes Pinto added a comment -

          Attached FLUME-1793.patch as a suggestion to fix the issue.

          Show
          Aline Guedes Pinto added a comment - Attached FLUME-1793 .patch as a suggestion to fix the issue.
          Aline Guedes Pinto made changes -
          Field Original Value New Value
          Attachment FLUME-1793.patch [ 12561508 ]
          Aline Guedes Pinto made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Labels test-patch
          Fix Version/s v1.3.0 [ 12322140 ]
          Hari Shreedharan made changes -
          Fix Version/s v1.4.0 [ 12323372 ]
          Fix Version/s v1.3.0 [ 12322140 ]
          Hide
          Mike Percy added a comment -

          Thanks Aline, this is a strange one. Can be made deterministic?

          Show
          Mike Percy added a comment - Thanks Aline, this is a strange one. Can be made deterministic?
          Hide
          Mike Percy added a comment -

          This probably won't make it into v1.4.0, clearing fixVersion

          Show
          Mike Percy added a comment - This probably won't make it into v1.4.0, clearing fixVersion
          Mike Percy made changes -
          Fix Version/s v1.4.0 [ 12323372 ]
          Hide
          li xiang added a comment -

          Hi Mike, this patch needs to be applied to 1.4.0, 1.5.0 and 1.6.0, I think.

          Show
          li xiang added a comment - Hi Mike, this patch needs to be applied to 1.4.0, 1.5.0 and 1.6.0, I think.
          Hide
          Edward Sargisson added a comment -

          Hi, just catching up on my emails now.

          Thanks for raising this and adding the patch. It's appreciated.

          IMO, if we have an ordering problem then the assertions should be written to ignore ordering - and not be conditional on the JVM vendor. There's nothing stopping this breaking even in the Oracle JVM.

          If you felt like making that change that would be great. Once it's sorted, please put it into a review in the Apache Review Board.

          Show
          Edward Sargisson added a comment - Hi, just catching up on my emails now. Thanks for raising this and adding the patch. It's appreciated. IMO, if we have an ordering problem then the assertions should be written to ignore ordering - and not be conditional on the JVM vendor. There's nothing stopping this breaking even in the Oracle JVM. If you felt like making that change that would be great. Once it's sorted, please put it into a review in the Apache Review Board.
          Hide
          li xiang added a comment -

          Hi Edward, sorry for my late response.
          As tested, only IBM JDK 1.7 has this problem, not for IBM JDK 1.6. I am studying on how to make the test ignore the order. Thanks !

          Show
          li xiang added a comment - Hi Edward, sorry for my late response. As tested, only IBM JDK 1.7 has this problem, not for IBM JDK 1.6. I am studying on how to make the test ignore the order. Thanks !
          li xiang made changes -
          Attachment FLUME-1793-1 [ 12655535 ]
          li xiang made changes -
          Attachment FLUME-1793-1 [ 12655535 ]
          li xiang made changes -
          Attachment FLUME-1793-1.patch [ 12655536 ]
          Hide
          li xiang added a comment -

          Hi Edward, I updated a new patch FLUME-1793-1.patch. The following 4 changes are included

          1. For testRoundTrip()
          (1) Use GSON, to make assertion ignore ordering
          (2) Add indents when building the expected result, to improve the readability, no logic is changed

          2. For shouldHandleInvalidJSONDuringComplexParsing()
          (1) Use GSON, to make assertion ignore ordering
          (2) Add indents when building the expected result, to improve the readability, no logic is changed

          Show
          li xiang added a comment - Hi Edward, I updated a new patch FLUME-1793 -1.patch. The following 4 changes are included 1. For testRoundTrip() (1) Use GSON, to make assertion ignore ordering (2) Add indents when building the expected result, to improve the readability, no logic is changed 2. For shouldHandleInvalidJSONDuringComplexParsing() (1) Use GSON, to make assertion ignore ordering (2) Add indents when building the expected result, to improve the readability, no logic is changed
          Hide
          Edward Sargisson added a comment -

          Hi,
          Thanks for the patch - could you add it to the review board (https://reviews.apache.org) please?

          It's much easier to review and comment there.

          Cheers!

          Show
          Edward Sargisson added a comment - Hi, Thanks for the patch - could you add it to the review board ( https://reviews.apache.org ) please? It's much easier to review and comment there. Cheers!
          li xiang made changes -
          Remote Link This issue links to "FLUME-1793 (Web Link)" [ 15861 ]
          li xiang made changes -
          Remote Link This issue links to "Review request 23475 on review board (Web Link)" [ 15862 ]
          li xiang made changes -
          Remote Link This issue links to "FLUME-1793 (Web Link)" [ 15861 ]
          Hide
          li xiang added a comment -

          Hi Edward, I submitted a review request, 23475, see the links. Thanks for the guide 8-)

          Show
          li xiang added a comment - Hi Edward, I submitted a review request, 23475, see the links. Thanks for the guide 8-)
          Hide
          Edward Sargisson added a comment -

          Reviewed - I like it.

          Hari Shreedharan et. al. Shall we ship it?

          Show
          Edward Sargisson added a comment - Reviewed - I like it. Hari Shreedharan et. al. Shall we ship it?
          Hide
          Hari Shreedharan added a comment -

          Will commit this later today.

          Show
          Hari Shreedharan added a comment - Will commit this later today.
          Hide
          li xiang added a comment -

          Hi Hari, would you pls help to view the patch FLUME-1793 when it is convenient for you ? Thanks

          Show
          li xiang added a comment - Hi Hari, would you pls help to view the patch FLUME-1793 when it is convenient for you ? Thanks
          Hide
          Hari Shreedharan added a comment -

          +1.

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

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

          FLUME-1793. Unit test TestElasticSearchLogStashEventSerializer fails with IBM JDK

          (Li Xiang via Hari Shreedharan. Reviewed by Edward Sargisson)

          Show
          ASF subversion and git services added a comment - Commit 8410ad307187b19ca3a4330859815223d1e6b1e2 in flume's branch refs/heads/trunk from Hari Shreedharan [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=8410ad3 ] FLUME-1793 . Unit test TestElasticSearchLogStashEventSerializer fails with IBM JDK (Li Xiang via Hari Shreedharan. Reviewed by Edward Sargisson)
          Hide
          Hari Shreedharan added a comment -

          Committed! Thanks Li for the patch, and Edward for the review!

          Show
          Hari Shreedharan added a comment - Committed! Thanks Li for the patch, and Edward for the review!
          Hari Shreedharan made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          ASF subversion and git services added a comment -

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

          FLUME-1793. Unit test TestElasticSearchLogStashEventSerializer fails with IBM JDK

          (Li Xiang via Hari Shreedharan. Reviewed by Edward Sargisson)

          Show
          ASF subversion and git services added a comment - Commit 1c6775c56d5b88d29ee238d2dc9c5127c5880bf7 in flume's branch refs/heads/flume-1.6 from Hari Shreedharan [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=1c6775c ] FLUME-1793 . Unit test TestElasticSearchLogStashEventSerializer fails with IBM JDK (Li Xiang via Hari Shreedharan. Reviewed by Edward Sargisson)
          Hide
          li xiang added a comment -

          Thanks Edward and Hari. This is my first contribution to Flume. being pleased. Keep going

          Show
          li xiang added a comment - Thanks Edward and Hari. This is my first contribution to Flume. being pleased. Keep going
          Hide
          Hudson added a comment -

          UNSTABLE: Integrated in flume-trunk #646 (See https://builds.apache.org/job/flume-trunk/646/)
          FLUME-1793. Unit test TestElasticSearchLogStashEventSerializer fails with IBM JDK (harishreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=8410ad307187b19ca3a4330859815223d1e6b1e2)

          • flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchLogStashEventSerializer.java
          Show
          Hudson added a comment - UNSTABLE: Integrated in flume-trunk #646 (See https://builds.apache.org/job/flume-trunk/646/ ) FLUME-1793 . Unit test TestElasticSearchLogStashEventSerializer fails with IBM JDK (harishreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=8410ad307187b19ca3a4330859815223d1e6b1e2 ) flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchLogStashEventSerializer.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Flume-trunk-hbase-98 #6 (See https://builds.apache.org/job/Flume-trunk-hbase-98/6/)
          FLUME-1793. Unit test TestElasticSearchLogStashEventSerializer fails with IBM JDK (harishreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=8410ad307187b19ca3a4330859815223d1e6b1e2)

          • flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchLogStashEventSerializer.java
          Show
          Hudson added a comment - FAILURE: Integrated in Flume-trunk-hbase-98 #6 (See https://builds.apache.org/job/Flume-trunk-hbase-98/6/ ) FLUME-1793 . Unit test TestElasticSearchLogStashEventSerializer fails with IBM JDK (harishreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=8410ad307187b19ca3a4330859815223d1e6b1e2 ) flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchLogStashEventSerializer.java
          Hide
          li xiang added a comment -

          Hi Hudson, thanks for the test result. But those failures does not result from the fix for FLUME-1793. 1793 only fix a test case issue. And this test case pass:

          Running org.apache.flume.sink.elasticsearch.TestElasticSearchLogStashEventSerializer
          Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.31 sec

          I will try to figure out the cause of the failure later 8-)

          Show
          li xiang added a comment - Hi Hudson, thanks for the test result. But those failures does not result from the fix for FLUME-1793 . 1793 only fix a test case issue. And this test case pass: Running org.apache.flume.sink.elasticsearch.TestElasticSearchLogStashEventSerializer Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.31 sec I will try to figure out the cause of the failure later 8-)
          Johny Rufus made changes -
          Fix Version/s v1.5.1 [ 12328958 ]
          Johny Rufus made changes -
          Fix Version/s v1.6.0 [ 12327047 ]
          Fix Version/s v1.5.1 [ 12328958 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Patch Available Patch Available
          35m 9s 1 Aline Guedes Pinto 18/Dec/12 16:32
          Patch Available Patch Available Resolved Resolved
          576d 8h 3m 1 Hari Shreedharan 18/Jul/14 01:35

            People

            • Assignee:
              Unassigned
              Reporter:
              Aline Guedes Pinto
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development