You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "Andrey Turbanov (Jira)" <ji...@apache.org> on 2020/01/31 23:44:00 UTC

[jira] [Commented] (LOG4J2-2769) AtomicMoveNotSupportedException shouldn't be logged as ERROR

    [ https://issues.apache.org/jira/browse/LOG4J2-2769?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17027886#comment-17027886 ] 

Andrey Turbanov commented on LOG4J2-2769:
-----------------------------------------

Proposed patch:
{code}
Index: log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameAction.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameAction.java	(revision ba14e2a44edcbca73cc70b5d56af73e5b22ef238)
+++ log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameAction.java	(date 1580514196922)
@@ -19,6 +19,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.nio.file.AtomicMoveNotSupportedException;
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.nio.file.StandardCopyOption;
@@ -121,43 +122,46 @@
                     LOGGER.trace("Renamed file {} to {} with Files.move", source.getAbsolutePath(),
                             destination.getAbsolutePath());
                     return true;
+                } catch (final AtomicMoveNotSupportedException exMove) {
+                    LOGGER.info("Unable to atomic move file {} to {}: {}", source.getAbsolutePath(),
+                            destination.getAbsolutePath(), exMove.getMessage());
                 } catch (final IOException exMove) {
                     LOGGER.error("Unable to move file {} to {}: {} {}", source.getAbsolutePath(),
                             destination.getAbsolutePath(), exMove.getClass().getName(), exMove.getMessage());
-                    boolean result = source.renameTo(destination);
-                    if (!result) {
-                        try {
-                            Files.copy(Paths.get(source.getAbsolutePath()), Paths.get(destination.getAbsolutePath()),
-                                    StandardCopyOption.REPLACE_EXISTING);
-                            try {
-                                Files.delete(Paths.get(source.getAbsolutePath()));
-                                result = true;
-                                LOGGER.trace("Renamed file {} to {} using copy and delete",
-                                        source.getAbsolutePath(), destination.getAbsolutePath());
-                            } catch (final IOException exDelete) {
-                                LOGGER.error("Unable to delete file {}: {} {}", source.getAbsolutePath(),
-                                        exDelete.getClass().getName(), exDelete.getMessage());
-                                try {
-                                    new PrintWriter(source.getAbsolutePath()).close();
-                                    result = true;
-                                    LOGGER.trace("Renamed file {} to {} with copy and truncation",
-                                            source.getAbsolutePath(), destination.getAbsolutePath());
-                                } catch (final IOException exOwerwrite) {
-                                    LOGGER.error("Unable to overwrite file {}: {} {}",
-                                            source.getAbsolutePath(), exOwerwrite.getClass().getName(),
-                                            exOwerwrite.getMessage());
-                                }
-                            }
-                        } catch (final IOException exCopy) {
-                            LOGGER.error("Unable to copy file {} to {}: {} {}", source.getAbsolutePath(),
-                                    destination.getAbsolutePath(), exCopy.getClass().getName(), exCopy.getMessage());
-                        }
-                    } else {
-                        LOGGER.trace("Renamed file {} to {} with source.renameTo",
-                                source.getAbsolutePath(), destination.getAbsolutePath());
-                    }
-                    return result;
-                }
+                }
+                boolean result = source.renameTo(destination);
+                if (!result) {
+                    try {
+                        Files.copy(Paths.get(source.getAbsolutePath()), Paths.get(destination.getAbsolutePath()),
+                                StandardCopyOption.REPLACE_EXISTING);
+                        try {
+                            Files.delete(Paths.get(source.getAbsolutePath()));
+                            result = true;
+                            LOGGER.trace("Renamed file {} to {} using copy and delete",
+                                    source.getAbsolutePath(), destination.getAbsolutePath());
+                        } catch (final IOException exDelete) {
+                            LOGGER.error("Unable to delete file {}: {} {}", source.getAbsolutePath(),
+                                    exDelete.getClass().getName(), exDelete.getMessage());
+                            try {
+                                new PrintWriter(source.getAbsolutePath()).close();
+                                result = true;
+                                LOGGER.trace("Renamed file {} to {} with copy and truncation",
+                                        source.getAbsolutePath(), destination.getAbsolutePath());
+                            } catch (final IOException exOwerwrite) {
+                                LOGGER.error("Unable to overwrite file {}: {} {}",
+                                        source.getAbsolutePath(), exOwerwrite.getClass().getName(),
+                                        exOwerwrite.getMessage());
+                            }
+                        }
+                    } catch (final IOException exCopy) {
+                        LOGGER.error("Unable to copy file {} to {}: {} {}", source.getAbsolutePath(),
+                                destination.getAbsolutePath(), exCopy.getClass().getName(), exCopy.getMessage());
+                    }
+                } else {
+                    LOGGER.trace("Renamed file {} to {} with source.renameTo",
+                            source.getAbsolutePath(), destination.getAbsolutePath());
+                }
+                return result;
             } catch (final RuntimeException ex) {
                 LOGGER.error("Unable to rename file {} to {}: {} {}", source.getAbsolutePath(),
                         destination.getAbsolutePath(), ex.getClass().getName(), ex.getMessage());
{code}

> AtomicMoveNotSupportedException shouldn't be logged as ERROR
> ------------------------------------------------------------
>
>                 Key: LOG4J2-2769
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-2769
>             Project: Log4j 2
>          Issue Type: Improvement
>          Components: Appenders
>    Affects Versions: 2.13.0
>            Reporter: Andrey Turbanov
>            Priority: Minor
>             Fix For: 2.13.1
>
>
> When RollingFileAppender moves file to another folder, it tries to use [ATOMIC_MOVE|https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/nio/file/StandardCopyOption.html#ATOMIC_MOVE] option as most strong one approach.
> In many common production cases this approach *always* fails, and {{RollingFileAppender}} fallbacks (which is good) to use [File.renameTo|https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/io/File.html#renameTo(java.io.File)] method. One of such common case - using network filesystem for _cold_ logs (E.g. CIFS).
> When atomic move fails it prints error to the log. Something like this:
> {code}
> ERROR Unable to move file /opt/log/console.60465.pp1.log to /opt/case/20200130/console.60465.pp1.20200130_193421.log: java.nio.file.AtomicMoveNotSupportedException /opt/log/console.60465.pp1.log -> /opt/case/20200130/console.60465.pp1.20200130_193421.log: Invalid cross-device link
> {code}
> Such logs are very distracting and can lead to unnecessary monitoring alerts.
> I suggest to lower logging level for such case to INFO or WARN. As it's *expected* scenario.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)