[GitHub] [cassandra] yifan-c commented on a change in pull request #1329: CASSANDRA-17159 Log queries that fail on timeout or unavailable errors up to once per minute by default

[GitBox <gi...@apache.org>]

yifan-c commented on a change in pull request #1329:
URL: https://github.com/apache/cassandra/pull/1329#discussion_r751727820



##########
File path: src/java/org/apache/cassandra/service/StorageProxy.java
##########
@@ -162,6 +164,9 @@
     private static final Logger logger = LoggerFactory.getLogger(StorageProxy.class);
 
     public static final String UNREACHABLE = "UNREACHABLE";
+    
+    public static final String REQUEST_FAIL_MESSAGE = "\"{}\" while executing {}";

Review comment:
       It does not seem to gain anything by declaring a template string... We can just reference to the string literal inline.

##########
File path: src/java/org/apache/cassandra/utils/NoSpamLogger.java
##########
@@ -84,21 +85,31 @@ private boolean shouldLog(long nowNanos)
             return nowNanos >= expected && compareAndSet(expected, nowNanos + minIntervalNanos);
         }
 
+        public boolean log(Level l, long nowNanos, Supplier<Object[]> objects)
+        {
+            if (!shouldLog(nowNanos)) return false;
+            return logNoCheck(l, objects.get());
+        }
+
         public boolean log(Level l, long nowNanos, Object... objects)
         {
             if (!shouldLog(nowNanos)) return false;
+            return logNoCheck(l, objects);
+        }
 
+        private boolean logNoCheck(Level l, Object... objects)
+        {
             switch (l)
             {
-            case INFO:
-                wrapped.info(statement, objects);
-                break;
-            case WARN:
-                wrapped.warn(statement, objects);
-                break;
-            case ERROR:
-                wrapped.error(statement, objects);
-                break;
+                case INFO:
+                    wrapped.info(statement, objects);
+                    break;
+                case WARN:
+                    wrapped.warn(statement, objects);
+                    break;
+                case ERROR:
+                    wrapped.error(statement, objects);
+                    break;

Review comment:
       Cassandra inherits most of Sun's java code style.. and for the switch statement, it seems no indentation for cases is required.
   https://www.oracle.com/java/technologies/javase/codeconventions-statements.html#468




-- 
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: pr-unsubscribe@cassandra.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org