[GitHub] [accumulo] ctubbsii commented on a diff in pull request #3206: Replace all instances of Guava cache with Caffeine

["ctubbsii (via GitHub)" <gi...@apache.org>]

ctubbsii commented on code in PR #3206:
URL: https://github.com/apache/accumulo/pull/3206#discussion_r1124620305


##########
core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java:
##########
@@ -170,8 +169,14 @@ public static class Reader implements Closeable {
 
     private long getCachedFileLen() throws IOException {
       try {
-        return fileLenCache.get(cacheId, lengthSupplier::get);
-      } catch (ExecutionException e) {
+        return fileLenCache.get(cacheId, k -> {
+          try {
+            return lengthSupplier.get();
+          } catch (IOException e) {
+            throw new UncheckedIOException(e);
+          }
+        });
+      } catch (Exception e) {

Review Comment:
   @cshannon Can this catch block be narrowed to just the UncheckedIOException that you're throwing internally, or in the worst case, catching all RTEs? I don't see what checked exceptions this needs to catch.



##########
server/manager/src/main/java/org/apache/accumulo/manager/recovery/RecoveryManager.java:
##########
@@ -144,8 +144,14 @@ private void initiateSort(String sortId, String source, final String destination
 
   private boolean exists(final Path path) throws IOException {
     try {
-      return existenceCache.get(path, () -> manager.getVolumeManager().exists(path));
-    } catch (ExecutionException e) {
+      return existenceCache.get(path, k -> {
+        try {
+          return manager.getVolumeManager().exists(path);
+        } catch (IOException e) {
+          throw new UncheckedIOException(e);
+        }
+      });
+    } catch (Exception e) {

Review Comment:
   Again, a very broad catch.



##########
pom.xml:
##########
@@ -1129,6 +1129,10 @@
                   <property name="format" value="&amp;quot; [+] &amp;quot;" />
                   <property name="message" value="Unnecessary concatenation of string literals" />
                 </module>
+                <module name="RegexpSinglelineJava">
+                  <property name="format" value="com.google.common.cache[.]" />

Review Comment:
   The pattern should be: `"com[.]google[.]common[.]cache[.]"



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org