jacek-lewandowski commented on code in PR #1488:
URL: https://github.com/apache/cassandra/pull/1488#discussion_r1082208962
##########
src/java/org/apache/cassandra/io/util/PathUtils.java:
##########
@@ -64,14 +64,16 @@
private static final Set<StandardOpenOption> READ_WRITE_OPTIONS =
unmodifiableSet(EnumSet.of(READ, WRITE, CREATE));
private static final FileAttribute<?>[] NO_ATTRIBUTES = new
FileAttribute[0];
+ private static final boolean USE_NIX_RECURSIVE_DELETE =
CassandraRelevantProperties.USE_NIX_RECURSIVE_DELETE.getBoolean();
+
private static final Logger logger =
LoggerFactory.getLogger(PathUtils.class);
private static final NoSpamLogger nospam1m =
NoSpamLogger.getLogger(logger, 1, TimeUnit.MINUTES);
private static Consumer<Path> onDeletion = path -> {
if (StorageService.instance.isDaemonSetupCompleted())
setDeletionListener(ignore -> {});
- else
- logger.info("Deleting file during startup: {}", path);
+ else if (logger.isTraceEnabled())
Review Comment:
Obviously this is not needed in this case.
However, in general case we should use `if` when:
```java
log.trace("message {}", someHeavyToString());
```
if we go with a method like `logIfTrace` we would have to either evaluate
that `someHeavyToString()` before going to that method regardless trace is
enable or not (which undermine the existence of `if` at all) or we would have
to pass a lambda, which is also much heavier than just checking
`logger.isTraceEnabled`.
I think the rules should be clear there - unless we do explicit string
concatenation (which enforces calling `toString()` for each element) or we call
something more than just trivial getters, we should have `if`, otherwise we can
skip it because it brings nothing.
On the other hand, `Logger` has methods with signatures:
```java
trace(String msg, Object ... paramas);
trace(String msg, Throwable t);
```
so basically if we want to pass a throwable we cannot make use of passing
string format parameters as objects without the need to early call their
`toString()` and actually doing manual string concatenation.
in particular, there is lack of a method like:
```java
trace(String msg, Throwable t, Object ... params)
```
Such a method could be useful in that context.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
