Bookkeeper
  1. Bookkeeper
  2. BOOKKEEPER-229

Deleted entry log files would be garbage collected again and again.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.1.0
    • Fix Version/s: 4.1.0
    • Component/s: bookkeeper-server
    • Labels:
      None

      Description

      after BOOKKEEPER-188 is applied, extractMetaFromEntryLogs is moved from EntryLogger to GarbageCollectorThread with some changed.

      Before BOOKKEEPER-188 is applied, we just add the entryLogMeta to entryLogMetaMap only when we could extract the entry log file. If a log file is garbage collected, its entryLogMeta would not be put in the map.

      -    protected Map<Long, EntryLogMetadata> extractMetaFromEntryLogs(Map<Long, EntryLogMetadata> entryLogMetaMap) throws IOException {
      -        // Extract it for every entry log except for the current one.
      -        // Entry Log ID's are just a long value that starts at 0 and increments
      -        // by 1 when the log fills up and we roll to a new one.
      -        long curLogId = logId;
      -        for (long entryLogId = 0; entryLogId < curLogId; entryLogId++) {
      -            // Comb the current entry log file if it has not already been extracted.
      -            if (entryLogMetaMap.containsKey(entryLogId)) {
      -                continue;
      -            }
      -            LOG.info("Extracting entry log meta from entryLogId: " + entryLogId);
      -            EntryLogMetadata entryLogMeta = new EntryLogMetadata(entryLogId);
      -            ExtractionScanner scanner = new ExtractionScanner(entryLogMeta);
      -            // Read through the entry log file and extract the entry log meta
      -            try {
      -                scanEntryLog(entryLogId, scanner);
      -                LOG.info("Retrieved entry log meta data entryLogId: " + entryLogId + ", meta: " + entryLogMeta);
      -                entryLogMetaMap.put(entryLogId, entryLogMeta);
      -            } catch(IOException e) {
      -              LOG.warn("Premature exception when processing " + entryLogId +
      -                       "recovery will take care of the problem", e);
      -            }
      -
      -        }
      -        return entryLogMetaMap;
      -    }
      

      But after BOOKKEEPER-188 is applied, an empty entryLogMeta would be put into entryLogMetaMap for those deleted entry log files. So GarbageCollectorThread would gc those deleted entry log files again. Then there is lots of such kind of error messages, these are noise error message but doesn't affect the logic.

      +    protected Map<Long, EntryLogMetadata> extractMetaFromEntryLogs(Map<Long, EntryLogMetadata> entryLogMetaMap)
      +            throws IOException {
      +        // Extract it for every entry log except for the current one.
      +        // Entry Log ID's are just a long value that starts at 0 and increments
      +        // by 1 when the log fills up and we roll to a new one.
      +        long curLogId = entryLogger.logId;
      +        for (long entryLogId = 0; entryLogId < curLogId; entryLogId++) {
      +            // Comb the current entry log file if it has not already been extracted.
      +            if (entryLogMetaMap.containsKey(entryLogId)) {
      +                continue;
      +            }
      +            LOG.info("Extracting entry log meta from entryLogId: " + entryLogId);
      +
      +            // Read through the entry log file and extract the entry log meta
      +            entryLogMetaMap.put(entryLogId,
      +                                extractMetaFromEntryLog(entryLogger, entryLogId));
      +        }
      +        return entryLogMetaMap;
      +    }
      +
      +    static EntryLogMetadata extractMetaFromEntryLog(EntryLogger entryLogger, long entryLogId)
      +            throws IOException {
      +        EntryLogMetadata entryLogMeta = new EntryLogMetadata(entryLogId);
      +        ExtractionScanner scanner = new ExtractionScanner(entryLogMeta);
      +        try {
      +            // Read through the entry log file and extract the entry log meta
      +            entryLogger.scanEntryLog(entryLogId, scanner);
      +            LOG.info("Retrieved entry log meta data entryLogId: "
      +                     + entryLogId + ", meta: " + entryLogMeta);
      +        } catch(IOException e) {
      +            LOG.warn("Premature exception when processing " + entryLogId +
      +                     "recovery will take care of the problem", e);
      +        }
      +
      +        return entryLogMeta;
      +    }
      
      1. BK-229.diff_v3
        5 kB
        Sijie Guo
      2. BK-229.diff_v2
        4 kB
        Sijie Guo
      3. BK-229.diff
        4 kB
        Sijie Guo

        Activity

        Hide
        Hudson added a comment -

        Integrated in bookkeeper-trunk #493 (See https://builds.apache.org/job/bookkeeper-trunk/493/)
        BOOKKEEPER-229: Deleted entry log files would be garbage collected again and again. (sijie via fpj) (Revision 1335367)

        Result = ABORTED
        fpj :
        Files :

        • /zookeeper/bookkeeper/trunk/CHANGES.txt
        • /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
        • /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/EntryLogTest.java
        Show
        Hudson added a comment - Integrated in bookkeeper-trunk #493 (See https://builds.apache.org/job/bookkeeper-trunk/493/ ) BOOKKEEPER-229 : Deleted entry log files would be garbage collected again and again. (sijie via fpj) (Revision 1335367) Result = ABORTED fpj : Files : /zookeeper/bookkeeper/trunk/CHANGES.txt /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/EntryLogTest.java
        Hide
        Flavio Junqueira added a comment -

        Committed revision 1335367.

        Show
        Flavio Junqueira added a comment - Committed revision 1335367.
        Hide
        Flavio Junqueira added a comment -

        +1, thanks sijie.

        Show
        Flavio Junqueira added a comment - +1, thanks sijie.
        Hide
        Sijie Guo added a comment -

        attach a new patch addressing Flavio's comments.

        Show
        Sijie Guo added a comment - attach a new patch addressing Flavio's comments.
        Hide
        Sijie Guo added a comment -

        OK thanks for pointing out. will update it ASAP.

        Show
        Sijie Guo added a comment - OK thanks for pointing out. will update it ASAP.
        Hide
        Flavio Junqueira added a comment -

        Got it, thanks. One small issue, I think that extractMetaFromEntryLogs does not need to throw an IOException any longer. If you remove it, I believe there is only one compilation error to fix.

        Show
        Flavio Junqueira added a comment - Got it, thanks. One small issue, I think that extractMetaFromEntryLogs does not need to throw an IOException any longer. If you remove it, I believe there is only one compilation error to fix.
        Hide
        Sijie Guo added a comment -

        the test change is not related to code change. because the patch changed extractMetaFromEntryLog to throw IOException, so EntryLogTest can't get EntryLogMeta. I just changed the method used in EntryLogTest not to break the test.

        Show
        Sijie Guo added a comment - the test change is not related to code change. because the patch changed extractMetaFromEntryLog to throw IOException, so EntryLogTest can't get EntryLogMeta. I just changed the method used in EntryLogTest not to break the test.
        Hide
        Flavio Junqueira added a comment -

        The patch looks good to me, but the test change does not fail when I only apply the test change, ignoring the change to GarbageCollectorThread. Cancelling the patch until we figure it out.

        Show
        Flavio Junqueira added a comment - The patch looks good to me, but the test change does not fail when I only apply the test change, ignoring the change to GarbageCollectorThread. Cancelling the patch until we figure it out.
        Hide
        Sijie Guo added a comment -

        attach a new patch adding an assertion in EntryLogTest based on previous patch.

        Show
        Sijie Guo added a comment - attach a new patch adding an assertion in EntryLogTest based on previous patch.
        Hide
        Sijie Guo added a comment -

        attach a patch to fix this noise error messages.

        Show
        Sijie Guo added a comment - attach a patch to fix this noise error messages.

          People

          • Assignee:
            Sijie Guo
            Reporter:
            Sijie Guo
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development