Hi, Zakelly. Thanks for driving this. Overall LGTM as we discussed offline.
Some comments/suggestions just came to mind: 1. Could execution.recovery also contain some other behaviors about recovery ? e.g. restart-strategy. 2. Could we also remove some legacy configuration value ? e.g. LEGACY Mode for execution.savepoint-restore-mode/execution.recovery.claim-mode. 3. Could the local checkpoint be cleaned if execution.checkpointing.local-copy.enabled is true and execution.recovery.from-local is false ? I found it's also an issue if current local-recovery from enabled to disabled. Maybe another ticket is needed. 4. +1 for enabling execution.checkpointing.incremental by default which is basically default configuration in our production environment. On Mon, Jan 8, 2024 at 6:06 PM Zakelly Lan <zakelly....@gmail.com> wrote: > Hi Yun, > > Thanks for your comments! > > 1. We shall not describe the configuration with its implementation for > > 'execution.checkpointing.local-copy.*' options, for hashmap > state-backend, > > it would write two streams and for Rocksdb state-backend, it would use > > hard-link for backup. Thus, I think > > 'execution.checkpointing.local-backup.*' looks better. > > I agreed that we'd better name the option in user's perspective instead of > the implementation, thus I name it as a copy of the checkpoint in the > local disk, regardless of the way of generating it. The word 'backup' is > also suitable for this case, so I agree to change to > 'execution.checkpointing.local-backup.*' if no one objects. > > 2. What does the 'execution.checkpointing.data-inline-threshold' mean? It > > seems not so easy to understand. > > The 'execution.checkpointing.data-inline-threshold' (original one as > 'state.storage.fs.memory-threshold') stands for the size threshold below > which state chunks will store inline with the metadata, thus I call it > 'data-inline-threshold'. > > > Best, > Zakelly > > On Mon, Jan 8, 2024 at 10:09 AM Yun Tang <myas...@live.com> wrote: > > > Hi Zakelly, > > > > Thanks for driving this topic. I have two concerns here: > > > > 1. We shall not describe the configuration with its implementation for > > 'execution.checkpointing.local-copy.*' options, for hashmap > state-backend, > > it would write two streams and for Rocksdb state-backend, it would use > > hard-link for backup. Thus, I think > > 'execution.checkpointing.local-backup.*' looks better. > > 2. What does the 'execution.checkpointing.data-inline-threshold' mean? > > It seems not so easy to understand. > > > > Best > > Yun Tang > > ________________________________ > > From: Piotr Nowojski <pnowoj...@apache.org> > > Sent: Thursday, January 4, 2024 22:37 > > To: dev@flink.apache.org <dev@flink.apache.org> > > Subject: Re: [DISCUSS] FLIP-406: Reorganize State & Checkpointing & > > Recovery Configuration > > > > Hi, > > > > Thanks for trying to clean this up! I don't have strong opinions on the > > topics discussed here, so generally speaking +1 from my side! > > > > Best, > > Piotrek > > > > śr., 3 sty 2024 o 04:16 Rui Fan <1996fan...@gmail.com> napisał(a): > > > > > Thanks for the feedback! > > > > > > Using the `execution.checkpointing.incremental.enabled`, > > > and enabling it by default sounds good to me. > > > > > > Best, > > > Rui > > > > > > On Wed, Jan 3, 2024 at 11:10 AM Zakelly Lan <zakelly....@gmail.com> > > wrote: > > > > > > > Hi Rui, > > > > > > > > Thanks for your comments! > > > > > > > > IMO, given that the state backend can be plugably loaded (as you can > > > > specify a state backend factory), I prefer not providing state > backend > > > > specified options in the framework. > > > > > > > > Secondly, the incremental checkpoint is actually a sharing file > > strategy > > > > across checkpoints, which means the state backend *could* reuse files > > > from > > > > previous cp but not *must* do so. When the state backend could not > > reuse > > > > the files, it is reasonable to fallback to a full checkpoint. > > > > > > > > Thus, I suggest we make it `execution.checkpointing.incremental` and > > > enable > > > > it by default. For those state backends not supporting this, they > > perform > > > > full checkpoints and print a warning to inform users. Users do not > need > > > to > > > > pay special attention to different options to control this across > > > different > > > > state backends. This is more user-friendly in my opinion. WDYT? > > > > > > > > On Tue, Jan 2, 2024 at 10:49 AM Rui Fan <1996fan...@gmail.com> > wrote: > > > > > > > > > Hi Zakelly, > > > > > > > > > > I'm not sure whether we could add the state backend type in the > > > > > new key name of state.backend.incremental. It means we use > > > > > `execution.checkpointing.rocksdb-incremental` or > > > > > `execution.checkpointing.rocksdb-incremental.enabled`. > > > > > > > > > > So far, state.backend.incremental only works for rocksdb state > > backend. > > > > > And this feature or optimization is very valuable and huge for > large > > > > > state flink jobs. I believe it's enabled for most production flink > > jobs > > > > > with large rocksdb state. > > > > > > > > > > If this option isn't generic for all state backend types, I guess > we > > > > > can enable `execution.checkpointing.rocksdb-incremental.enabled` > > > > > by default in Flink 2.0. > > > > > > > > > > But if it works for all state backends, it's hard to enable it by > > > > default. > > > > > Enabling great and valuable features or improvements are useful > > > > > for users, especially a lot of new flink users. Out-of-the-box > > options > > > > > are good for users. > > > > > > > > > > WDYT? > > > > > > > > > > Best, > > > > > Rui > > > > > > > > > > On Fri, Dec 29, 2023 at 1:45 PM Zakelly Lan <zakelly....@gmail.com > > > > > > wrote: > > > > > > > > > > > Hi everyone, > > > > > > > > > > > > Thanks all for your comments! > > > > > > > > > > > > As many of you have questions about the names for boolean > options, > > I > > > > > > suggest we make a naming rule for them. For now I could think of > > > three > > > > > > options: > > > > > > > > > > > > Option 1: Use enumeration options if possible. But this may cause > > > some > > > > > name > > > > > > collisions or confusion as we discussed and we should unify the > > > > statement > > > > > > everywhere. > > > > > > Option 2: Use boolean options and add 'enabled' as the suffix. > > > > > > Option 3: Use boolean options and ONLY add 'enabled' when there > are > > > > more > > > > > > detailed configurations under the same prefix, to prevent one > name > > > from > > > > > > serving as a prefix to another. > > > > > > > > > > > > I am slightly inclined to Option 3, since it is more in line with > > > > current > > > > > > practice and friendly for existing users. Also It reduces the > > length > > > of > > > > > > configuration names as much as possible. I really want to hear > your > > > > > > opinions. > > > > > > > > > > > > > > > > > > @Xuannan > > > > > > > > > > > > I agree with your comments 1 and 3. > > > > > > > > > > > > For 2, If we decide to change the name, maybe > > > > > > `execution.checkpointing.parallel-cleaner` is better? And as for > > > > whether > > > > > to > > > > > > add 'enabled' I suggest we discuss the rule above. WDYT? > > > > > > Thanks! > > > > > > > > > > > > > > > > > > Best, > > > > > > Zakelly > > > > > > > > > > > > On Fri, Dec 29, 2023 at 12:02 PM Xuannan Su < > suxuanna...@gmail.com > > > > > > > > wrote: > > > > > > > > > > > > > Hi Zakelly, > > > > > > > > > > > > > > Thanks for driving this! The organization of the configuration > > > option > > > > > > > in the FLIP looks much cleaner and easier to understand. +1 to > > the > > > > > > > FLIP. > > > > > > > > > > > > > > Just some questions from me. > > > > > > > > > > > > > > 1. I think the change to the ConfigOptions should be put in the > > > > > > > `Public Interface` section, instead of `Proposed Changed`, as > > those > > > > > > > configuration options are public interface. > > > > > > > > > > > > > > 2. The key `state.checkpoint.cleaner.parallel-mode` seems > > > confusing. > > > > > > > It feels like it is used to choose different modes. In fact, it > > is > > > a > > > > > > > boolean flag to indicate whether to enable parallel clean. How > > > about > > > > > > > making it `state.checkpoint.cleaner.parallel-mode.enabled`? > > > > > > > > > > > > > > 3. The `execution.checkpointing.write-buffer` may better be > > > > > > > `execution.checkpointing.write-buffer-size` so that we know it > is > > > > > > > configuring the size of the buffer. > > > > > > > > > > > > > > Best, > > > > > > > Xuannan > > > > > > > > > > > > > > > > > > > > > On Wed, Dec 27, 2023 at 7:17 PM Yanfei Lei < > fredia...@gmail.com> > > > > > wrote: > > > > > > > > > > > > > > > > Hi Zakelly, > > > > > > > > > > > > > > > > > Considering the name occupation, how about naming it as > > > > > > > `execution.checkpointing.type`? > > > > > > > > > > > > > > > > `Checkpoint Type`[1,2] is used to describe aligned/unaligned > > > > > > > > checkpoint, I am inclined to make a choice between > > > > > > > > `execution.checkpointing.incremental` and > > > > > > > > `execution.checkpointing.incremental.enabled`. > > > > > > > > > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://nightlies.apache.org/flink/flink-docs-release-1.18/docs/ops/monitoring/checkpoint_monitoring/ > > > > > > > > [2] > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/flink/blob/master/flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html#L27 > > > > > > > > > > > > > > > > -- > > > > > > > > Best, > > > > > > > > Yanfei > > > > > > > > > > > > > > > > Zakelly Lan <zakelly....@gmail.com> 于2023年12月27日周三 14:41写道： > > > > > > > > > > > > > > > > > > Hi Lijie, > > > > > > > > > > > > > > > > > > Thanks for the reminder! I missed this. > > > > > > > > > > > > > > > > > > Considering the name occupation, how about naming it as > > > > > > > > > `execution.checkpointing.type`? > > > > > > > > > > > > > > > > > > Actually I think the current `execution.checkpointing.mode` > > is > > > > > > > confusing in > > > > > > > > > some ways, maybe `execution.checkpointing.data-consistency` > > is > > > > > > better. > > > > > > > > > > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > Zakelly > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Dec 27, 2023 at 12:59 PM Lijie Wang < > > > > > > wangdachui9...@gmail.com> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Hi Zakelly, > > > > > > > > > > > > > > > > > > > > >> I'm wondering if > `execution.checkpointing.savepoint-dir` > > > > would > > > > > > be > > > > > > > > > > better. > > > > > > > > > > > > > > > > > > > > `execution.checkpointing.dir` and > > > > > > > `execution.checkpointing.savepoint-dir` > > > > > > > > > > are also fine for me. > > > > > > > > > > > > > > > > > > > > >> So I think an enumeration option > > > > > `execution.checkpointing.mode` > > > > > > > which > > > > > > > > > > can be 'full' (default) or 'incremental' would be better > > > > > > > > > > > > > > > > > > > > I agree with using an enumeration option. But currently > > there > > > > is > > > > > > > already a > > > > > > > > > > configuration option called > `execution.checkpointing.mode`, > > > > which > > > > > > is > > > > > > > used > > > > > > > > > > to choose EXACTLY_ONCE or AT_LEAST_ONCE. Maybe we need to > > use > > > > > > > another name > > > > > > > > > > or merge these two options. > > > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > > Lijie > > > > > > > > > > > > > > > > > > > > Zakelly Lan <zakelly....@gmail.com> 于2023年12月27日周三 > > 11:43写道： > > > > > > > > > > > > > > > > > > > > > Hi everyone, > > > > > > > > > > > > > > > > > > > > > > Thanks all for your comments! > > > > > > > > > > > > > > > > > > > > > > @Yanfei > > > > > > > > > > > > > > > > > > > > > > > 1. For some state backends that do not support > > > incremental > > > > > > > checkpoint, > > > > > > > > > > > > how does the > execution.checkpointing.incrementaloption > > > take > > > > > > > effect? Or > > > > > > > > > > > > is it better to put incremental under > > > > > > > state.backend.xxx.incremental? > > > > > > > > > > > > > > > > > > > > > > > I'd rather not put the option for incremental > checkpoint > > > > under > > > > > > the > > > > > > > > > > > 'state.backend', since it is more about the > checkpointing > > > > > instead > > > > > > > of > > > > > > > > > > state > > > > > > > > > > > accessing. Of course, the state backend may not > > necessarily > > > > do > > > > > > > > > > incremental > > > > > > > > > > > checkpoint as requested. If the state backend is not > > > capable > > > > of > > > > > > > taking > > > > > > > > > > > incremental cp, it is better to fallback to the full > cp. > > > > > > > > > > > > > > > > > > > > > > 2. I'm a little worried that putting all configurations > > > into > > > > > > > > > > > > `ExecutionCheckpointingOptions` will introduce some > > > > > dependency > > > > > > > > > > > > problems. Some options would be used by flink-runtime > > > > module, > > > > > > but > > > > > > > > > > > > flink-runtime should not depend on > > flink-streaming-java. > > > > e.g. > > > > > > > > > > > > FLINK-28286[1]. > > > > > > > > > > > > So, I prefer to move configurations to > > > > > `CheckpointingOptions`, > > > > > > > WDYT? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, that's a very good point. Moving to > > > > > > > > > > > `CheckpointingOptions`(flink-core) makes sense. > > > > > > > > > > > > > > > > > > > > > > @Lijie > > > > > > > > > > > > > > > > > > > > > > How about > > > > > > > > > > > > state.savepoints.dir -> > > > > execution.checkpointing.savepoint.dir > > > > > > > > > > > > state.checkpoints.dir -> > > > > > execution.checkpointing.checkpoint.dir > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Actually, I think the `checkpointing.checkpoint` may > > cause > > > > some > > > > > > > > > > confusion. > > > > > > > > > > > But I'm ok if others agree. > > > > > > > > > > > I'm wondering if > `execution.checkpointing.savepoint-dir` > > > > would > > > > > be > > > > > > > better. > > > > > > > > > > > WDYT? > > > > > > > > > > > > > > > > > > > > > > 2. We changed the execution.checkpointing.local-copy' > to > > > > > > > > > > > > 'execution.checkpointing.local-copy.enabled'. Should > we > > > > also > > > > > > add > > > > > > > > > > > "enabled" > > > > > > > > > > > > suffix for other boolean type configuration options ? > > For > > > > > > > example, > > > > > > > > > > > > execution.checkpointing.incremental -> > > > > > > > > > > > > execution.checkpointing.incremental.enabled > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Actually, the incremental cp is something like > choosing a > > > > mode > > > > > > for > > > > > > > doing > > > > > > > > > > > checkpoint instead of enabling a function. So I think > an > > > > > > > enumeration > > > > > > > > > > option > > > > > > > > > > > `execution.checkpointing.mode` which can be 'full' > > > (default) > > > > or > > > > > > > > > > > 'incremental' would be better, WDYT? > > > > > > > > > > > And @Rui Fan @Yanfei What do you think about this? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Dec 26, 2023 at 5:15 PM Lijie Wang < > > > > > > > wangdachui9...@gmail.com> > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > Hi Zakelly, > > > > > > > > > > > > > > > > > > > > > > > > Thanks for driving the discussion. > > > > > > > > > > > > > > > > > > > > > > > > 1. > > > > > > > > > > > > >> But I'm not so sure since there is only one > > > > > > savepoint-related > > > > > > > > > > option. > > > > > > > > > > > > Maybe someone else could share some thoughts here. > > > > > > > > > > > > > > > > > > > > > > > > How about > > > > > > > > > > > > state.savepoints.dir -> > > > > execution.checkpointing.savepoint.dir > > > > > > > > > > > > state.checkpoints.dir -> > > > > > execution.checkpointing.checkpoint.dir > > > > > > > > > > > > > > > > > > > > > > > > 2. We changed the execution.checkpointing.local-copy' > > to > > > > > > > > > > > > 'execution.checkpointing.local-copy.enabled'. Should > we > > > > also > > > > > > add > > > > > > > > > > > "enabled" > > > > > > > > > > > > suffix for other boolean type configuration options ? > > For > > > > > > > example, > > > > > > > > > > > > execution.checkpointing.incremental -> > > > > > > > > > > > > execution.checkpointing.incremental.enabled > > > > > > > > > > > > > > > > > > > > > > > > In this way, the naming style of configuration > options > > is > > > > > > > unified, and > > > > > > > > > > it > > > > > > > > > > > > can avoid potential similar problems (for example, we > > may > > > > > need > > > > > > > to add > > > > > > > > > > > more > > > > > > > > > > > > options for incremental checkpoint in the future). > > > > > > > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > > > > Lijie > > > > > > > > > > > > > > > > > > > > > > > > Yanfei Lei <fredia...@gmail.com> 于2023年12月26日周二 > > 12:05写道： > > > > > > > > > > > > > > > > > > > > > > > > > Hi Zakelly, > > > > > > > > > > > > > > > > > > > > > > > > > > Thank you for creating the FLIP and starting the > > > > > discussion. > > > > > > > > > > > > > > > > > > > > > > > > > > The current arrangement of these options is indeed > > > > somewhat > > > > > > > > > > haphazard, > > > > > > > > > > > > > and the new arrangement looks much better. I have > > some > > > > > > > questions > > > > > > > > > > about > > > > > > > > > > > > > the arrangement of some new configuration options: > > > > > > > > > > > > > > > > > > > > > > > > > > 1. For some state backends that do not support > > > > incremental > > > > > > > > > > checkpoint, > > > > > > > > > > > > > how does the > > execution.checkpointing.incrementaloption > > > > take > > > > > > > effect? > > > > > > > > > > Or > > > > > > > > > > > > > is it better to put incremental under > > > > > > > state.backend.xxx.incremental? > > > > > > > > > > > > > > > > > > > > > > > > > > 2. I'm a little worried that putting all > > configurations > > > > > into > > > > > > > > > > > > > `ExecutionCheckpointingOptions` will introduce some > > > > > > dependency > > > > > > > > > > > > > problems. Some options would be used by > flink-runtime > > > > > module, > > > > > > > but > > > > > > > > > > > > > flink-runtime should not depend on > > > flink-streaming-java. > > > > > e.g. > > > > > > > > > > > > > FLINK-28286[1]. > > > > > > > > > > > > > So, I prefer to move configurations to > > > > > > `CheckpointingOptions`, > > > > > > > WDYT? > > > > > > > > > > > > > > > > > > > > > > > > > > [1] > > https://issues.apache.org/jira/browse/FLINK-28286 > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > > Best, > > > > > > > > > > > > > Yanfei > > > > > > > > > > > > > > > > > > > > > > > > > > Zakelly Lan <zakelly....@gmail.com> 于2023年12月25日周一 > > > > > 21:14写道： > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Rui Fan and Junrui, > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the reminder! I agree to change the > > > > > > > > > > > > > > 'execution.checkpointing.local-copy' to > > > > > > > > > > > > > > 'execution.checkpointing.local-copy.enabled'. > > > > > > > > > > > > > > > > > > > > > > > > > > > > And for other suggestions Rui proposed: > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. How about execution.checkpointing.storage.type > > > > instead > > > > > > > > > > > > > > > of execution.checkpointing.storage? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Ah, I missed something here. Actually I suggest > we > > > > could > > > > > > > merge the > > > > > > > > > > > > > current > > > > > > > > > > > > > > 'state.checkpoints.dir' and > > > 'state.checkpoint-storage' > > > > > into > > > > > > > one URI > > > > > > > > > > > > > > configuration named > 'execution.checkpointing.dir'. > > > > WDYT? > > > > > > > > > > > > > > > > > > > > > > > > > > > > 3. execution.checkpointing.savepoint.dir is a > > little > > > > > weird. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, I think it is better to make 'savepoint' and > > > > > > > 'checkpoint' the > > > > > > > > > > > same > > > > > > > > > > > > > > level. But I'm not so sure since there is only > one > > > > > > > > > > savepoint-related > > > > > > > > > > > > > > option. Maybe someone else could share some > > thoughts > > > > > here. > > > > > > > > > > > > > > > > > > > > > > > > > > > > 4. How about execution.recovery.claim-mode > instead > > of > > > > > > > > > > > > > > > execution.recovery.mode? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Agreed. That's more accurate. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Many thanks for your suggestions! > > > > > > > > > > > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > > > > > > Zakelly > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 25, 2023 at 8:18 PM Junrui Lee < > > > > > > > jrlee....@gmail.com> > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Zakelly, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for driving this. I agree that the > > proposed > > > > > > > restructuring > > > > > > > > > > of > > > > > > > > > > > > the > > > > > > > > > > > > > > > configuration options is largely positive. It > > will > > > > make > > > > > > > > > > > understanding > > > > > > > > > > > > > and > > > > > > > > > > > > > > > working with Flink configurations more > intuitive. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Most of the proposed changes look great. Just a > > > > > heads-up, > > > > > > > as Rui > > > > > > > > > > > Fan > > > > > > > > > > > > > > > mentioned, Flink currently requires that no > > > > > > configOption's > > > > > > > key be > > > > > > > > > > > the > > > > > > > > > > > > > > > prefix of another to avoid issues when we > > > eventually > > > > > > adopt > > > > > > > a > > > > > > > > > > > standard > > > > > > > > > > > > > YAML > > > > > > > > > > > > > > > parser, as detailed in FLINK-29372 ( > > > > > > > > > > > > > > > > > https://issues.apache.org/jira/browse/FLINK-29372 > > > ). > > > > > > > Therefore, > > > > > > > > > > > it's > > > > > > > > > > > > > better > > > > > > > > > > > > > > > to change the key > > > > 'execution.checkpointing.local-copy' > > > > > > > because it > > > > > > > > > > > > > serves as > > > > > > > > > > > > > > > a prefix to the key > > > > > > > 'execution.checkpointing.local-copy.dir'. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Best regards, > > > > > > > > > > > > > > > Junrui > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Rui Fan <1996fan...@gmail.com> 于2023年12月25日周一 > > > > 19:11写道： > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Zakelly, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thank you for driving this proposal! > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Overall good for me. I have some questions > > about > > > > > these > > > > > > > names. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. How about > > execution.checkpointing.storage.type > > > > > > > instead of > > > > > > > > > > > > > > > > execution.checkpointing.storage? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It's similar to state.backend.type. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2. How about > > > > > execution.checkpointing.local-copy.enabled > > > > > > > instead > > > > > > > > > > > of > > > > > > > > > > > > > > > > execution.checkpointing.local-copy? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > You added a new option: > > > > > > > execution.checkpointing.local-copy.dir. > > > > > > > > > > > > > > > > IIUC, one option name shouldn't be the prefix > > of > > > > > other > > > > > > > options. > > > > > > > > > > > > > > > > If you add a new option > > > > > > > execution.checkpointing.local-copy, > > > > > > > > > > > > > > > > flink CI will fail directly. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 3. execution.checkpointing.savepoint.dir is a > > > > little > > > > > > > weird. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > For old options: state.savepoints.dir and > > > > > > > > > > state.checkpoints.dir, > > > > > > > > > > > > > > > > the savepoint and checkpoint are the same > > level. > > > It > > > > > > means > > > > > > > > > > > > > > > > it's a checkpoint or savepoint. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The new option execution.checkpointing.dir is > > > fine > > > > > for > > > > > > > me. > > > > > > > > > > > > > > > > However, > execution.checkpointing.savepoint.dir > > > is a > > > > > > > little > > > > > > > > > > weird. > > > > > > > > > > > > > > > > I don't know which name is better now. Let us > > > think > > > > > > > about it > > > > > > > > > > > more. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 4. How about execution.recovery.claim-mode > > > instead > > > > of > > > > > > > > > > > > > > > > execution.recovery.mode? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The meaning of mode is too broad. The > > claim-mode > > > > may > > > > > > > > > > > > > > > > be more accurate for users. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > WDYT? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > > > > > > > > Rui > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 25, 2023 at 5:14 PM Zakelly Lan < > > > > > > > > > > > zakelly....@gmail.com > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi devs, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I'd like to start a discussion on FLIP-406: > > > > > > Reorganize > > > > > > > State > > > > > > > > > > & > > > > > > > > > > > > > > > > > Checkpointing & Recovery Configuration[1]. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Currently, the configuration options > > pertaining > > > > to > > > > > > > > > > > checkpointing, > > > > > > > > > > > > > > > > recovery, > > > > > > > > > > > > > > > > > and state management are primarily grouped > > > under > > > > > the > > > > > > > > > > following > > > > > > > > > > > > > > > prefixes: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > - state.backend.* : configurations > related > > > to > > > > > > state > > > > > > > > > > > accessing > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > checkpointing, as well as specific > options > > > for > > > > > > > individual > > > > > > > > > > > > state > > > > > > > > > > > > > > > > backends > > > > > > > > > > > > > > > > > - execution.checkpointing.* : > > configurations > > > > > > > associated > > > > > > > > > > with > > > > > > > > > > > > > > > > checkpoint > > > > > > > > > > > > > > > > > execution and recovery > > > > > > > > > > > > > > > > > - execution.savepoint.*: configurations > > for > > > > > > > recovery from > > > > > > > > > > > > > savepoint > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > In addition, there are several individual > > > options > > > > > > such > > > > > > > as ' > > > > > > > > > > > > > > > > > *state.checkpoint-storage*' and > > > > > > > '*state.checkpoints.dir*' > > > > > > > > > > that > > > > > > > > > > > > fall > > > > > > > > > > > > > > > > outside > > > > > > > > > > > > > > > > > of these prefixes. The current arrangement > of > > > > these > > > > > > > options, > > > > > > > > > > > > which > > > > > > > > > > > > > span > > > > > > > > > > > > > > > > > multiple modules, is somewhat haphazard and > > > > lacks a > > > > > > > > > > systematic > > > > > > > > > > > > > > > structure. > > > > > > > > > > > > > > > > > For example, the options under the > > > > > > > '*CheckpointingOptions*' > > > > > > > > > > > and ' > > > > > > > > > > > > > > > > > *ExecutionCheckpointingOptions*' are > related > > > and > > > > > have > > > > > > > no > > > > > > > > > > clear > > > > > > > > > > > > > > > boundaries > > > > > > > > > > > > > > > > > from the user's perspective, but there is > no > > > > > unified > > > > > > > prefix > > > > > > > > > > for > > > > > > > > > > > > > them. > > > > > > > > > > > > > > > > With > > > > > > > > > > > > > > > > > the upcoming release of Flink 2.0, we have > an > > > > > > excellent > > > > > > > > > > > > > opportunity to > > > > > > > > > > > > > > > > > overhaul and restructure the configurations > > > > related > > > > > > to > > > > > > > > > > > > > checkpointing, > > > > > > > > > > > > > > > > > recovery, and state management. This FLIP > > > > proposes > > > > > to > > > > > > > > > > > reorganize > > > > > > > > > > > > > these > > > > > > > > > > > > > > > > > settings, making it more coherent by > module, > > > > which > > > > > > > would > > > > > > > > > > > > > significantly > > > > > > > > > > > > > > > > > lower the barriers for understanding and > > reduce > > > > the > > > > > > > > > > development > > > > > > > > > > > > > costs > > > > > > > > > > > > > > > > > moving forward. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Looking forward to hearing from you! > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=284789560 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > > > > > > > > > Zakelly > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- Best, Hangxiang.
