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. BOOKKEEPER-554.diff
        3 kB
        Sijie Guo
      2. 0001-BOOKKEEPER-554.patch
        4 kB
        Ivan Kelly

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Patch Available Patch Available Open Open
        17d 7h 4m 1 Ivan Kelly 06/Feb/13 14:39
        Open Open Patch Available Patch Available
        4d 23h 10m 2 Ivan Kelly 11/Feb/13 13:47
        Patch Available Patch Available Resolved Resolved
        15h 12m 1 Sijie Guo 12/Feb/13 05:00
        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
        Ivan Kelly made changes -
        Fix Version/s 4.2.1 [ 12323840 ]
        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
        Sijie Guo made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Sijie Guo added a comment -

        committed as r1445033.

        Show
        Sijie Guo added a comment - committed as r1445033.
        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
        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/
        Ivan Kelly made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Ivan Kelly made changes -
        Attachment 0001-BOOKKEEPER-554.patch [ 12568821 ]
        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
        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.
        Ivan Kelly made changes -
        Status Patch Available [ 10002 ] Open [ 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
        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
        Uma Maheswara Rao G made changes -
        Affects Version/s 4.2.0 [ 12320244 ]
        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/
        Sijie Guo made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Sijie Guo made changes -
        Field Original Value New Value
        Attachment BOOKKEEPER-554.diff [ 12565671 ]
        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.
        Sijie Guo created issue -

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development