-
Notifications
You must be signed in to change notification settings - Fork 13.8k
[FLINK-15068][rocksdb] stop writing to RocksDB logs by default #10437
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
Conversation
This commit changes the default RocksDB configuration for all PredefinedOptions so that they use log level HEADER_LEVEL and disable periodic statistics dumps to the LOG file. Please note that there is no need to also change DefaultConfigurableOptionsFactory since this is only applied after any PredefinedOptions, and there is always one - at least PredefinedOptions#DEFAULT. The problem with this file is that is will grow indefinitely until it is deleted when the job is cancelled/restarted since it lives in RocksDB's local directory. Therefore, it cannot be used for troubleshooting errors. For looking into performance, metrics are probably better in the first place. Note: Theoretically, we could even set the log level to NUM_INFO_LOG_LEVELS which even removes (most of) the headers, but although that is working, it is practically an invalid value for the log level and would be a bit hacky.
|
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit 50314f6 (Thu Dec 05 12:01:26 UTC 2019) Warnings:
Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. DetailsThe Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
|
Do we still want to put all options in the "predefined options"? I would assume that from 1.10 on, the predefined options would not be used by default any more, because we configure the memory out of the box differently. |
|
There is a discussion about adjusting this order, but for now, the change makes sense as it is. +1, will merge this... |
This commit changes the default RocksDB configuration for all PredefinedOptions so that they use log level HEADER_LEVEL and disable periodic statistics dumps to the LOG file. Please note that there is no need to also change DefaultConfigurableOptionsFactory since this is only applied after any PredefinedOptions, and there is always one - at least PredefinedOptions#DEFAULT. The problem with this file is that is will grow indefinitely until it is deleted when the job is cancelled/restarted since it lives in RocksDB's local directory. Therefore, it cannot be used for troubleshooting errors. For looking into performance, metrics are probably better in the first place. Note: Theoretically, we could even set the log level to NUM_INFO_LOG_LEVELS which even removes (most of) the headers, but although that is working, it is practically an invalid value for the log level and would be a bit hacky. This closes apache#10437
What is the purpose of the change
With Flink's default settings for RocksDB, it will write a
LOGfile (not the WAL, but pure logging statements) into the data folder. Besides periodic statistics, it will log compaction attempts, new memtable creations, flushes, etc. This file grows indefinitely and may fill the disk without this log being actually used anywhere (it will be deleted with the job anway).With this PR, we are effectively disabling the log by default. If anyone wants to retain it, it can be re-configured at will providing an own
OptionsFactory.Brief change log
PredefinedOptionsso that they use log level
HEADER_LEVELLOGfileDefaultConfigurableOptionsFactoryVerifying this change
I ran a Flink cluster with these changes and the
LOGfile now only contains some headers and is then never written to again. Normal behaviour is otherwise covered by existing tests.Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation
This change is