Hi Yuan,
I would like to share two cents here.
No matter what results we can achieve, we would all agree that the
configuration at the specific state-backend level should be prioritized than
the overall `state.backend.incremental` config.
With the above rules, let's take a close look at the implementation of how to
support the RocksDB state-backend with incremental checkpoints by default while
the HashMap state-backend does not.
First of all, we need to make `state.backend.incremental` with no explicit
default value, and thus could be overwritten by specific state-backends in the
default case with new configurations, e.g. `state.backend.rocksdb.incremental`
and `state.backend.hashmap.incremental`.
We can create a relationship matrix below, the value true or false in the
column means whether the incremental checkpoint is enabled:
incremenatl(not set) |
incremenatl(true) | incremenatl(false)
rocksdb.incremenatl (not set) YES YES
NO
rocksdb.incremenatl (true) YES YES
YES
rocksdb.incremenatl (false) NO NO
NO
As we can see, we also need to make `state.backend.rocksdb.incremental` without
an explicit default value to make the logic could work, which is a bit complex
for users to understand.
Thus, in my mind, we should avoid introducing such new configurations unless we
have to.
Best
Yun Tang
________________________________
From: Yuan Mei <yuanmei.w...@gmail.com>
Sent: Monday, July 11, 2022 11:53
To: dev <dev@flink.apache.org>
Cc: ro...@apache.org <ro...@apache.org>
Subject: Re: Re: Re: [DISCUSS ] Make state.backend.incremental as true by
default
Hey Lihe Ma,
1. I do not think making different state backends with different default
setups (configs) is a bad idea. I have some similar ideas as Zakelly,
Hangxiang and you mentioned above.
Think about this: even though the incremental checkpoint for HashMap
Statebackend is released in 1.16, it may still take a while to stabilize,
and is probably not needed as a default for HashMap.
2. Incremental checkpoint for HashMap Statebackend is a feature for HashMap
Statebackend. Introducing a new state backend for this purpose does not
sound right here, as reasons have been discussed in my previous reply.
3. "Many guys show big +1" indicates that incremental checkpoint as default
is needed for RocksDB, and I am with big +1 on this as well. However, this
does not mean we can ignore the problems I've stated or ignore the needs
for other state backends. The right way is to find acceptable solutions and
to make "incremental checkpoint as default for RocksDB" happen.
Best,
Yuan
On Mon, Jun 27, 2022 at 12:28 PM Hangxiang Yu <master...@gmail.com> wrote:
> The suggestion of Zakelly (Using noDefaultValue to make different
> StateBackend have its own default value) makes sense to me.
> I haven't gotten a better idea about it.
> Maybe @ro...@apache.org <ro...@apache.org> who is the owner of incremental
> checkpoint support of HashMapStateBackend could share more ideas about it.
>
> On Sat, Jun 25, 2022 at 2:38 PM Lihe Ma <ma_l...@163.com> wrote:
>
> > Hi, Yuan Mei,
> >
> >
> > Thanks for your feedback.
> >
> > The configuration state.backend.incremental should work for all state
> > backends, which I think is reasonable. If we introduce more
> > configurations(such as state.backend.hashmap.incremental,
> > state.backend.rocksdb.incremental), it does not bring enough benefit ,
> and
> > users will be confused by the relationship of these configurations as
> > state.backend.incremental has been widely used for a long time.
> >
> >
> > If you prefer to reuse existing HashMapStateBackend to support the newly
> > incremental checkpoint feature, to be honest, I did not come up with an
> > idea to make everyone satisfied without introducing more configurations.
> It
> > seems that you have some ideas in your last reply, would you like to
> share
> > something here? Otherwise, I think we have to postpone the change to next
> > releases and let the beta feature of incremental checkpoint hashmap
> > state-backend merged first in the flink-1.16 release, even though many
> guys
> > show big +1 for the change of state.backend.incremental in this
> discussion.
> >
> >
> > Best,
> > Lihe Ma
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > At 2022-06-19 20:39:45, "Yuan Mei" <yuanmei.w...@gmail.com> wrote:
> > >Hey @Lihe Ma,
> > >
> > >Thanks and I appreciate your feedback!
> > >
> > >*What do I think of introducing a new state backend?*
> > >I am hesitant to follow this way of introducing a new state backend to
> > >implement `HashMapStateBackend with incremental checkpoint enabled`,
> > >because:
> > >- It causes more confusion for users: why incremental checkpoint can
> > >be *enabled
> > >through a flag for RocksDB* but having to *write a new Statebackend for
> > >HashMap*?
> > >- For the same reason above, this causes *inconsistent system behavior*
> > and
> > >potentially lead to more problems.
> > > For example, do we want to eventually keep both state backends
> > (HashMap
> > >and the newly introduced one) or deprecate one?
> > > How to deprecate one if both are widely used in prod, especially
> since
> > >people can set up state backends in the job code, which means they have
> to
> > >change/update their job code for depreciation?
> > >- Most importantly, I do not think introducing a new state backend to
> > >implement `HashMapStateBackend with incremental checkpoint enabled`
> > >resolves the problem. I will explain that later.
> > >
> > >Now let's come back to the original proposal
> > >*Make incremental checkpoint as default for RocksDB?*
> > >As I've already stated previously, I am +1 on this. And the reason is
> well
> > >explained (the motivation, backward compatibilities, and previous
> > glitches
> > >solved by FLIP 193 e.t.c) in the original proposal and other
> > >threads/tickets.
> > >
> > >However, there is a gap between the above goal and the way proposed to
> > >achieve it: "set as the default state.backend.incremental = true". The
> way
> > >proposed is to make incremental checkpoint default universally for all
> > >state backends.
> > >
> > >*Should we make incremental checkpoints as default universally* *for all
> > >state backends?*
> > >I am not sure about this, or at least before a few questions are
> > >answered/discussed/solved.
> > >
> > >1. Is incremental checkpoints as default needed for all state backends?
> > > Probably not, but if one state backend (like RocksDB) has strong
> > >benefits, but others are neutral or non-neg effects, it is acceptable.
> > >2. What is the impact of the default flag switch on the currently
> > supported
> > >state backends?
> > > RocksDB prefers incremental checkpoints, and HashMap (In Memory)
> does
> > >not support incremental yet (no effect).
> > >3. How about system extensibility and future migration?
> > > There is a problem here. Nowadays, HashMap (In Memory) state
> > >backends are widely used in prod as well when the state size is small
> > >enough (in most cases based on user feedback). The support for
> incremental
> > >checkpoints on HashMapStatebackend is asked by users. Then think about
> > >this: if we set default state.backend.incremental = true, and launch the
> > >incremental checkpoint feature for HashMapStatebackend, there is a big
> > >upgrading cost for existing users that uses HashMapStatebackend.
> > > Introducing a new state backend does not solve the problem of
> "system
> > >extensibility". It introduces new problems like "deprecating and
> > migration"
> > >as stated above.
> > >
> > >*What is my suggestion?*
> > >- Design a way to support different default configurations for different
> > >state backends (I do not think it is difficult to do).
> > >- Have a better way to solve the "system extensibility" problem.
> > >
> > >Best,
> > >Yuan
> > >
> > >
> > >On Fri, Jun 17, 2022 at 11:49 AM LuNing Wang <wang4lun...@gmail.com>
> > wrote:
> > >
> > >> Strongly +1
> > >>
> > >> Best,
> > >> LuNing Wang
> > >>
> > >> Zakelly Lan <zakelly....@gmail.com> 于2022年6月17日周五 00:15写道：
> > >>
> > >> > Thanks for bringing this up.
> > >> > I'm +1 on enabling the incremental checkpoint by default on
> RocksDB.
> > >> But I
> > >> > also agree with Yuan about not enabling this on newly implemented
> > >> > incremental checkpoint for hashmap statebackend.
> > >> > I am wondering can we make it behave differently for different state
> > >> > backends when ```state.backend.incremental``` is not set? Although
> it
> > may
> > >> > bring some confusion in the default behaviour.
> > >> >
> > >> > On Thu, Jun 16, 2022 at 10:18 PM Lihe Ma <ma_l...@163.com> wrote:
> > >> >
> > >> > > Thanks for the suggestion, Yuan.
> > >> > >
> > >> > >
> > >> > > How about naming the newly in-memory state-backend, which supports
> > >> > > incremental checkpoint, as HeapStateBackend . And let the default
> > >> > > state-backend still stay as HashMapStateBackend.
> > >> > > By doing so, we can:
> > >> > > 1) the default value of parameter state.backend.incremental could
> be
> > >> set
> > >> > > as true safely, which is so widely accepted according to the
> > replies in
> > >> > > this thread.
> > >> > > 2) your newly in-memory state-backend could also be merged and
> users
> > >> who
> > >> > > want to try in-memory incremental checkpoints could also have a
> > >> solution.
> > >> > > We can also set the newly HeapStateBackend as default
> state-backend
> > in
> > >> > the
> > >> > > future if the feature of incremental checkpoint is stable. What do
> > you
> > >> > > think?
> > >> > >
> > >> > >
> > >> > > Best,
> > >> > > Lihe Ma
> > >> > >
> > >> > >
> > >> > >
> > >> > >
> > >> > >
> > >> > >
> > >> > >
> > >> > >
> > >> > >
> > >> > >
> > >> > >
> > >> > >
> > >> > >
> > >> > >
> > >> > >
> > >> > > 在 2022-06-15 21:03:31，"Feifan Wang" <zoltar9...@163.com> 写道：
> > >> > > >Thanks for bringing this up.
> > >> > > >Strongly +1
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > > >――――――――――――――
> > >> > > >Name: Feifan Wang
> > >> > > >Email: zoltar9...@163.com
> > >> > > >
> > >> > > >
> > >> > > >---- Replied Message ----
> > >> > > >| From | Yuan Mei<yuanmei.w...@gmail.com> |
> > >> > > >| Date | 06/15/2022 11:41 |
> > >> > > >| To | dev<dev@flink.apache.org> ,
> > >> > > ><ro...@ververica.com> |
> > >> > > >| Subject | Re: [DISCUSS ] Make state.backend.incremental as true
> > by
> > >> > > default |
> > >> > > >Thanks for bringing this up.
> > >> > > >
> > >> > > >I am +1 on making incremental checkpoints by default for RocksDB,
> > but
> > >> > not
> > >> > > >universally for all state backends.
> > >> > > >
> > >> > > >Besides being widely used in prod, enabling incremental
> checkpoint
> > for
> > >> > > >RocksDB by default is also a pre-requisite when enabling
> > task-local by
> > >> > > >default FLINK-15507 <
> > >> https://issues.apache.org/jira/browse/FLINK-15507>
> > >> > > >
> > >> > > >The incremental checkpoint for the hashmap statebackend is under
> > >> review
> > >> > > >right now. CC @ro...@ververica.com <ro...@ververica.com> , which
> > is
> > >> > not a
> > >> > > >good idea being enabled by default in the first version.
> > >> > > >
> > >> > > >Best,
> > >> > > >
> > >> > > >Yuan
> > >> > > >
> > >> > > >On Tue, Jun 14, 2022 at 7:33 PM Jiangang Liu <
> > >> liujiangangp...@gmail.com
> > >> > >
> > >> > > >wrote:
> > >> > > >
> > >> > > >+1 for the suggestion. We have use the incremental checkpoint in
> > our
> > >> > > >production for a long time.
> > >> > > >
> > >> > > >Hangxiang Yu <master...@gmail.com> 于2022年6月14日周二 15:41写道：
> > >> > > >
> > >> > > >+1
> > >> > > >It's basically enabled in most scenarios in production
> > environments.
> > >> > > >For HashMapStateBackend, it will adopt a full checkpoint even if
> we
> > >> > > >enable
> > >> > > >incremental checkpoint. It will also support incremental
> checkpoint
> > >> > after
> > >> > > >[1]. It's compatible.
> > >> > > >BTW, I think we may also need to improve the documentation of
> > >> > incremental
> > >> > > >checkpoints which users usually ask. There are some tickets like
> > >> [2][3].
> > >> > > >
> > >> > > >Best,
> > >> > > >Hangxiang.
> > >> > > >
> > >> > > >[1] https://issues.apache.org/jira/browse/FLINK-21648
> > >> > > >[2] https://issues.apache.org/jira/browse/FLINK-22797
> > >> > > >[3] https://issues.apache.org/jira/browse/FLINK-7449
> > >> > > >
> > >> > > >On Mon, Jun 13, 2022 at 7:48 PM Rui Fan <1996fan...@gmail.com>
> > wrote:
> > >> > > >
> > >> > > >Strongly +1
> > >> > > >
> > >> > > >Best,
> > >> > > >Rui Fan
> > >> > > >
> > >> > > >On Mon, Jun 13, 2022 at 7:35 PM Martijn Visser <
> > >> > > >martijnvis...@apache.org
> > >> > > >
> > >> > > >wrote:
> > >> > > >
> > >> > > >BTW, from my knowledge, nothing would happen for
> > >> > > >HashMapStateBackend,
> > >> > > >which does not support incremental checkpoint yet, when enabling
> > >> > > >incremental checkpoints.
> > >> > > >
> > >> > > >Thanks Yun, if no errors would occur then definitely +1 to enable
> > it
> > >> > > >by
> > >> > > >default
> > >> > > >
> > >> > > >Op ma 13 jun. 2022 om 12:42 schreef Alexander Fedulov <
> > >> > > >alexan...@ververica.com>:
> > >> > > >
> > >> > > >+1
> > >> > > >
> > >> > > >From my experience, it is actually hard to come up with use cases
> > >> > > >where
> > >> > > >incremental checkpoints should explicitly not be enabled with the
> > >> > > >RocksDB
> > >> > > >state backend. If the state is so small that the full snapshots
> do
> > >> > > >not
> > >> > > >have any negative impact, one should consider using
> > >> > > >HashMapStateBackend
> > >> > > >anyway.
> > >> > > >
> > >> > > >Best,
> > >> > > >Alexander Fedulov
> > >> > > >
> > >> > > >
> > >> > > >On Mon, Jun 13, 2022 at 12:26 PM Jing Ge <j...@ververica.com>
> > >> > > >wrote:
> > >> > > >
> > >> > > >+1
> > >> > > >
> > >> > > >Glad to see the kickoff of this discussion. Thanks Lihe for
> > >> > > >driving
> > >> > > >this!
> > >> > > >
> > >> > > >We have actually already discussed it internally a few months
> > >> > > >ago.
> > >> > > >After
> > >> > > >considering some corner cases, all agreed on enabling the
> > >> > > >incremental
> > >> > > >checkpoint as default.
> > >> > > >
> > >> > > >Best regards,
> > >> > > >Jing
> > >> > > >
> > >> > > >On Mon, Jun 13, 2022 at 12:17 PM Yun Tang <myas...@live.com>
> > >> > > >wrote:
> > >> > > >
> > >> > > >Strongly +1 for making incremental checkpoints as default. Many
> > >> > > >users
> > >> > > >have
> > >> > > >ever been asking why this configuration is not enabled by
> > >> > > >default.
> > >> > > >
> > >> > > >BTW, from my knowledge, nothing would happen for
> > >> > > >HashMapStateBackend,
> > >> > > >which does not support incremental checkpoint yet, when
> > >> > > >enabling
> > >> > > >incremental checkpoints.
> > >> > > >
> > >> > > >
> > >> > > >Best
> > >> > > >Yun Tang
> > >> > > >________________________________
> > >> > > >From: Martijn Visser <martijnvis...@apache.org>
> > >> > > >Sent: Monday, June 13, 2022 18:05
> > >> > > >To: dev@flink.apache.org <dev@flink.apache.org>
> > >> > > >Subject: Re: [DISCUSS ] Make state.backend.incremental as true
> > >> > > >by
> > >> > > >default
> > >> > > >
> > >> > > >Hi Lihe,
> > >> > > >
> > >> > > >What happens if we enable incremental checkpoints by default
> > >> > > >while
> > >> > > >the
> > >> > > >used
> > >> > > >memory backend is HashMapStateBackend, which doesn't support
> > >> > > >incremental
> > >> > > >checkpoints?
> > >> > > >
> > >> > > >Best regards,
> > >> > > >
> > >> > > >Martijn
> > >> > > >
> > >> > > >Op ma 13 jun. 2022 om 11:59 schreef Lihe Ma <ma_l...@163.com>:
> > >> > > >
> > >> > > >Hi, Everyone,
> > >> > > >
> > >> > > >I would like to open a discussion on setting incremental
> > >> > > >checkpoint
> > >> > > >as
> > >> > > >default behavior.
> > >> > > >
> > >> > > >Currently, the configuration of state.backend.incremental is
> > >> > > >set
> > >> > > >as
> > >> > > >false
> > >> > > >by default. Incremental checkpoint has been adopted widely in
> > >> > > >industry
> > >> > > >community for many years , and it is also well-tested from
> > >> > > >the
> > >> > > >feedback
> > >> > > >in
> > >> > > >the community discussion. Incremental checkpointing is more
> > >> > > >light-weighted:
> > >> > > >shorter checkpoint duration, less uploaded data and less
> > >> > > >resource
> > >> > > >consumption.
> > >> > > >
> > >> > > >In terms of backward compatibility, enable incremental
> > >> > > >checkpointing
> > >> > > >would
> > >> > > >not make any data loss no matter restoring from a full
> > >> > > >checkpoint/savepoint
> > >> > > >or an incremental checkpoint.
> > >> > > >
> > >> > > >FLIP-193 (Snapshot ownership)[1] has been released in 1.15,
> > >> > > >incremental
> > >> > > >checkpoint no longer depends on a previous restored
> > >> > > >checkpoint
> > >> > > >in
> > >> > > >default
> > >> > > >NO_CLAIM mode, which makes the checkpoint lineage much
> > >> > > >cleaner,
> > >> > > >it
> > >> > > >is a
> > >> > > >good chance to change the configuration
> > >> > > >state.backend.incremental
> > >> > > >to
> > >> > > >true
> > >> > > >as default.
> > >> > > >
> > >> > > >Thus, based on the above discussion, I suggest to make
> > >> > > >state.backend.incremental as true by default. What do you
> > >> > > >think
> > >> > > >of
> > >> > > >this
> > >> > > >proposal?
> > >> > > >
> > >> > > >[1]
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-193%3A+Snapshots+ownership
> > >> > > >
> > >> > > >Best regards,
> > >> > > >Lihe Ma
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> >
>
