Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.2.0
    • Fix Version/s: 4.2.1, 4.3.0
    • Component/s: bookkeeper-server
    • Labels:
      None

      Description

      a file info is get when moving ledger index, but it doesn't release after use. so the reference counting for file info stays more than zero, the file channel would never be closed even the file is evicted from ledger cache.

      1. 0001-BOOKKEEPER-554.patch
        4 kB
        Ivan Kelly
      2. BOOKKEEPER-554.diff
        3 kB
        Sijie Guo

        Activity

        Hide
        Sijie Guo added a comment -

        straightforward patch and add reference counting assert in ledger cache test case.

        Show
        Sijie Guo added a comment - straightforward patch and add reference counting assert in ledger cache test case.
        Hide
        Hadoop QA added a comment -

        Testing JIRA BOOKKEEPER-554

        Patch BOOKKEEPER-554.diff downloaded at Sun Jan 20 07:41:38 UTC 2013

        ----------------------------

        +1 PATCH_APPLIES
        +1 CLEAN
        +1 RAW_PATCH_ANALYSIS
        . +1 the patch does not introduce any @author tags
        . +1 the patch does not introduce any tabs
        . +1 the patch does not introduce any trailing spaces
        . +1 the patch does not introduce any line longer than 120
        . +1 the patch does adds/modifies 1 testcase(s)
        +1 RAT
        . +1 the patch does not seem to introduce new RAT warnings
        +1 JAVADOC
        . +1 the patch does not seem to introduce new Javadoc warnings
        +1 COMPILE
        . +1 HEAD compiles
        . +1 patch compiles
        . +1 the patch does not seem to introduce new javac warnings
        +1 FINDBUGS
        . +1 the patch does not seem to introduce new Findbugs warnings
        +1 TESTS
        . Tests run: 790
        +1 DISTRO
        . +1 distro tarball builds with the patch

        ----------------------------
        +1 Overall result, good!, no -1s

        The full output of the test-patch run is available at

        . https://builds.apache.org/job/bookkeeper-trunk-precommit-build/251/

        Show
        Hadoop QA added a comment - Testing JIRA BOOKKEEPER-554 Patch BOOKKEEPER-554.diff downloaded at Sun Jan 20 07:41:38 UTC 2013 ---------------------------- +1 PATCH_APPLIES +1 CLEAN +1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . +1 the patch does not introduce any line longer than 120 . +1 the patch does adds/modifies 1 testcase(s) +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 FINDBUGS . +1 the patch does not seem to introduce new Findbugs warnings +1 TESTS . Tests run: 790 +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- +1 Overall result, good!, no -1s The full output of the test-patch run is available at . https://builds.apache.org/job/bookkeeper-trunk-precommit-build/251/
        Hide
        Uma Maheswara Rao G added a comment -

        Good catch Sijie!, thanks a lot for noticing it.
        +1

        Show
        Uma Maheswara Rao G added a comment - Good catch Sijie!, thanks a lot for noticing it. +1
        Hide
        Ivan Kelly added a comment -

        Fix looks good. Two style comments though.

        1. getUseCount should be annotated with @VisibleForTesting
        2. getFileInfo never returns null, so we don't need to check null != fi in the finally (findbugs 2.5.2 will show this as an error)
        Show
        Ivan Kelly added a comment - Fix looks good. Two style comments though. getUseCount should be annotated with @VisibleForTesting getFileInfo never returns null, so we don't need to check null != fi in the finally (findbugs 2.5.2 will show this as an error)
        Hide
        Sijie Guo added a comment -

        > getFileInfo never returns null,

        getFileInfo would throw exception. so we had to check null in finally block.

        Show
        Sijie Guo added a comment - > getFileInfo never returns null, getFileInfo would throw exception. so we had to check null in finally block.
        Hide
        Ivan Kelly added a comment -

        Added @VisibleForTesting annotation. Once latest patch gets +1, ill push it in.

        Show
        Ivan Kelly added a comment - Added @VisibleForTesting annotation. Once latest patch gets +1, ill push it in.
        Hide
        Hadoop QA added a comment -

        Testing JIRA BOOKKEEPER-554

        Patch 0001-BOOKKEEPER-554.patch downloaded at Mon Feb 11 13:55:54 UTC 2013

        ----------------------------

        +1 PATCH_APPLIES
        +1 CLEAN
        +1 RAW_PATCH_ANALYSIS
        . +1 the patch does not introduce any @author tags
        . +1 the patch does not introduce any tabs
        . +1 the patch does not introduce any trailing spaces
        . +1 the patch does not introduce any line longer than 120
        . +1 the patch does adds/modifies 1 testcase(s)
        +1 RAT
        . +1 the patch does not seem to introduce new RAT warnings
        +1 JAVADOC
        . +1 the patch does not seem to introduce new Javadoc warnings
        +1 COMPILE
        . +1 HEAD compiles
        . +1 patch compiles
        . +1 the patch does not seem to introduce new javac warnings
        +1 FINDBUGS
        . +1 the patch does not seem to introduce new Findbugs warnings
        +1 TESTS
        . Tests run: 816
        +1 DISTRO
        . +1 distro tarball builds with the patch

        ----------------------------
        +1 Overall result, good!, no -1s

        The full output of the test-patch run is available at

        . https://builds.apache.org/job/bookkeeper-trunk-precommit-build/265/

        Show
        Hadoop QA added a comment - Testing JIRA BOOKKEEPER-554 Patch 0001-BOOKKEEPER-554.patch downloaded at Mon Feb 11 13:55:54 UTC 2013 ---------------------------- +1 PATCH_APPLIES +1 CLEAN +1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . +1 the patch does not introduce any line longer than 120 . +1 the patch does adds/modifies 1 testcase(s) +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 FINDBUGS . +1 the patch does not seem to introduce new Findbugs warnings +1 TESTS . Tests run: 816 +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- +1 Overall result, good!, no -1s The full output of the test-patch run is available at . https://builds.apache.org/job/bookkeeper-trunk-precommit-build/265/
        Hide
        Sijie Guo added a comment -

        +1 for the new patch. thanks Ivan. will commit it.

        Show
        Sijie Guo added a comment - +1 for the new patch. thanks Ivan. will commit it.
        Hide
        Sijie Guo added a comment -

        committed as r1445033.

        Show
        Sijie Guo added a comment - committed as r1445033.
        Hide
        Hudson added a comment -

        Integrated in bookkeeper-trunk #98 (See https://builds.apache.org/job/bookkeeper-trunk/98/)
        BOOKKEEPER-554: fd leaking when move ledger index file (sijie, ivank via sijie) (Revision 1445033)

        Result = SUCCESS
        sijie :
        Files :

        • /zookeeper/bookkeeper/trunk/CHANGES.txt
        • /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfo.java
        • /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java
        • /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java
        Show
        Hudson added a comment - Integrated in bookkeeper-trunk #98 (See https://builds.apache.org/job/bookkeeper-trunk/98/ ) BOOKKEEPER-554 : fd leaking when move ledger index file (sijie, ivank via sijie) (Revision 1445033) Result = SUCCESS sijie : Files : /zookeeper/bookkeeper/trunk/CHANGES.txt /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfo.java /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java
        Hide
        Ivan Kelly added a comment -

        Committed to 4.2 branch as 1445763

        Show
        Ivan Kelly added a comment - Committed to 4.2 branch as 1445763

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development