Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.2.0
    • Fix Version/s: 1.4.0
    • Component/s: None
    • Labels:
      None

      Description

      It shouldn't happen since the directories are checked on startup to ensure they exist, but if an IO error occurred or if something changed underneath it, LogUtils.getLogs could throw an NPE in the for loop.

        Issue Links

          Activity

          Hide
          hshreedharan Hari Shreedharan added a comment -

          Since FLUME-1664 was marked as flume-1.3.0, I pushed this to that branch as well.

          Show
          hshreedharan Hari Shreedharan added a comment - Since FLUME-1664 was marked as flume-1.3.0, I pushed this to that branch as well.
          Hide
          hudson Hudson added a comment -

          Integrated in flume-trunk #317 (See https://builds.apache.org/job/flume-trunk/317/)
          FLUME-1652. Logutils.getLogs could throw NPE. (Revision 5416e77e16bb1906ed3b64754301825c5aeea6b3)

          Result = SUCCESS
          hshreedharan : http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=5416e77e16bb1906ed3b64754301825c5aeea6b3
          Files :

          • flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogUtils.java
          Show
          hudson Hudson added a comment - Integrated in flume-trunk #317 (See https://builds.apache.org/job/flume-trunk/317/ ) FLUME-1652 . Logutils.getLogs could throw NPE. (Revision 5416e77e16bb1906ed3b64754301825c5aeea6b3) Result = SUCCESS hshreedharan : http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=5416e77e16bb1906ed3b64754301825c5aeea6b3 Files : flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogUtils.java
          Hide
          brocknoland Brock Noland added a comment -

          OK cool. Yes they are checked during initialization to make sure they are directories. I created FLUME-1664 and attached a trivial patch to it.

          Show
          brocknoland Brock Noland added a comment - OK cool. Yes they are checked during initialization to make sure they are directories. I created FLUME-1664 and attached a trivial patch to it.
          Hide
          hshreedharan Hari Shreedharan added a comment -

          Yes. I think we should remove the check and throw anyway. We do check during initialization itself that the path represents a directory, right? I didn't notice the I/O error part in the Javadoc. +1 on removing the check.

          Show
          hshreedharan Hari Shreedharan added a comment - Yes. I think we should remove the check and throw anyway. We do check during initialization itself that the path represents a directory, right? I didn't notice the I/O error part in the Javadoc. +1 on removing the check.
          Hide
          brocknoland Brock Noland added a comment -

          OK thanks. I think we should remove the if(!logDir.isDirectory()) check. The reason being under normal circumstances listFiles() should never return null. Either the directory does not exist or an io error occurred. If the directory had logs but an io error occurred, we would just return as if nothing was there possibly losing data.

          Thoughts?

          Show
          brocknoland Brock Noland added a comment - OK thanks. I think we should remove the if(!logDir.isDirectory()) check. The reason being under normal circumstances listFiles() should never return null. Either the directory does not exist or an io error occurred. If the directory had logs but an io error occurred, we would just return as if nothing was there possibly losing data. Thoughts?
          Hide
          hshreedharan Hari Shreedharan added a comment -

          Pushed to trunk and flume-1.4. Rev: 5416e77e16bb1906ed3b64754301825c5aeea6b3

          If you'd like to put this in flume-1.3.0, please cherry-pick to the branch.

          Show
          hshreedharan Hari Shreedharan added a comment - Pushed to trunk and flume-1.4. Rev: 5416e77e16bb1906ed3b64754301825c5aeea6b3 If you'd like to put this in flume-1.3.0, please cherry-pick to the branch.
          Hide
          brocknoland Brock Noland added a comment -

          Attached patch improves the error message

          Show
          brocknoland Brock Noland added a comment - Attached patch improves the error message

            People

            • Assignee:
              brocknoland Brock Noland
              Reporter:
              brocknoland Brock Noland
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development