vy commented on code in PR #4071:
URL: https://github.com/apache/logging-log4j2/pull/4071#discussion_r3257810908
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/CommonsCompressAction.java:
##########
@@ -65,12 +70,32 @@ public final class CommonsCompressAction extends
AbstractAction {
*/
public CommonsCompressAction(
final String name, final File source, final File destination,
final boolean deleteSource) {
+ this(name, source, destination, deleteSource, 0);
+ }
+
+ /**
+ * Creates new instance of Bzip2CompressAction.
+ *
+ * @param name the compressor name. One of "gz", "bzip2", "xz", "zst",
"pack200", or "deflate".
+ * @param source file to compress, may not be null.
+ * @param destination compressed file, may not be null.
+ * @param deleteSource if true, attempt to delete file on completion.
Failure to delete does not cause an exception
+ * to be thrown or affect return value.
+ * @param maxDelaySeconds maximum delay in seconds before compression.
+ */
+ public CommonsCompressAction(
Review Comment:
Missing `@since`. (Add this to the bottom of the Javadoc, not before
`@param`s.)
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/AbstractAction.java:
##########
@@ -82,6 +82,25 @@ public synchronized void close() {
interrupted = true;
}
+ /**
+ * Blocks the current thread for a random delay up to maxDelaySeconds.
+ *
+ * @since2.26.0
+ * @param maxDelaySeconds maximum delay in seconds before returning.
Review Comment:
```suggestion
* Blocks the current thread for a random delay up to {@code
maxDelaySeconds}.
*
* @param maxDelaySeconds maximum delay in seconds before returning.
* @since 2.26.0
```
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/ZipCompressAction.java:
##########
@@ -51,23 +51,63 @@ public final class ZipCompressAction extends AbstractAction
{
*/
private final int level;
+ /**
+ * Maximum delay in seconds before compression.
+ */
+ private final int maxDelaySeconds;
+
+ /**
+ * Validates that the compression level is in the valid range [-1, 9].
+ *
+ * @param level the compression level to validate
+ * @return the level if valid
+ * @throws IllegalArgumentException if level is not in the range [-1, 9]
+ */
+ private static int checkLevel(final int level) {
+ if (level < java.util.zip.Deflater.DEFAULT_COMPRESSION || level > 9) {
+ throw new IllegalArgumentException("Compression level must be in
the range [-1, 9], got: " + level);
+ }
+ return level;
+ }
+
/**
* Creates new instance of GzCompressAction.
*
* @param source file to compress, may not be null.
* @param destination compressed file, may not be null.
* @param deleteSource if true, attempt to delete file on completion.
Failure to delete does not cause an exception
* to be thrown or affect return value.
- * @param level TODO
+ * @param level the compression level
+ * @since2.26.0
+ * @param maxDelaySeconds maximum delay in seconds before compression.
Review Comment:
```suggestion
* @param maxDelaySeconds maximum delay in seconds before compression.
* @since 2.26.0
```
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java:
##########
@@ -359,6 +363,11 @@ public Builder withConfig(final Configuration config) {
this.config = config;
return this;
}
+
+ public Builder setMaxCompressionDelaySeconds(final int
maxCompressionDelaySeconds) {
Review Comment:
Add `@since`.
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java:
##########
@@ -468,6 +482,39 @@ protected DefaultRolloverStrategy(
final Action[] customActions,
final boolean stopCustomActionsOnError,
final String tempCompressedFilePatternString) {
+ this(
+ minIndex,
+ maxIndex,
+ useMax,
+ compressionLevel,
+ strSubstitutor,
+ customActions,
+ stopCustomActionsOnError,
+ tempCompressedFilePatternString,
+ 0);
+ }
+
+ /**
+ * Constructs a new instance.
+ *
+ * @param minIndex The minimum index.
+ * @param maxIndex The maximum index.
+ * @param customActions custom actions to perform asynchronously after
rollover
+ * @param stopCustomActionsOnError whether to stop executing asynchronous
actions if an error occurs
+ * @param tempCompressedFilePatternString File pattern of the working file
+ * used during compression, if null no
temporary file are used
+ * @param maxCompressionDelaySeconds maximum delay in seconds before
compression.
+ */
Review Comment:
```suggestion
* @since 2.27.0
*/
```
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/GzCompressAction.java:
##########
@@ -33,6 +33,10 @@ public final class GzCompressAction extends AbstractAction {
private static final int BUF_SIZE = 8192;
+ private static final int MIN_COMPRESSION_LEVEL =
Deflater.DEFAULT_COMPRESSION;
+
+ private static final int MAX_COMPRESSION_LEVEL = Deflater.BEST_COMPRESSION;
Review Comment:
No need for these constants. They're used only at one method, you can place
them there as locals.
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/ZipCompressAction.java:
##########
@@ -78,6 +118,16 @@ public ZipCompressAction(final File source, final File
destination, final boolea
*/
@Override
public boolean execute() throws IOException {
+ if (maxDelaySeconds > 0) {
Review Comment:
Why aren't we using `AbstractAction::blockThread`?
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java:
##########
@@ -507,6 +555,10 @@ public PatternProcessor getTempCompressedFilePattern() {
return tempCompressedFilePattern;
}
+ public int getMaxCompressionDelaySeconds() {
Review Comment:
Add `@since`.
--
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]
