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

idLogFileMap in Log can lose track of file ids

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.4.0
    • Fix Version/s: 1.6.0
    • Component/s: File Channel
    • Labels:
      None

      Description

      The following code from Log's writeCheckpoint method can lose track of file ids to Reader mappings which will lead to a NPE being thrown in subsequent calls to writeCheckpoint.

      Log#writeCheckpoint
      Iterator<Integer> idIterator = logFileRefCountsAll.iterator();
      while (idIterator.hasNext()) {
          int id = idIterator.next();
          LogFile.RandomReader reader = idLogFileMap.remove(id);
          File file = reader.getFile();
          reader.close();
          LogFile.MetaDataWriter writer =
                  LogFileFactory.getMetaDataWriter(file, id);
          try {
              writer.markCheckpoint(logWriteOrderID);
           } finally {
              writer.close();
          }
          reader = LogFileFactory.getRandomReader(file, encryptionKeyProvider);
          idLogFileMap.put(id, reader);
          LOGGER.debug("Updated checkpoint for file: " + file
                  + "logWriteOrderID " + logWriteOrderID);
          idIterator.remove();
      }
      

      The problem occurs when writer.markCheckpoint throws an exception and the id -> reader mapping is not added back to idLogFileMap. The next time writeCheckpoint is called logFileRefCountsAll still contains the file id, but idLogFileMap.remove(id) returns null since the id is no longer in the map. Attempting to call reader.getFile() then throws a NPE.

      Is there a reason that the initial reader obtained from idLogFileMap is closed and then a new reader for the same file is later created an inserted into the map? If that is not required, then one possible solution would be to remove this logic and not remove the id -> reader mapping in idLogFileMap. If that logic is required, then perhaps the code to insert a new id -> reader mapping into idLogFileMap could be added to the finally block.

        Activity

        Hide
        brocknoland Brock Noland added a comment -

        Good catch! FYI Hari Shreedharan

        Show
        brocknoland Brock Noland added a comment - Good catch! FYI Hari Shreedharan
        Hide
        jrufus Johny Rufus added a comment -

        Attaching a patch that contains a fix as suggested by Steve Zesch.

        Show
        jrufus Johny Rufus added a comment - Attaching a patch that contains a fix as suggested by Steve Zesch.
        Hide
        hshreedharan Hari Shreedharan added a comment -

        +1. Looks good

        Show
        hshreedharan Hari Shreedharan added a comment - +1. Looks good
        Hide
        jira-bot ASF subversion and git services added a comment -

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

        FLUME-2346. idLogFileMap in Log can lose track of file ids.

        (Johny Rufus via Hari)

        Show
        jira-bot ASF subversion and git services added a comment - Commit 35cf900017a797e5ff3aedb01f05c88bb3f86ca0 in flume's branch refs/heads/trunk from Hari Shreedharan [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=35cf900 ] FLUME-2346 . idLogFileMap in Log can lose track of file ids. (Johny Rufus via Hari)
        Hide
        jira-bot ASF subversion and git services added a comment -

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

        FLUME-2346. idLogFileMap in Log can lose track of file ids.

        (Johny Rufus via Hari)

        Show
        jira-bot ASF subversion and git services added a comment - Commit a601c97a2376110b7fbe9a69f653a89f510cd333 in flume's branch refs/heads/flume-1.6 from Hari Shreedharan [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=a601c97 ] FLUME-2346 . idLogFileMap in Log can lose track of file ids. (Johny Rufus via Hari)
        Hide
        hshreedharan Hari Shreedharan added a comment -

        Committed! Thanks Johny!

        Show
        hshreedharan Hari Shreedharan added a comment - Committed! Thanks Johny!
        Hide
        hudson Hudson added a comment -

        UNSTABLE: Integrated in flume-trunk #669 (See https://builds.apache.org/job/flume-trunk/669/)
        FLUME-2346. idLogFileMap in Log can lose track of file ids. (hshreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=35cf900017a797e5ff3aedb01f05c88bb3f86ca0)

        • flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java
        Show
        hudson Hudson added a comment - UNSTABLE: Integrated in flume-trunk #669 (See https://builds.apache.org/job/flume-trunk/669/ ) FLUME-2346 . idLogFileMap in Log can lose track of file ids. (hshreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=35cf900017a797e5ff3aedb01f05c88bb3f86ca0 ) flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java
        Hide
        hudson Hudson added a comment -

        UNSTABLE: Integrated in Flume-trunk-hbase-98 #29 (See https://builds.apache.org/job/Flume-trunk-hbase-98/29/)
        FLUME-2346. idLogFileMap in Log can lose track of file ids. (hshreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=35cf900017a797e5ff3aedb01f05c88bb3f86ca0)

        • flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java
        Show
        hudson Hudson added a comment - UNSTABLE: Integrated in Flume-trunk-hbase-98 #29 (See https://builds.apache.org/job/Flume-trunk-hbase-98/29/ ) FLUME-2346 . idLogFileMap in Log can lose track of file ids. (hshreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=35cf900017a797e5ff3aedb01f05c88bb3f86ca0 ) flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java

          People

          • Assignee:
            jrufus Johny Rufus
            Reporter:
            szesch Steve Zesch
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development