Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The immediateFlush option in both rolling file appenders is ignored #2025

Open
jvz opened this issue Dec 1, 2023 · 8 comments
Open

The immediateFlush option in both rolling file appenders is ignored #2025

jvz opened this issue Dec 1, 2023 · 8 comments
Labels
appenders:Rolling Affects log file rolling functionality bug Incorrect, unexpected, or unintended behavior of existing code

Comments

@jvz
Copy link
Member

jvz commented Dec 1, 2023

Description

Both the RollingFileAppender and RollingRandomAccessFileAppender plugins offer a boolean flag, immediateFlush, to allow for immediate flushing of buffers when writing data. This option is ultimately discarded when creating the corresponding RollingFileManager and RollingRandomAccessFileManager instances.

Configuration

Version: 2.x and main

Operating system: all

JDK: all

Logs

n/a

Reproduction

The amusing part is that we have tons of tests that set immediateFlush to true or false, yet none of these tests verify that it does anything.

@jvz jvz added bug Incorrect, unexpected, or unintended behavior of existing code appenders:Rolling Affects log file rolling functionality labels Dec 1, 2023
@ppkarwasz
Copy link
Contributor

The property is used in AbstractOutputStreamAppender#directEncodeEvent and AbstractOutputStreamAppender#writeByteArrayToManager. As far as I can tell, it is not ignored.

@jvz
Copy link
Member Author

jvz commented Dec 1, 2023

Which is why static analysis is complaining about what exactly?
image

@ppkarwasz
Copy link
Contributor

ppkarwasz commented Dec 1, 2023

The SAST tool is right: RollingFileManager doesn't do anything with the property, but RollingFileAppender does.

It would be useful to remove the property from RollingFileManager since it is not used.

@jvz
Copy link
Member Author

jvz commented Dec 1, 2023

Oh, then consider this a bug for that. Unused fields are a bug.

@tcmot
Copy link

tcmot commented Dec 21, 2023

Hi,

RandomAccessFile 

FileChannel channel = randomAccessFile.getChannel();
randomAccessFile.write();
channel.force(true);

Use the force method,the file be updated in real time. But affects performance.
Log4j2,not use the force method.

Rewrite RollingRandomAccessFileManager and RandomAccessFileManager flush method. Add the code above.

@ppkarwasz
Copy link
Contributor

@jvz,

I investigated the effects of flush() on the FileAppender a little bit further and it is a no-op in FileOutputStream (the output stream does not use any Java buffers, so there is nothing to flush). So I would propose to:

  • remove immediateFlush from FileAppender.Builder and similar,
  • remove any references to immediateFlush in the documentation.

@jvz
Copy link
Member Author

jvz commented Dec 26, 2023

@ppkarwasz sounds like a good idea to me!

@ppkarwasz
Copy link
Contributor

Actually there is another usage of immediateFlush: resource managers use a ByteBuffer to format each log event.

If immediateFlush is true, the byte buffer is flushed at each event.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
appenders:Rolling Affects log file rolling functionality bug Incorrect, unexpected, or unintended behavior of existing code
Projects
None yet
Development

No branches or pull requests

3 participants