[FLINK-36278] Reduce log size #119
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AHeise
force-pushed
the
FLINK-36278-reduce-log-size
branch
from
273a68a to
bc5f8b9
Compare
|
Looking at the current test failures of this PR the log size (https://github.com/apache/flink-connector-kafka/actions/runs/10881066111/job/30189211552?pr=119) is still 50MB+. I wonder if this PR fixes the issue. |
AHeise
force-pushed
the
FLINK-36278-reduce-log-size
branch
3 times, most recently
from
61b0c89 to
6501a1e
Compare
AHeise
force-pushed
the
FLINK-36278-reduce-log-size
branch
from
6501a1e to
25a2828
Compare
Currently, container logs appear under an o.a.f logger and thus are visible on CI. This results in compressed log size >40MB for a run and often leads to download errors. This PR reroutes container logs to a special container logger. It also uses a custom format to significantly reduce the size of each log line. The logs for containers are disabled by default.
We should never use INFO for tracking records on the hotpath. Ideally, we would use trace but for this commit I just decreased to DEBUG to minimize the impact on production settings (is it even possible to leave production on INFO currently?).
AHeise
force-pushed
the
FLINK-36278-reduce-log-size
branch
from
25a2828 to
0454a0f
Compare
AHeise
changed the title
fapaul
approved these changes
Sep 19, 2024
| return LoggerFactory.getLogger("container." + containerName); | ||
| } | ||
|
|
||
| public static Logger getLogger(String type, Class<?> testClass) { |
There was a problem hiding this comment.
Afaict this method is always called with "flink" as a first parameter. WDY about removing the type parameter and always set it to flink when called with a testClass.
There was a problem hiding this comment.
Good idea. I'll rename to getFlinkLogger then.
Comment on lines
40
to
41
| # For old school kafka test that spawn the test server in the same JVM | ||
| logger.kafka2.name = kafka |
There was a problem hiding this comment.
Which tests still spawn kafka in the same JVM?
There was a problem hiding this comment.
Errr it seems none. Good catch.
We still have test dependencies on kafka server and zookeeper that I can probably remove and with it these log confs.
AHeise
force-pushed
the
FLINK-36278-reduce-log-size
branch
from
aebc234 to
0f95d4c
Compare
Reduce the information that Kafka consumer and producer is logging + Kafka server for old school tests.
Keep INFO only for connector related logs. This will avoid all the different JM and TM logs that deal with task life-cycles. Exceptional things still bubble up as warnings.
…ithCancellation Make a copy of AbstractPartitionDiscoverer#getAllTopics before modifying it.
AHeise
force-pushed
the
FLINK-36278-reduce-log-size
branch
from
b052d72 to
ffa72b4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, container logs appear under an o.a.f logger and thus are visible on CI. This results in compressed log size >40MB for a run and often leads to download errors.
This PR reroutes container logs to a special container logger. It also uses a custom format to significantly reduce the size of each log line. The logs for containers are disabled by default.
Here is the original log of an ITCase (308 KB)
old.log
New formatter, same level (199 KB)
with container.log
Disabled container log (113 KB)
no container.log