katstack commented on PR #4234: URL: https://github.com/apache/logging-log4j2/pull/4234#issuecomment-6045524635
Before I make another code change, could we agree on the semantics for unsupported `compressionLevel` values? | Format | Accepted configured levels | Effective default for `-1` | Meaning of `0` | Behavior before this branch | Proposed handling of unsupported values | | --- | --- | --- | --- | --- | --- | | ZIP | `-1`, `0..9` | JDK `Deflater` default | No compression | Valid levels were applied. Unsupported values were rejected while creating the rollover action, which could lose the triggering event. | Fall back to the default and log a warning asynchronously. | | GZip | `-1`, `0..9` | JDK `Deflater` default | No compression | Valid levels were applied. Unsupported values were rejected while creating the rollover action, which could lose the triggering event. | Fall back to the default and log a warning asynchronously. | | BZip2 | `-1`, `1..9` | `9` | Unsupported | `compressionLevel` was ignored; compression always used the Commons Compress default level `9`. | Fall back to `9` and log a warning asynchronously. | | Deflate | `-1`, `0..9` | JDK `Deflater` default | No compression | `compressionLevel` was ignored; compression always used the Commons Compress / JDK default. | Fall back to the default and log a warning asynchronously. | | Zstd | `-1`, `0`, `1..ZSTD_CLEVEL_MAX` | `ZSTD_CLEVEL_DEFAULT` (currently `3`) | Zstd default | `compressionLevel` was ignored; compression always used the Commons Compress / Zstd default. | Fall back to the default and log a warning asynchronously. | I would prefer to avoid format-specific coercions. `-1` would remain the Log4j sentinel for selecting the format-specific default, and supported values would be passed through unchanged. Zstd would be the only explicit mapping: its native `-1` represents a fast-compression level, while Log4j uses `-1` for an unspecified level, so Log4j's `-1` needs to map to the Zstd default. Other negative Zstd fast-compression levels would remain unsupported for now and use the fallback behavior above. With this approach, BZip2 level `0` would not be coerced to level `1`; it would be treated as unsupported and fall back to BZip2's default level (`9`). Does that match the intended contract for the shared `compressionLevel` attribute? -- 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]
