Flume
  1. Flume
  2. FLUME-1264

HDFEventSink throws NPE if event is generated without timestamp in header

    Details

    • Type: Bug Bug
    • Status: Patch Available
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Sinks+Sources
    • Labels:

      Description

      If timestamp is null then it may defaults to new Date().getTime()

      long ts = Long.valueOf(headers.get("timestamp"));

      2012-06-09 00:48:36,166 ERROR hdfs.HDFSEventSink: process failed
      java.lang.NumberFormatException: null
      	at java.lang.Long.parseLong(Long.java:375)
      	at java.lang.Long.valueOf(Long.java:525)
      	at org.apache.flume.formatter.output.BucketPath.replaceShorthand(BucketPath.java:220)
      	at org.apache.flume.formatter.output.BucketPath.escapeString(BucketPath.java:310)
      	at org.apache.flume.sink.hdfs.HDFSEventSink.process(HDFSEventSink.java:366)
      	at org.apache.flume.sink.DefaultSinkProcessor.process(DefaultSinkProcessor.java:68)
      	at org.apache.flume.SinkRunner$PollingRunner.run(SinkRunner.java:147)
      	at java.lang.Thread.run(Thread.java:662)
      2012-06-09 00:48:36,168 ERROR flume.SinkRunner: Unable to deliver event. Exception follows.
      org.apache.flume.EventDeliveryException: java.lang.NumberFormatException: null
      	at org.apache.flume.sink.hdfs.HDFSEventSink.process(HDFSEventSink.java:414)
      	at org.apache.flume.sink.DefaultSinkProcessor.process(DefaultSinkProcessor.java:68)
      	at org.apache.flume.SinkRunner$PollingRunner.run(SinkRunner.java:147)
      	at java.lang.Thread.run(Thread.java:662)
      Caused by: java.lang.NumberFormatException: null
      	at java.lang.Long.parseLong(Long.java:375)
      	at java.lang.Long.valueOf(Long.java:525)
      	at org.apache.flume.formatter.output.BucketPath.replaceShorthand(BucketPath.java:220)
      	at org.apache.flume.formatter.output.BucketPath.escapeString(BucketPath.java:310)
      	at org.apache.flume.sink.hdfs.HDFSEventSink.process(HDFSEventSink.java:366)
      	... 3 more
      
      
      
      1. FLUME-1264_1.patch
        3 kB
        Alexander Alten-Lorenz
      2. FLUME-1264.trunk.v1.patch
        2 kB
        Mubarak Seyed

        Issue Links

          Activity

          Hide
          Mubarak Seyed added a comment -

          Is it fine to default the timestamp to system's current time when header's timestamp key is null?

          This is what i am thinking about the fix

          long ts = Long.valueOf(headers.get("timestamp") != null ? headers.get("timestamp") : new Date().getTime());
          
          Show
          Mubarak Seyed added a comment - Is it fine to default the timestamp to system's current time when header's timestamp key is null? This is what i am thinking about the fix long ts = Long .valueOf(headers.get( "timestamp" ) != null ? headers.get( "timestamp" ) : new Date().getTime());
          Hide
          Mike Percy added a comment -

          Hi Mubarak, I'd recommend using the Timestamp interceptor. See FLUME-1215 for details.

          Show
          Mike Percy added a comment - Hi Mubarak, I'd recommend using the Timestamp interceptor. See FLUME-1215 for details.
          Hide
          Mike Percy added a comment -

          BTW, to use the Timestamp Interceptor for this use case, you would configure it with preserveExisting = true.

          Show
          Mike Percy added a comment - BTW, to use the Timestamp Interceptor for this use case, you would configure it with preserveExisting = true .
          Hide
          Inder SIngh added a comment -

          Mike what your thoughts about the following suggestion -

          1. If a configuration exists which uses shortHands, then not providing time-stamp interceptor is an INVALID configuration. Since almost shorthands work over timestamp this might be good validation to have.

          Show
          Inder SIngh added a comment - Mike what your thoughts about the following suggestion - 1. If a configuration exists which uses shortHands, then not providing time-stamp interceptor is an INVALID configuration. Since almost shorthands work over timestamp this might be good validation to have.
          Hide
          Mubarak Seyed added a comment -

          The attached file is a patch. Thanks.

          Show
          Mubarak Seyed added a comment - The attached file is a patch. Thanks.
          Hide
          Mubarak Seyed added a comment -

          @Mike,
          If there is no timestamp header in event then timestamp interceptor defaults to preserveExisting = false, which adds the timestamp header with system's current time in event header. This patch checks only null value in timestamp header and then intercept timestamp interceptor.

          If passed timestamp value is not a valid number then parseLong still throws NumberFormatException, which is valid?

          Show
          Mubarak Seyed added a comment - @Mike, If there is no timestamp header in event then timestamp interceptor defaults to preserveExisting = false , which adds the timestamp header with system's current time in event header. This patch checks only null value in timestamp header and then intercept timestamp interceptor. If passed timestamp value is not a valid number then parseLong still throws NumberFormatException , which is valid?
          Hide
          Mubarak Seyed added a comment -

          Posted for code review at https://reviews.apache.org/r/5348/

          Show
          Mubarak Seyed added a comment - Posted for code review at https://reviews.apache.org/r/5348/
          Hide
          Alexander Alten-Lorenz added a comment -

          Patch was missing on https://reviews.apache.org/r/5348/, attached.

          Show
          Alexander Alten-Lorenz added a comment - Patch was missing on https://reviews.apache.org/r/5348/ , attached.
          Hide
          Alexander Alten-Lorenz added a comment -

          attached patch doesn't appear like a diff, recreate them and attach again

          Show
          Alexander Alten-Lorenz added a comment - attached patch doesn't appear like a diff, recreate them and attach again
          Hide
          Alexander Alten-Lorenz added a comment -
          Show
          Alexander Alten-Lorenz added a comment - new RB request: https://reviews.apache.org/r/7896/

            People

            • Assignee:
              Mubarak Seyed
              Reporter:
              Mubarak Seyed
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:

                Development