-
Notifications
You must be signed in to change notification settings - Fork 13k
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
[FLINK-25402] Introduce working directory for Flink processes #18083
Conversation
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit 7370627 (Fri Dec 10 16:49:12 UTC 2021) 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. The 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:
|
a9f6043
to
6d9a087
Compare
6d9a087
to
20c2702
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't gone through the whole PR, just care about how to handle multi directories from java.io.tmpdir
property.
flink-runtime/src/main/java/org/apache/flink/runtime/entrypoint/ClusterEntrypointUtils.java
Outdated
Show resolved
Hide resolved
2ebb41f
to
a0a1e32
Compare
a0a1e32
to
50f2652
Compare
flink-core/src/main/java/org/apache/flink/configuration/JobManagerOptions.java
Show resolved
Hide resolved
flink-core/src/main/java/org/apache/flink/configuration/ClusterOptions.java
Outdated
Show resolved
Hide resolved
flink-runtime/src/test/java/org/apache/flink/runtime/entrypoint/WorkingDirectoryTest.java
Outdated
Show resolved
Hide resolved
flink-core/src/main/java/org/apache/flink/configuration/CheckpointingOptions.java
Show resolved
Hide resolved
flink-runtime/src/main/java/org/apache/flink/runtime/entrypoint/WorkingDirectory.java
Show resolved
Hide resolved
...tebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBOptions.java
Show resolved
Hide resolved
flink-runtime/src/test/java/org/apache/flink/runtime/util/JvmExitOnFatalErrorTest.java
Outdated
Show resolved
Hide resolved
50f2652
to
9d0fdb4
Compare
Thanks a lot for the review @zentol. I've addressed most of your comments. The one I only addressed by commenting on it, I've left unresolved. I've pushed and update (unfortunately I screwed up and had to force push :-( ). PTAL. |
9d0fdb4
to
400d832
Compare
@zentol I've update the PR. Hopefully, I have addressed all your comments. PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please take a look at my comment to shuffle the tmp directories with multi disks.
flink-core/src/main/java/org/apache/flink/configuration/CheckpointingOptions.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 from my side.
0645606
to
de3c9fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update! LGTM.
Renames ShutdownBehaviour.STOP_APPLICATION -> ShutdownBehaviour.GRACEFUL_SHUTDOWN ShutdownBehaviour.STOP_PROCESS -> ShutdownBehaviour.PROCESS_FAILURE
…bmanager.resource-id
…onfiguring a process directory This commit introduces the configuration option to configure a working directory for the JobManager and TaskManager process. The resulting working directory will be <WORKING_DIR_BASE>/jm_<RESOURCE_ID> and <WORKING_DIR_BASE>/tm_<RESOURCE_ID> respectively.
This commit ensures that the <WORKING_DIR>/tmp directory is always empty when a process is being started.
…NG_DIR>/localState
…NG_DIR>/tmp This is a behaviour change because before the RocksDB local dir would default to io.tmp.dirs. This closes apache#18083.
de3c9fb
to
95f3f7d
Compare
…NG_DIR>/tmp This is a behaviour change because before the RocksDB local dir would default to io.tmp.dirs. This closes apache#18083.
This PR implements FLIP-198. It consists of several commits that implement the proposed changes:
b70bdca: Make JobManager process resource id configurable via jobmanager.resource-id
This commit introduces a new configuration option
jobmanager.resource-id
to control theResourceID
of the JobManager process.308bb99: Introduce ClusterOptions.PROCESS_WORKING_DIR_BASE for configuring a process directory
This commit introduces the configuration options
process.working-dir
,process.jobmanager.working-dir
andprocess.taskmanager.working-dir
to configure a working directory for the JobManagerand TaskManager process. The resulting working directory will be <WORKING_DIR_BASE>/jm_<RESOURCE_ID> and <WORKING_DIR_BASE>/tm_<RESOURCE_ID>.
45d92bb: Clear tmp directory when setting up the working directory
This commit introduces a
tmp
directory that will be created in each working directory. Moreover, this tmp directory is cleaned whenever the process restarts.f51113f: Let taskmanager.state.local.root-dirs default to <WORKING_DIR>/localState
This commit makes
<WORKING_DIR>/localState
the default fortaskmanager.state.local.root-dirs
. That way the local state will be stored in the working directory if not explicitly configured.dd6db5f: Set state.backend.rocksdb.localdir to default to <WORKING_DIR>/tmp
This commit makes
<WORKING_DIR>/tmp
the default forstate.backend.rocksdb.localdir
if not explicitly configured. This is a behaviour change because before the RocksDB local dir would default to io.tmp.dirs. With this change, the default RocksDB working directory is colocated on the same volume as the local state directory.