Flume
  1. Flume
  2. FLUME-2130

Handle larger payloads via SyslogUDPSource

    Details

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

      Description

      Syslog messages sent via UDP can contain a payload up to 65507 bytes. Payloads more than 768 bytes are currently truncated because the Netty's buffer size set to this value.

      1. FLUME-2130-2.patch
        5 kB
        Ashish Paliwal
      2. FLUME-2130-1.patch
        4 kB
        Ashish Paliwal
      3. FLUME-2130-0.patch
        4 kB
        Ashish Paliwal
      4. 0001-Handle-larger-UDP-payloads-via-SyslogUDPSource.patch
        5 kB
        Roman Scherer

        Issue Links

          Activity

          Roman Scherer created issue -
          Roman Scherer made changes -
          Field Original Value New Value
          Status Open [ 1 ] Patch Available [ 10002 ]
          Affects Version/s v1.3.0 [ 12322140 ]
          Affects Version/s v1.2.0 [ 12320243 ]
          Affects Version/s v1.4.0 [ 12323372 ]
          Roman Scherer made changes -
          Ashish Paliwal made changes -
          Assignee Ashish Paliwal [ paliwalashish ]
          Hide
          Ashish Paliwal added a comment -

          Working on rebasing the patch.

          Show
          Ashish Paliwal added a comment - Working on rebasing the patch.
          Ashish Paliwal made changes -
          Attachment FLUME-2130-0.patch [ 12616018 ]
          Ashish Paliwal made changes -
          Remote Link This issue links to "Review Link (Web Link)" [ 13415 ]
          Hide
          Gopinathan A added a comment -

          +1 for patch

          Show
          Gopinathan A added a comment - +1 for patch
          Hide
          Hari Shreedharan added a comment -

          Hi Ashish,

          Thanks for the patch. I have some comments on this patch. Looks like this patch creates a ByteArrayOutputStream with initial size of 64K - similarly for the AdaptiveReceiveBufferSizePredictorFactory. I think we should start with a smaller buffer - probably at 2K and set the max to 64K.
          So this can be AdaptiveReceiveBufferSizePredictorFactory (2048, 2048, 65536) and the ByteArrayOutputStream can be created using the current code itself. When the message is received, the current code already sets the maxSize to the event size.

          I think the only change required to the original code is to use AdaptiveReceiveBufferSizePredictorFactory, but created with smaller buffer values. We can maybe ask the user for hints through config on average event size, but I doubt it is necessary.

          Show
          Hari Shreedharan added a comment - Hi Ashish, Thanks for the patch. I have some comments on this patch. Looks like this patch creates a ByteArrayOutputStream with initial size of 64K - similarly for the AdaptiveReceiveBufferSizePredictorFactory. I think we should start with a smaller buffer - probably at 2K and set the max to 64K. So this can be AdaptiveReceiveBufferSizePredictorFactory (2048, 2048, 65536) and the ByteArrayOutputStream can be created using the current code itself. When the message is received, the current code already sets the maxSize to the event size. I think the only change required to the original code is to use AdaptiveReceiveBufferSizePredictorFactory, but created with smaller buffer values. We can maybe ask the user for hints through config on average event size, but I doubt it is necessary.
          Hide
          Ashish Paliwal added a comment -

          Hi Hari,

          Let me rework the patch and put it up for review again. I shall rebase the patch with latest code as well as incorporate the changes recommended.

          Show
          Ashish Paliwal added a comment - Hi Hari, Let me rework the patch and put it up for review again. I shall rebase the patch with latest code as well as incorporate the changes recommended.
          Hide
          Ashish Paliwal added a comment -

          Updated patch as per review comments. min size and initial size has been hardcoded to 2048. It's not configurable. We can alternatively make it configurable by asking from User. Please review and let me know if we need to do this.

          Show
          Ashish Paliwal added a comment - Updated patch as per review comments. min size and initial size has been hardcoded to 2048. It's not configurable. We can alternatively make it configurable by asking from User. Please review and let me know if we need to do this.
          Ashish Paliwal made changes -
          Attachment FLUME-2130-1.patch [ 12618022 ]
          Hide
          Hari Shreedharan added a comment -

          The patch does not seem to apply (perhaps after yesterday's commit?).

          Also, even now the SyslogUtils instance is being instantiated with maxSize as the size of the ByteArrayOutputStream - perhaps this should also be using DEFAULT_INITIAL_SIZE. BAOS does expand when needed.

          Show
          Hari Shreedharan added a comment - The patch does not seem to apply (perhaps after yesterday's commit?). Also, even now the SyslogUtils instance is being instantiated with maxSize as the size of the ByteArrayOutputStream - perhaps this should also be using DEFAULT_INITIAL_SIZE. BAOS does expand when needed.
          Hide
          Ashish Paliwal added a comment -

          Rebased
          Updated as per review comment
          The test class was changed, added new test case for larger layload

          Show
          Ashish Paliwal added a comment - Rebased Updated as per review comment The test class was changed, added new test case for larger layload
          Ashish Paliwal made changes -
          Attachment FLUME-2130-2.patch [ 12618775 ]
          Hide
          Hari Shreedharan added a comment -

          This looks good. The only issue is that UDP packets can't be fragmented, so even "large" packets may not reach the source at all based on the MTU of the netwrok being used. For example, I modified this patch's test and it was fine until I went up slightly about 7K (using the test in the patch itself).

          Show
          Hari Shreedharan added a comment - This looks good. The only issue is that UDP packets can't be fragmented, so even "large" packets may not reach the source at all based on the MTU of the netwrok being used. For example, I modified this patch's test and it was fine until I went up slightly about 7K (using the test in the patch itself).
          Hide
          Hari Shreedharan added a comment -

          +1. I am going to commit this. Thanks!

          Show
          Hari Shreedharan added a comment - +1. I am going to commit this. Thanks!
          Hide
          ASF subversion and git services added a comment -

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

          FLUME-2130. Handle larger payloads via SyslogUDPSource

          (Ashish Paliwal via Hari Shreedharan)

          Show
          ASF subversion and git services added a comment - Commit e07a0a6883c84836e618d187c1381d47a26bfc71 in branch refs/heads/trunk from Hari Shreedharan [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=e07a0a6 ] FLUME-2130 . Handle larger payloads via SyslogUDPSource (Ashish Paliwal via Hari Shreedharan)
          Hide
          ASF subversion and git services added a comment -

          Commit 5de9a8af4d0f859bd88e65996e7543302fd9eb1f in branch refs/heads/flume-1.5 from Hari Shreedharan
          [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=5de9a8a ]

          FLUME-2130. Handle larger payloads via SyslogUDPSource

          (Ashish Paliwal via Hari Shreedharan)

          Show
          ASF subversion and git services added a comment - Commit 5de9a8af4d0f859bd88e65996e7543302fd9eb1f in branch refs/heads/flume-1.5 from Hari Shreedharan [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=5de9a8a ] FLUME-2130 . Handle larger payloads via SyslogUDPSource (Ashish Paliwal via Hari Shreedharan)
          Hide
          Hari Shreedharan added a comment -

          Committed! Thanks Ashish!

          Show
          Hari Shreedharan added a comment - Committed! Thanks Ashish!
          Hari Shreedharan made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in flume-trunk #547 (See https://builds.apache.org/job/flume-trunk/547/)
          FLUME-2130. Handle larger payloads via SyslogUDPSource (hshreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=e07a0a6883c84836e618d187c1381d47a26bfc71)

          • flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java
          • flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUdpSource.java
          Show
          Hudson added a comment - SUCCESS: Integrated in flume-trunk #547 (See https://builds.apache.org/job/flume-trunk/547/ ) FLUME-2130 . Handle larger payloads via SyslogUDPSource (hshreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=e07a0a6883c84836e618d187c1381d47a26bfc71 ) flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUdpSource.java
          Hari Shreedharan made changes -
          Affects Version/s v1.3.1 [ 12323879 ]
          Affects Version/s v1.2.0 [ 12320243 ]
          Affects Version/s v1.3.0 [ 12322140 ]
          Affects Version/s v1.4.0 [ 12323372 ]
          Fix Version/s v1.5.0 [ 12324642 ]
          Hari Shreedharan made changes -
          Affects Version/s v1.4.0 [ 12323372 ]
          Affects Version/s v1.3.1 [ 12323879 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Patch Available Patch Available
          12m 21s 1 Roman Scherer 19/Jul/13 16:22
          Patch Available Patch Available Resolved Resolved
          203d 5h 12m 1 Hari Shreedharan 07/Feb/14 20:34

            People

            • Assignee:
              Ashish Paliwal
              Reporter:
              Roman Scherer
            • Votes:
              2 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development