Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-5256

Use guava LoadingCache to implement DFSClientCache

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.2.0
    • Component/s: nfs
    • Labels:
      None

      Description

      Google Guava provides an implementation of LoadingCache. Use the LoadingCache to implement DFSClientCache in NFS.

      1. HDFS-5256.004.patch
        10 kB
        Haohui Mai
      2. HDFS-5256.003.patch
        10 kB
        Haohui Mai
      3. HDFS-5256.002.patch
        10 kB
        Haohui Mai
      4. HDFS-5256.001.patch
        9 kB
        Haohui Mai
      5. HDFS-5256.000.patch
        9 kB
        Haohui Mai

        Activity

        Haohui Mai created issue -
        Haohui Mai made changes -
        Field Original Value New Value
        Attachment HDFS-5256.000.patch [ 12604925 ]
        Haohui Mai made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Haohui Mai made changes -
        Attachment HDFS-5256.000.patch [ 12604925 ]
        Haohui Mai made changes -
        Attachment HDFS-5256.000.patch [ 12604926 ]
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12604926/HDFS-5256.000.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-nfs.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5031//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5031//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12604926/HDFS-5256.000.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-nfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5031//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5031//console This message is automatically generated.
        Hide
        Haohui Mai added a comment -

        Address Jing's comment + plus a little bit of formatting.

        Show
        Haohui Mai added a comment - Address Jing's comment + plus a little bit of formatting.
        Haohui Mai made changes -
        Attachment HDFS-5256.001.patch [ 12605294 ]
        Haohui Mai made changes -
        Attachment HDFS-5256.001.patch [ 12605294 ]
        Haohui Mai made changes -
        Attachment HDFS-5256.001.patch [ 12605309 ]
        Hide
        Haohui Mai added a comment -

        Address Brandon's comments. More comprehensive unit tests.

        Show
        Haohui Mai added a comment - Address Brandon's comments. More comprehensive unit tests.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12605309/HDFS-5256.001.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-nfs.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5047//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5047//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12605309/HDFS-5256.001.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-nfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5047//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5047//console This message is automatically generated.
        Haohui Mai made changes -
        Attachment HDFS-5256.002.patch [ 12605321 ]
        Hide
        Brandon Li added a comment -

        Thanks Haohui for adding unit test. More ideas for better unit tests of the new cache:
        1. add n clients to the cache, check the cache size is n, remove some client, and make sure the cache size changes correspondingly
        2. do cache.get() for the same user, make sure the cache doesn't add new entry for the same user
        3. make sure the evicted client is closed (which you are already trying to test with current patch)
        4. give the cache a short expiration time to verify the client can be evicted and closed after expiration time

        Show
        Brandon Li added a comment - Thanks Haohui for adding unit test. More ideas for better unit tests of the new cache: 1. add n clients to the cache, check the cache size is n, remove some client, and make sure the cache size changes correspondingly 2. do cache.get() for the same user, make sure the cache doesn't add new entry for the same user 3. make sure the evicted client is closed (which you are already trying to test with current patch) 4. give the cache a short expiration time to verify the client can be evicted and closed after expiration time
        Haohui Mai made changes -
        Attachment HDFS-5256.003.patch [ 12605324 ]
        Hide
        Haohui Mai added a comment -

        Addressed Brandon's comments.

        Show
        Haohui Mai added a comment - Addressed Brandon's comments.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12605321/HDFS-5256.002.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-nfs.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5048//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5048//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12605321/HDFS-5256.002.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-nfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5048//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5048//console This message is automatically generated.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12605324/HDFS-5256.003.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-nfs.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5050//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5050//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12605324/HDFS-5256.003.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-nfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5050//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5050//console This message is automatically generated.
        Hide
        Brandon Li added a comment -

        Haohui, let's remove the TTL since the cache entry usage inside NFS servers may not actually visible here. Also please make this warning message more verbose in DFSClientCache.java, such as:
        LOG.warn("DFSClientCache got IOException when closing the DFSClient("
        + notification.getValue() + "): " + e);

        Show
        Brandon Li added a comment - Haohui, let's remove the TTL since the cache entry usage inside NFS servers may not actually visible here. Also please make this warning message more verbose in DFSClientCache.java, such as: LOG.warn("DFSClientCache got IOException when closing the DFSClient(" + notification.getValue() + "): " + e);
        Hide
        Haohui Mai added a comment -

        Thanks Brandon Li for the comments. Address the comments.

        Show
        Haohui Mai added a comment - Thanks Brandon Li for the comments. Address the comments.
        Haohui Mai made changes -
        Attachment HDFS-5256.004.patch [ 12605382 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12605382/HDFS-5256.004.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        -1 eclipse:eclipse. The patch failed to build with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-nfs.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5051//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5051//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12605382/HDFS-5256.004.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 eclipse:eclipse . The patch failed to build with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-nfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5051//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5051//console This message is automatically generated.
        Hide
        Brandon Li added a comment -

        +1. Thank you, Haohui, for refining the patch's details!

        Show
        Brandon Li added a comment - +1. Thank you, Haohui, for refining the patch's details!
        Brandon Li made changes -
        Issue Type Bug [ 1 ] Improvement [ 4 ]
        Hide
        Brandon Li added a comment -

        I've committed the patch. Thank you, Haohui, for the contribution!

        Show
        Brandon Li added a comment - I've committed the patch. Thank you, Haohui, for the contribution!
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in Hadoop-trunk-Commit #4496 (See https://builds.apache.org/job/Hadoop-trunk-Commit/4496/)
        HDFS-5256. Use guava LoadingCache to implement DFSClientCache. Contributed by Haohui Mai (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1527452)

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/DFSClientCache.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/LruCache.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/test/java/org/apache/hadoop/hdfs/nfs/nfs3/TestDFSClientCache.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #4496 (See https://builds.apache.org/job/Hadoop-trunk-Commit/4496/ ) HDFS-5256 . Use guava LoadingCache to implement DFSClientCache. Contributed by Haohui Mai (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1527452 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/DFSClientCache.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/LruCache.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/test/java/org/apache/hadoop/hdfs/nfs/nfs3/TestDFSClientCache.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Brandon Li made changes -
        Fix Version/s 2.1.2-beta [ 12325049 ]
        Brandon Li made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk #348 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/348/)
        HDFS-5256. Use guava LoadingCache to implement DFSClientCache. Contributed by Haohui Mai (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1527452)

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/DFSClientCache.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/LruCache.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/test/java/org/apache/hadoop/hdfs/nfs/nfs3/TestDFSClientCache.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #348 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/348/ ) HDFS-5256 . Use guava LoadingCache to implement DFSClientCache. Contributed by Haohui Mai (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1527452 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/DFSClientCache.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/LruCache.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/test/java/org/apache/hadoop/hdfs/nfs/nfs3/TestDFSClientCache.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Hdfs-trunk #1538 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1538/)
        HDFS-5256. Use guava LoadingCache to implement DFSClientCache. Contributed by Haohui Mai (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1527452)

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/DFSClientCache.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/LruCache.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/test/java/org/apache/hadoop/hdfs/nfs/nfs3/TestDFSClientCache.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk #1538 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1538/ ) HDFS-5256 . Use guava LoadingCache to implement DFSClientCache. Contributed by Haohui Mai (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1527452 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/DFSClientCache.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/LruCache.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/test/java/org/apache/hadoop/hdfs/nfs/nfs3/TestDFSClientCache.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #1564 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1564/)
        HDFS-5256. Use guava LoadingCache to implement DFSClientCache. Contributed by Haohui Mai (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1527452)

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/DFSClientCache.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/LruCache.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/test/java/org/apache/hadoop/hdfs/nfs/nfs3/TestDFSClientCache.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #1564 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1564/ ) HDFS-5256 . Use guava LoadingCache to implement DFSClientCache. Contributed by Haohui Mai (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1527452 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/DFSClientCache.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/LruCache.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-nfs/src/test/java/org/apache/hadoop/hdfs/nfs/nfs3/TestDFSClientCache.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        Eli Collins added a comment -
        • How does testEviction test eviction? It sets a max cache size of 2 and only ever caches one entry?
        • Nit: clientCache is overloaded as both an integer and a LoadingCache object in the constructor
        • Nit: Spelling mistake in "clientRemovealListener"
        Show
        Eli Collins added a comment - How does testEviction test eviction? It sets a max cache size of 2 and only ever caches one entry? Nit: clientCache is overloaded as both an integer and a LoadingCache object in the constructor Nit: Spelling mistake in "clientRemovealListener"
        Hide
        Haohui Mai added a comment -

        How does testEviction test eviction? It sets a max cache size of 2 and only ever caches one entry?

        The guava cache only holds MAX_CACHE_SIZE - 1 entries, therefore the unit test is a basic test whether the eviction works.

        The unit test is basically a rewrite of the legacy unit test. We used to implement the cache in house, but
        given the fact that DFSClientCache is a thin wrap of guava cache, the test looks more like a unit test of the guava cache rather than a test of DFSClientCache.

        Nit: clientCache is overloaded as both an integer and a LoadingCache object in the constructor

        Testing is the only reason of implementing two constructors of DFSClientCache(). It should be cleaned up once we reach a consensus on the unit test.

        Nit: Spelling mistake in "clientRemovealListener"

        I'll fix it in a separate jira.

        Show
        Haohui Mai added a comment - How does testEviction test eviction? It sets a max cache size of 2 and only ever caches one entry? The guava cache only holds MAX_CACHE_SIZE - 1 entries, therefore the unit test is a basic test whether the eviction works. The unit test is basically a rewrite of the legacy unit test. We used to implement the cache in house, but given the fact that DFSClientCache is a thin wrap of guava cache, the test looks more like a unit test of the guava cache rather than a test of DFSClientCache. Nit: clientCache is overloaded as both an integer and a LoadingCache object in the constructor Testing is the only reason of implementing two constructors of DFSClientCache(). It should be cleaned up once we reach a consensus on the unit test. Nit: Spelling mistake in "clientRemovealListener" I'll fix it in a separate jira.

          People

          • Assignee:
            Haohui Mai
            Reporter:
            Haohui Mai
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development