You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by Zakelly Lan <za...@gmail.com> on 2023/10/07 08:07:43 UTC
Re: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Hi Jing,
Sorry for the late reply! I agree with you that we do not expect users
to do anything with Flink and we won't "bother" them with those
exceptions. However, users can still catch the `Throwable` and perform
any necessary logging activities, similar to how they use Java
Collection interfaces.
Thanks for your insights!
Best,
Zakelly
On Thu, Sep 21, 2023 at 8:43 PM Jing Ge <ji...@ververica.com.invalid> wrote:
>
> Fair enough! Thanks Zakelly for the information. Afaic, even users can do
> nothing with Flink, they still can do something in their territory, at
> least doing some logging and metrics stuff, or triggering some other
> services in their ecosystem. After all, the Flink jobs they build are part
> of their service component. It didn't change the fact that we are going to
> use the anti-pattern. Just because we didn't expect users to do
> anything with Flink, does not mean users don't expect to do something with
> the expected exception. Anyway, I am open to hearing different opinions.
>
> Best regards,
> Jing
>
> On Thu, Sep 21, 2023 at 7:02 AM Zakelly Lan <za...@gmail.com> wrote:
>
> > Hi Martijn,
> >
> > Thanks for the reminder!
> >
> > This FLIP proposes a change to the state API that is annotated as
> > @PublicEvolving and targets version 1.19. I have clarified this in
> > the "Proposed Change" section of the FLIP.
> >
> >
> > Hi Jing,
> >
> > Thanks for sharing your thoughts! Here are my opinions:
> >
> > 1. The exceptions of the state API are usually treated as critical
> > ones. In other words, if anything goes wrong with state accessing, the
> > element processing cannot proceed and the job should fail. Flink users
> > may not know what to do when they encounter these exceptions. I
> > believe this is the main reason why we want to replace them with
> > unchecked exceptions.
> > 2. There have also been some further discussions[1][2] from Stephan
> > and Shixiaogang below the one you pointed out [3], and it seems they
> > come to an agreement to use unchecked exceptions. After reviewing the
> > entire discussion on that PR, I think their arguments are reasonable
> > given the use case.
> >
> > Looking forward to your feedback.
> >
> >
> > Best,
> > Zakelly
> >
> > [1] https://github.com/apache/flink/pull/3380#issuecomment-286807853
> > [2] https://github.com/apache/flink/pull/3380#issuecomment-286932133
> > [3] https://github.com/apache/flink/pull/3380#issuecomment-281631160
> >
> > On Thu, Sep 21, 2023 at 1:27 AM Jing Ge <ji...@ververica.com.invalid>
> > wrote:
> > >
> > > sorry, typo: It is a known "anti-pattern" instead of "ant-pattern"
> > >
> > > Best regards,
> > > Jing
> > >
> > > On Wed, Sep 20, 2023 at 7:23 PM Jing Ge <ji...@ververica.com> wrote:
> > >
> > > > Hi Zakelly,
> > > >
> > > > Thanks for driving this topic. From good software engineering's
> > > > perspective, I have different thoughts:
> > > >
> > > > 1. The idea to get rid of all checked Exceptions and replace them with
> > > > unchecked Exceptions is a known ant-pattern: "Generally speaking, do
> > not
> > > > throw a RuntimeException or create a subclass of RuntimeException
> > simply
> > > > because you don't want to be bothered with specifying the exceptions
> > your
> > > > methods can throw." [1] Checked Exceptions mean expected exceptions
> > that
> > > > can help developers find a way to catch them and decide what to do. It
> > is
> > > > part of the public API signature that can help developers build robust
> > > > systems. We should not mix concepts and build expected exceptions with
> > > > unchecked Java Exception classes.
> > > > 2. The comment Stephan left [2] clearly pointed out that we should
> > avoid
> > > > using generic Java Exceptions, and "find some more 'specific'
> > exceptions
> > > > for the signature, like throws IOException or throws
> > StateAccessException."
> > > > So, the idea is to define/use specific checked Exception classes
> > instead of
> > > > using unchecked Exceptions.
> > > >
> > > > Looking forward to your thoughts.
> > > >
> > > > Best regards,
> > > > Jing
> > > >
> > > >
> > > > [1]
> > > >
> > https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
> > > > [2] https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > >
> > > > On Wed, Sep 20, 2023 at 4:52 PM Zakelly Lan <za...@gmail.com>
> > wrote:
> > > >
> > > >> Hi Yanfei,
> > > >>
> > > >> Thanks for your reply!
> > > >>
> > > >> Yes, this FLIP aims to change all state-related exceptions to
> > > >> unchecked exceptions and remove all exceptions from the signature. So
> > > >> I believe we have come to an agreement to keep the interfaces simple.
> > > >>
> > > >>
> > > >> Best regards,
> > > >> Zakelly
> > > >>
> > > >> On Wed, Sep 20, 2023 at 2:26 PM Zakelly Lan <za...@gmail.com>
> > > >> wrote:
> > > >> >
> > > >> > Hi Hangxiang,
> > > >> >
> > > >> > Thank you for your response! Here are my thoughts:
> > > >> >
> > > >> > 1. Regarding the exceptions thrown by internal interfaces, I suggest
> > > >> > keeping them as checked exceptions. Since these exceptions will be
> > > >> > handled by the internal callers, it is meaningful to throw them as
> > > >> > checked ones. If we need to make changes to these classes, we can
> > > >> > create separate tickets alongside this FLIP. What are your thoughts
> > on
> > > >> > this?
> > > >> > 2. StateIOException is primarily thrown by file-based state like
> > > >> > RocksDB, while StateAccessException is more generic and can be
> > thrown
> > > >> > by heap states. Additionally, I believe there may be more subclasses
> > > >> > of StateAccessException that we can add. We can consider this when
> > > >> > implementing.
> > > >> > 3. I would like to make this change in version 1.19. As mentioned in
> > > >> > this FLIP, many users do not catch any exceptions since the element
> > > >> > processing function exposes the exception to the upper layer.
> > > >> > Therefore, the impact is manageable. And I completely agree that we
> > > >> > should clearly document this change in the next release notes.
> > > >> >
> > > >> >
> > > >> > Best regards,
> > > >> > Zakelly
> > > >> >
> > > >> > On Wed, Sep 20, 2023 at 12:35 PM Yanfei Lei <fr...@gmail.com>
> > > >> wrote:
> > > >> > >
> > > >> > > Hi Zakelly,
> > > >> > >
> > > >> > > Thanks for bringing this up. +1 for reorganizing.
> > > >> > >
> > > >> > > IIUC, this proposal aims to change all state-related exceptions to
> > > >> > > unchecked exceptions. If users have caught checked exceptions
> > (such as
> > > >> > > IOException ) in their code, leaving the code as is would also
> > work.
> > > >> > >
> > > >> > > Is it possible not to put any exceptions in the signature of
> > > >> > > user-facing interfaces? As the proposal mentioned, users can do a
> > few
> > > >> > > things even if they catch the exceptions. Keeping the interface
> > simple
> > > >> > > may be easier to understand.
> > > >> > >
> > > >> > >
> > > >> > > Best,
> > > >> > > Yanfei
> > > >> > >
> > > >> > > Hangxiang Yu <ma...@gmail.com> 于2023年9月19日周二 18:16写道：
> > > >> > > >
> > > >> > > > Hi, Zakelly.
> > > >> > > >
> > > >> > > > Thanks for the proposal.
> > > >> > > >
> > > >> > > > +1 for reorganizing exceptions of state interfaces which indeed
> > > >> confuses me
> > > >> > > > currently.
> > > >> > > >
> > > >> > > > From my experience, users usually omit these exceptions because
> > > >> they cannot
> > > >> > > > do much even if they catch the exceptions.
> > > >> > > >
> > > >> > > > I have some problems and suggestions, PTAL:
> > > >> > > >
> > > >> > > > 1. Could we also reorganize or add more state exceptions
> > (may be
> > > >> related
> > > >> > > > to other state interfaces/classes e.g. KeyedStateBackend)
> > into
> > > >> the
> > > >> > > > exception class diagrams ? Although these state-related
> > classes
> > > >> may not
> > > >> > > > be public, it could be better to consider them together to
> > make
> > > >> all
> > > >> > > > state-related exceptions clear. For example, we could
> > reorganize
> > > >> some
> > > >> > > > existing exceptions such as StateMigrationException, add some
> > > >> exceptions
> > > >> > > > such as StateNotFoundException.
> > > >> > > > 2. Could you clarify or give an example about the extended
> > > >> relation
> > > >> > > > "StateAccessException -- StateIOException" ? When do we just
> > > >> return
> > > >> > > > StateAccessException instead of StateIOException or others ?
> > > >> > > > 3. Which version do you want to implement it in ? Since it
> > has
> > > >> to break
> > > >> > > > changes for users who have catched the IOException, If we
> > want
> > > >> to implement
> > > >> > > > it in 1.19, we must mark it very clearly in the release
> > note, or
> > > >> we should
> > > >> > > > make it in 2.0.
> > > >> > > >
> > > >> > > >
> > > >> > > > On Tue, Sep 19, 2023 at 5:08 PM Zakelly Lan <
> > zakelly.lan@gmail.com>
> > > >> wrote:
> > > >> > > >
> > > >> > > > > Hi everyone,
> > > >> > > > >
> > > >> > > > > I would like to initiate a discussion on FLIP-368, which
> > focuses
> > > >> on
> > > >> > > > > reorganizing the exceptions thrown in state interfaces [1].
> > > >> > > > >
> > > >> > > > > Currently, we have identified several problems with the
> > exceptions
> > > >> > > > > thrown by state-related interfaces:
> > > >> > > > > 1. The exception types thrown by each interface are
> > > >> inconsistent.
> > > >> > > > > While most of the interfaces claim to throw `Exception`, the
> > > >> > > > > interfaces of `ValueState` throw `IOException`. Additionally,
> > the
> > > >> > > > > `State#clear()` never throws an exception. This can be
> > confusing
> > > >> for
> > > >> > > > > users.
> > > >> > > > > 2. The use of `Exception` or `IOException` as the thrown
> > > >> exception
> > > >> > > > > type is too generic and lacks specificity.
> > > >> > > > > 3. Users may not be able to handle these exceptions. In
> > cases
> > > >> where
> > > >> > > > > an exception occurs while accessing state, the job should
> > fail.
> > > >> This
> > > >> > > > > aligns more with the characteristic of *unchecked exceptions*
> > > >> instead
> > > >> > > > > of checked exceptions.
> > > >> > > > >
> > > >> > > > > To address these issues, we borrow the idea of throwing
> > unchecked
> > > >> > > > > exceptions in Java Collection API and propose the following
> > > >> changes in
> > > >> > > > > state-related exceptions:
> > > >> > > > > 1. Introduction of specific unchecked exception types for
> > > >> different
> > > >> > > > > reasons, providing users with more precise information about
> > the
> > > >> cause
> > > >> > > > > of the exception.
> > > >> > > > > 2. Removal of all checked exceptions from interface
> > signatures
> > > >> and
> > > >> > > > > instead, throwing newly introduced unchecked exceptions in the
> > > >> > > > > implementations.
> > > >> > > > >
> > > >> > > > > Please share your thoughts and suggestions regarding the
> > proposed
> > > >> > > > > changes. Thank you for your attention and support.
> > > >> > > > >
> > > >> > > > >
> > > >> > > > > Best,
> > > >> > > > > Zakelly
> > > >> > > > >
> > > >> > > > > [1] FLIP-368: Reorganize the exceptions thrown in state
> > > >> interfaces,
> > > >> > > > > https://cwiki.apache.org/confluence/x/eZ2zDw
> > > >> > > > >
> > > >> > > >
> > > >> > > >
> > > >> > > > --
> > > >> > > > Best,
> > > >> > > > Hangxiang.
> > > >>
> > > >
> >
Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Posted by Zakelly Lan <za...@gmail.com>.
Hi all,
After rethinking, I‘ve come to agree that the Flink 2.0 would be a fittable
version for this kind of change. I'd like to reopen this discussion for
making this in 2.0. I suggest modifying the exception directly in Flink 2.0
rather than introducing a new set of APIs without any additional
adjustments. Having two sets of APIs with only exceptions changed could
lead to confusion and unnecessary maintenance burden. WDYT? @Jing Ge @David
Radley @Yuan Mei
Best,
Zakelly
On Mon, Oct 16, 2023 at 7:20 PM Jing Ge <ji...@ververica.com.invalid> wrote:
> Hi Zakelly,
>
> Thanks for the clarification. But I have to agree with Yuan. It is a
> breaking change as you mentioned that users who catch those exceptions will
> be forced to change their code. Changing it during the minor releases
> violates the backward compatibility promise of Flink public(Evolving) API.
> I would suggest explicitly writing down the breaking change in the FLIP and
> let the community decide. For me, it might not be an optimized solution to
> introduce breaking changes between minor releases, but I personally won't
> block your effort.
>
> Best regards,
> Jing
>
> On Fri, Oct 13, 2023 at 9:37 PM Zakelly Lan <za...@gmail.com> wrote:
>
> > Hi Jing,
> >
> > For jobs that catch exceptions, they will need to modify their code.
> > If they still wish to catch the exception, they should catch the
> > `Throwable` instead.
> >
> > As I explained, this use case is not common because:
> > 1. The user-defined processElement function, where the state APIs are
> > called, already declares that it throws `Exception`, so there is no
> > need for user code to catch these exceptions. The compiler or IDE will
> > not prompt them to do so.
> > 2. These exceptions are fatal, and users can do very little or nothing
> > to prevent a job failure. Also in this case, Flink already provides
> > error logging for users.
> >
> > And for minor releases, APIs annotated with @PublicEvolving are
> > allowed to have signature changes[1]. So I think the callers will
> > expect this when migrating to a new version.
> >
> >
> > Hi David,
> >
> > It is true that recompilation is necessary when migrating to the next
> > minor release, as Flink is a rapidly evolving project. API
> > compatibility is essential, but it should not hinder development.
> > That's why the API compatibility guarantees of Flink[1] were created.
> > If we expect stable binaries/sources across dot versions, then we
> > should prohibit any breaking changes, not just this one. Therefore, we
> > can discuss a new rule for this. However, for now, this proposal
> > aligns with the current rules.
> >
> > Regarding Flink 2.0, if my estimate is correct, it will be released at
> > least one year later. The API will undergo a significant redesign, and
> > user code may need to be essentially rewritten. The main focus of
> > Flink 2.0 will be on API design, not on reorganizing exceptions as
> > proposed here. After Flink 2.0 is released, many existing users may
> > continue using Flink 1.x for a long time. This work aims to provide a
> > cleaner API for Flink 1.x before we stop updating it.
> >
> >
> > Best,
> > Zakelly
> >
> >
> > [1]
> >
> https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees
> >
> >
> > On Fri, Oct 13, 2023 at 11:16 PM David Radley <da...@uk.ibm.com>
> > wrote:
> > >
> > > Hi,
> > > It seems that migrating to the next dot version and finding you need to
> > recompile, would be frustrating and couse unexpected work, as I suspect a
> > jar file using the old API will give an exception around not finding the
> > method – I am not sure how this would surface in typical applications at
> > runtime.
> > >
> > > Technically because this is tagged as @PublicEvolving then we can make
> > this breaking change.
> > >
> > > So there will be migration issues if people are using this API, have we
> > an idea on how many of our users are using it?
> > >
> > > If we use @PublicEvolving then maybe we should have stable binaries
> that
> > only include public APIs and then another bleeding edge package
> containing
> > @PublicEvolving content, so users can choose.
> > >
> > > Organisations I have worked with would not tend to want to or expect to
> > have to recompile their applications on a dot version – as this would
> > normally mean a lot more testing for them.
> > >
> > > On balance, as I am risk averse, I would suggest delaying this to v2 as
> > Jing has proposed. This is a cleaner API, is there a demand for this in a
> > dot version? If the community think this is too risk averse, then we
> could
> > go with 1.19.
> > > WDYT?
> > >
> > > Kind regards, David.
> > >
> > >
> > >
> > > From: Jing Ge <ji...@ververica.com.INVALID>
> > > Date: Friday, 13 October 2023 at 14:30
> > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > Subject: [EXTERNAL] Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the
> > exceptions thrown in state interfaces
> > > HI Zakelly,
> > >
> > > What about the jobs that catch those exceptions? Will these downstream
> > > callers that expect this exception to be thrown break, since the
> > exceptions
> > > are part of the method's contract?
> > >
> > > Best regards,
> > > Jing
> > >
> > > On Fri, Oct 13, 2023 at 11:01 AM Zakelly Lan <za...@gmail.com>
> > wrote:
> > >
> > > > Hi Yuan,
> > > >
> > > > Thanks for sharing your thoughts!
> > > >
> > > > In Flink 2.0 I believe we could design brand-new state APIs, which is
> > > > uncertain and may take some time. However, this proposal primarily
> > > > focuses on improving the consistency of interface definitions and
> > > > enhancing the user experience in error handling for the current APIs.
> > > > Therefore, I would prefer to make it in version 1.19. Furthermore,
> the
> > > > impact of this API change can be controlled since most Flink users do
> > > > not actively catch these exceptions. For them, a simple code
> recompile
> > > > is sufficient and acceptable when migrating to a new minor release.
> > > > The change is not that big that we need a major version to apply.
> > > >
> > > >
> > > > Best,
> > > > Zakelly
> > > >
> > > > On Fri, Oct 13, 2023 at 3:03 PM Yuan Mei <yu...@gmail.com>
> > wrote:
> > > > >
> > > > > +1 for the proposal
> > > > >
> > > > > But "Since the signature of the public state API has been changed",
> > I was
> > > > > wondering whether this would be more fittable in Flink 2.0, instead
> > of
> > > > 1.19?
> > > > >
> > > > > WDYT?
> > > > >
> > > > > Best
> > > > > Yuan
> > > > >
> > > > > On Wed, Oct 11, 2023 at 4:34 PM David Radley <
> > david_radley@uk.ibm.com>
> > > > > wrote:
> > > > >
> > > > > > Hi Zakelly,
> > > > > > Thanks for making this clear for me. We should document the
> > impact on
> > > > the
> > > > > > user in the release notes, which will be a minimal rewrite and
> > > > recompile of
> > > > > > any java using the old APIs.
> > > > > > I think it is a good point you make about if there are future
> > > > > > implementations that are
> > > > > > worth retrying (such as network access) – then there could be
> > retries.
> > > > I
> > > > > > agree we should not be trying to create code now for an
> > implementation
> > > > > > consideration that is not there yet,
> > > > > >
> > > > > > +1 from me ,
> > > > > > Kind regards, David.
> > > > > >
> > > > > > From: Zakelly Lan <za...@gmail.com>
> > > > > > Date: Wednesday, 11 October 2023 at 04:25
> > > > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > > > Subject: [EXTERNAL] Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the
> > > > > > exceptions thrown in state interfaces
> > > > > > Hi David,
> > > > > >
> > > > > > Thanks for your response.
> > > > > >
> > > > > > The exceptions thrown by state interfaces are NOT retriable. For
> > > > > > example, there may be some elements sent to the wrong subtask due
> > to a
> > > > > > non-deterministic hashCode() algorithm and the key group is not
> > > > > > matching. Or the rocksdb may fail to read a file if it has been
> > > > > > deleted by the user. If there are future implementations that are
> > > > > > worth retrying (such as network access), it would be better to
> let
> > the
> > > > > > implementation itself handle the retries and provide a
> > configuration
> > > > > > for this, rather than requiring users to catch these exceptions.
> > > > > >
> > > > > > Regarding the release and documentation, I have mentioned that
> this
> > > > > > change is targeted for version 1.19 with proper documentation.
> You
> > may
> > > > > > have noticed that state interfaces are annotated with
> > @PublicEvolving,
> > > > > > which means these interfaces may change across versions. The
> > changes
> > > > > > are suitable for a minor release (1.18.0 currently to 1.19.0 in
> the
> > > > > > future) as defined by the API compatibility guarantees of
> Flink[1].
> > > > > >
> > > > > >
> > > > > >
> > > > > > Best,
> > > > > > Zakelly
> > > > > >
> > > > > >
> > > > > > [1]
> > > > > >
> > > >
> >
> https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees
> > > > > >
> > > > > > On Tue, Oct 10, 2023 at 6:19 PM David Radley <
> > david_radley@uk.ibm.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi,
> > > > > > > I notice
> > > > > >
> > > >
> >
> https://nightlies.apache.org/flink/flink-docs-master/api/java/org/apache/flink/api/common/state/ValueState.html
> > > > > > is an external API. I am concerned that this change will break
> > existing
> > > > > > applications using the old interface, they are likely to have
> > catches /
> > > > > > throws around the existing checked Exceptions.
> > > > > > >
> > > > > > > If we go with RunTimeException, I would suggest that this sort
> of
> > > > > > breaking change should be done on a Flink version change, where
> it
> > is
> > > > > > appropriate to make breaking changes to the API with associated
> > > > > > documentation.
> > > > > > >
> > > > > > > If we want this change on a minor release, we could create a
> new
> > > > class
> > > > > > ValueState2– that is used internally with the cleaned up
> > Exceptions,
> > > > but
> > > > > > still expose the old class and Exceptions for existing external
> > > > > > applications. I guess new applications could use the new
> > ValueState2 .
> > > > > > >
> > > > > > > What do you think?
> > > > > > > Kind regards, David.
> > > > > > >
> > > > > > >
> > > > > > > From: David Radley <da...@uk.ibm.com>
> > > > > > > Date: Tuesday, 10 October 2023 at 09:49
> > > > > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > > > > Subject: [EXTERNAL] RE: [DISCUSS] FLIP-368 Reorganize the
> > exceptions
> > > > > > thrown in state interfaces
> > > > > > > Hi ,
> > > > > > > The argument seems to be that the errors cannot be acted on so
> > > > should be
> > > > > > runtime exceptions. I want to confirm that none of these errors
> > could /
> > > > > > should be retriable. If there is a possibility that the state is
> > > > available
> > > > > > at some time later then I assume a checked retriable Exception
> > would be
> > > > > > appropriate for those cases; and be part of the contract with the
> > > > caller.
> > > > > > Can we be sure that there is no possibility that the state will
> > become
> > > > > > available; if so then I agree that a runtime Exception is
> > appropriate.
> > > > What
> > > > > > do you think?
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > Kind regards, David.
> > > > > > >
> > > > > > >
> > > > > > > From: Zakelly Lan <za...@gmail.com>
> > > > > > > Date: Monday, 9 October 2023 at 18:12
> > > > > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > > > > Subject: [EXTERNAL] Re: [DISCUSS] FLIP-368 Reorganize the
> > exceptions
> > > > > > thrown in state interfaces
> > > > > > > Hi everyone,
> > > > > > >
> > > > > > > It seems we're gradually reaching a consensus. So I would like
> to
> > > > > > > start a vote after 72 hours if there are no further
> discussions.
> > > > > > >
> > > > > > > Please let me know if you have any concerns, thanks!
> > > > > > >
> > > > > > >
> > > > > > > Best,
> > > > > > > Zakelly
> > > > > > >
> > > > > > >
> > > > > > > On Sat, Oct 7, 2023 at 4:07 PM Zakelly Lan <
> > zakelly.lan@gmail.com>
> > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi Jing,
> > > > > > > >
> > > > > > > > Sorry for the late reply! I agree with you that we do not
> > expect
> > > > users
> > > > > > > > to do anything with Flink and we won't "bother" them with
> those
> > > > > > > > exceptions. However, users can still catch the `Throwable`
> and
> > > > perform
> > > > > > > > any necessary logging activities, similar to how they use
> Java
> > > > > > > > Collection interfaces.
> > > > > > > >
> > > > > > > >
> > > > > > > > Thanks for your insights!
> > > > > > > >
> > > > > > > > Best,
> > > > > > > > Zakelly
> > > > > > > >
> > > > > > > > On Thu, Sep 21, 2023 at 8:43 PM Jing Ge
> > <jing@ververica.com.invalid
> > > > >
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Fair enough! Thanks Zakelly for the information. Afaic,
> even
> > > > users
> > > > > > can do
> > > > > > > > > nothing with Flink, they still can do something in their
> > > > territory,
> > > > > > at
> > > > > > > > > least doing some logging and metrics stuff, or triggering
> > some
> > > > other
> > > > > > > > > services in their ecosystem. After all, the Flink jobs they
> > build
> > > > > > are part
> > > > > > > > > of their service component. It didn't change the fact that
> > we are
> > > > > > going to
> > > > > > > > > use the anti-pattern. Just because we didn't expect users
> to
> > do
> > > > > > > > > anything with Flink, does not mean users don't expect to do
> > > > > > something with
> > > > > > > > > the expected exception. Anyway, I am open to hearing
> > different
> > > > > > opinions.
> > > > > > > > >
> > > > > > > > > Best regards,
> > > > > > > > > Jing
> > > > > > > > >
> > > > > > > > > On Thu, Sep 21, 2023 at 7:02 AM Zakelly Lan <
> > > > zakelly.lan@gmail.com>
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi Martijn,
> > > > > > > > > >
> > > > > > > > > > Thanks for the reminder!
> > > > > > > > > >
> > > > > > > > > > This FLIP proposes a change to the state API that is
> > annotated
> > > > as
> > > > > > > > > > @PublicEvolving and targets version 1.19. I have
> clarified
> > > > this in
> > > > > > > > > > the "Proposed Change" section of the FLIP.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Hi Jing,
> > > > > > > > > >
> > > > > > > > > > Thanks for sharing your thoughts! Here are my opinions:
> > > > > > > > > >
> > > > > > > > > > 1. The exceptions of the state API are usually treated as
> > > > critical
> > > > > > > > > > ones. In other words, if anything goes wrong with state
> > > > accessing,
> > > > > > the
> > > > > > > > > > element processing cannot proceed and the job should
> fail.
> > > > Flink
> > > > > > users
> > > > > > > > > > may not know what to do when they encounter these
> > exceptions. I
> > > > > > > > > > believe this is the main reason why we want to replace
> them
> > > > with
> > > > > > > > > > unchecked exceptions.
> > > > > > > > > > 2. There have also been some further discussions[1][2]
> from
> > > > Stephan
> > > > > > > > > > and Shixiaogang below the one you pointed out [3], and it
> > seems
> > > > > > they
> > > > > > > > > > come to an agreement to use unchecked exceptions. After
> > > > reviewing
> > > > > > the
> > > > > > > > > > entire discussion on that PR, I think their arguments are
> > > > > > reasonable
> > > > > > > > > > given the use case.
> > > > > > > > > >
> > > > > > > > > > Looking forward to your feedback.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Best,
> > > > > > > > > > Zakelly
> > > > > > > > > >
> > > > > > > > > > [1]
> > > > > > https://github.com/apache/flink/pull/3380#issuecomment-286807853
> > > > > > > > > > [2]
> > > > > > https://github.com/apache/flink/pull/3380#issuecomment-286932133
> > > > > > > > > > [3]
> > > > > > https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > > > > > > >
> > > > > > > > > > On Thu, Sep 21, 2023 at 1:27 AM Jing Ge
> > > > <jing@ververica.com.invalid
> > > > > > >
> > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > sorry, typo: It is a known "anti-pattern" instead of
> > > > > > "ant-pattern"
> > > > > > > > > > >
> > > > > > > > > > > Best regards,
> > > > > > > > > > > Jing
> > > > > > > > > > >
> > > > > > > > > > > On Wed, Sep 20, 2023 at 7:23 PM Jing Ge <
> > jing@ververica.com>
> > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > > Hi Zakelly,
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks for driving this topic. From good software
> > > > engineering's
> > > > > > > > > > > > perspective, I have different thoughts:
> > > > > > > > > > > >
> > > > > > > > > > > > 1. The idea to get rid of all checked Exceptions and
> > > > replace
> > > > > > them with
> > > > > > > > > > > > unchecked Exceptions is a known ant-pattern:
> "Generally
> > > > > > speaking, do
> > > > > > > > > > not
> > > > > > > > > > > > throw a RuntimeException or create a subclass of
> > > > > > RuntimeException
> > > > > > > > > > simply
> > > > > > > > > > > > because you don't want to be bothered with specifying
> > the
> > > > > > exceptions
> > > > > > > > > > your
> > > > > > > > > > > > methods can throw." [1] Checked Exceptions mean
> > expected
> > > > > > exceptions
> > > > > > > > > > that
> > > > > > > > > > > > can help developers find a way to catch them and
> decide
> > > > what
> > > > > > to do. It
> > > > > > > > > > is
> > > > > > > > > > > > part of the public API signature that can help
> > developers
> > > > > > build robust
> > > > > > > > > > > > systems. We should not mix concepts and build
> expected
> > > > > > exceptions with
> > > > > > > > > > > > unchecked Java Exception classes.
> > > > > > > > > > > > 2. The comment Stephan left [2] clearly pointed out
> > that we
> > > > > > should
> > > > > > > > > > avoid
> > > > > > > > > > > > using generic Java Exceptions, and "find some more
> > > > 'specific'
> > > > > > > > > > exceptions
> > > > > > > > > > > > for the signature, like throws IOException or throws
> > > > > > > > > > StateAccessException."
> > > > > > > > > > > > So, the idea is to define/use specific checked
> > Exception
> > > > > > classes
> > > > > > > > > > instead of
> > > > > > > > > > > > using unchecked Exceptions.
> > > > > > > > > > > >
> > > > > > > > > > > > Looking forward to your thoughts.
> > > > > > > > > > > >
> > > > > > > > > > > > Best regards,
> > > > > > > > > > > > Jing
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > [1]
> > > > > > > > > > > >
> > > > > > > > > >
> > > > > >
> > > >
> >
> https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
> > > > > > > > > > > > [2]
> > > > > > https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > > > > > > > > >
> > > > > > > > > > > > On Wed, Sep 20, 2023 at 4:52 PM Zakelly Lan <
> > > > > > zakelly.lan@gmail.com>
> > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > >> Hi Yanfei,
> > > > > > > > > > > >>
> > > > > > > > > > > >> Thanks for your reply!
> > > > > > > > > > > >>
> > > > > > > > > > > >> Yes, this FLIP aims to change all state-related
> > > > exceptions to
> > > > > > > > > > > >> unchecked exceptions and remove all exceptions from
> > the
> > > > > > signature. So
> > > > > > > > > > > >> I believe we have come to an agreement to keep the
> > > > interfaces
> > > > > > simple.
> > > > > > > > > > > >>
> > > > > > > > > > > >>
> > > > > > > > > > > >> Best regards,
> > > > > > > > > > > >> Zakelly
> > > > > > > > > > > >>
> > > > > > > > > > > >> On Wed, Sep 20, 2023 at 2:26 PM Zakelly Lan <
> > > > > > zakelly.lan@gmail.com>
> > > > > > > > > > > >> wrote:
> > > > > > > > > > > >> >
> > > > > > > > > > > >> > Hi Hangxiang,
> > > > > > > > > > > >> >
> > > > > > > > > > > >> > Thank you for your response! Here are my thoughts:
> > > > > > > > > > > >> >
> > > > > > > > > > > >> > 1. Regarding the exceptions thrown by internal
> > > > interfaces,
> > > > > > I suggest
> > > > > > > > > > > >> > keeping them as checked exceptions. Since these
> > > > exceptions
> > > > > > will be
> > > > > > > > > > > >> > handled by the internal callers, it is meaningful
> to
> > > > throw
> > > > > > them as
> > > > > > > > > > > >> > checked ones. If we need to make changes to these
> > > > classes,
> > > > > > we can
> > > > > > > > > > > >> > create separate tickets alongside this FLIP. What
> > are
> > > > your
> > > > > > thoughts
> > > > > > > > > > on
> > > > > > > > > > > >> > this?
> > > > > > > > > > > >> > 2. StateIOException is primarily thrown by
> > file-based
> > > > state
> > > > > > like
> > > > > > > > > > > >> > RocksDB, while StateAccessException is more
> generic
> > and
> > > > can
> > > > > > be
> > > > > > > > > > thrown
> > > > > > > > > > > >> > by heap states. Additionally, I believe there may
> be
> > > > more
> > > > > > subclasses
> > > > > > > > > > > >> > of StateAccessException that we can add. We can
> > consider
> > > > > > this when
> > > > > > > > > > > >> > implementing.
> > > > > > > > > > > >> > 3. I would like to make this change in version
> > 1.19. As
> > > > > > mentioned in
> > > > > > > > > > > >> > this FLIP, many users do not catch any exceptions
> > since
> > > > the
> > > > > > element
> > > > > > > > > > > >> > processing function exposes the exception to the
> > upper
> > > > > > layer.
> > > > > > > > > > > >> > Therefore, the impact is manageable. And I
> > completely
> > > > agree
> > > > > > that we
> > > > > > > > > > > >> > should clearly document this change in the next
> > release
> > > > > > notes.
> > > > > > > > > > > >> >
> > > > > > > > > > > >> >
> > > > > > > > > > > >> > Best regards,
> > > > > > > > > > > >> > Zakelly
> > > > > > > > > > > >> >
> > > > > > > > > > > >> > On Wed, Sep 20, 2023 at 12:35 PM Yanfei Lei <
> > > > > > fredialei@gmail.com>
> > > > > > > > > > > >> wrote:
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > Hi Zakelly,
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > Thanks for bringing this up. +1 for
> reorganizing.
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > IIUC, this proposal aims to change all
> > state-related
> > > > > > exceptions to
> > > > > > > > > > > >> > > unchecked exceptions. If users have caught
> checked
> > > > > > exceptions
> > > > > > > > > > (such as
> > > > > > > > > > > >> > > IOException ) in their code, leaving the code as
> > is
> > > > would
> > > > > > also
> > > > > > > > > > work.
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > Is it possible not to put any exceptions in the
> > > > signature
> > > > > > of
> > > > > > > > > > > >> > > user-facing interfaces? As the proposal
> mentioned,
> > > > users
> > > > > > can do a
> > > > > > > > > > few
> > > > > > > > > > > >> > > things even if they catch the exceptions.
> Keeping
> > the
> > > > > > interface
> > > > > > > > > > simple
> > > > > > > > > > > >> > > may be easier to understand.
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > Best,
> > > > > > > > > > > >> > > Yanfei
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > Hangxiang Yu <ma...@gmail.com>
> 于2023年9月19日周二
> > > > 18:16写道：
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > > Hi, Zakelly.
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > > Thanks for the proposal.
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > > +1 for reorganizing exceptions of state
> > interfaces
> > > > > > which indeed
> > > > > > > > > > > >> confuses me
> > > > > > > > > > > >> > > > currently.
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > > From my experience, users usually omit these
> > > > exceptions
> > > > > > because
> > > > > > > > > > > >> they cannot
> > > > > > > > > > > >> > > > do much even if they catch the exceptions.
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > > I have some problems and suggestions, PTAL:
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > > 1. Could we also reorganize or add more
> state
> > > > > > exceptions
> > > > > > > > > > (may be
> > > > > > > > > > > >> related
> > > > > > > > > > > >> > > > to other state interfaces/classes e.g.
> > > > > > KeyedStateBackend)
> > > > > > > > > > into
> > > > > > > > > > > >> the
> > > > > > > > > > > >> > > > exception class diagrams ? Although these
> > > > > > state-related
> > > > > > > > > > classes
> > > > > > > > > > > >> may not
> > > > > > > > > > > >> > > > be public, it could be better to consider
> > them
> > > > > > together to
> > > > > > > > > > make
> > > > > > > > > > > >> all
> > > > > > > > > > > >> > > > state-related exceptions clear. For
> example,
> > we
> > > > could
> > > > > > > > > > reorganize
> > > > > > > > > > > >> some
> > > > > > > > > > > >> > > > existing exceptions such as
> > > > StateMigrationException,
> > > > > > add some
> > > > > > > > > > > >> exceptions
> > > > > > > > > > > >> > > > such as StateNotFoundException.
> > > > > > > > > > > >> > > > 2. Could you clarify or give an example
> > about the
> > > > > > extended
> > > > > > > > > > > >> relation
> > > > > > > > > > > >> > > > "StateAccessException -- StateIOException"
> ?
> > > > When do
> > > > > > we just
> > > > > > > > > > > >> return
> > > > > > > > > > > >> > > > StateAccessException instead of
> > StateIOException
> > > > or
> > > > > > others ?
> > > > > > > > > > > >> > > > 3. Which version do you want to implement
> it
> > in ?
> > > > > > Since it
> > > > > > > > > > has
> > > > > > > > > > > >> to break
> > > > > > > > > > > >> > > > changes for users who have catched the
> > > > IOException,
> > > > > > If we
> > > > > > > > > > want
> > > > > > > > > > > >> to implement
> > > > > > > > > > > >> > > > it in 1.19, we must mark it very clearly in
> > the
> > > > > > release
> > > > > > > > > > note, or
> > > > > > > > > > > >> we should
> > > > > > > > > > > >> > > > make it in 2.0.
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > > On Tue, Sep 19, 2023 at 5:08 PM Zakelly Lan <
> > > > > > > > > > zakelly.lan@gmail.com>
> > > > > > > > > > > >> wrote:
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > > > Hi everyone,
> > > > > > > > > > > >> > > > >
> > > > > > > > > > > >> > > > > I would like to initiate a discussion on
> > FLIP-368,
> > > > > > which
> > > > > > > > > > focuses
> > > > > > > > > > > >> on
> > > > > > > > > > > >> > > > > reorganizing the exceptions thrown in state
> > > > > > interfaces [1].
> > > > > > > > > > > >> > > > >
> > > > > > > > > > > >> > > > > Currently, we have identified several
> problems
> > > > with
> > > > > > the
> > > > > > > > > > exceptions
> > > > > > > > > > > >> > > > > thrown by state-related interfaces:
> > > > > > > > > > > >> > > > > 1. The exception types thrown by each
> > interface
> > > > are
> > > > > > > > > > > >> inconsistent.
> > > > > > > > > > > >> > > > > While most of the interfaces claim to throw
> > > > > > `Exception`, the
> > > > > > > > > > > >> > > > > interfaces of `ValueState` throw
> > `IOException`.
> > > > > > Additionally,
> > > > > > > > > > the
> > > > > > > > > > > >> > > > > `State#clear()` never throws an exception.
> > This
> > > > can be
> > > > > > > > > > confusing
> > > > > > > > > > > >> for
> > > > > > > > > > > >> > > > > users.
> > > > > > > > > > > >> > > > > 2. The use of `Exception` or `IOException`
> > as
> > > > the
> > > > > > thrown
> > > > > > > > > > > >> exception
> > > > > > > > > > > >> > > > > type is too generic and lacks specificity.
> > > > > > > > > > > >> > > > > 3. Users may not be able to handle these
> > > > > > exceptions. In
> > > > > > > > > > cases
> > > > > > > > > > > >> where
> > > > > > > > > > > >> > > > > an exception occurs while accessing state,
> > the job
> > > > > > should
> > > > > > > > > > fail.
> > > > > > > > > > > >> This
> > > > > > > > > > > >> > > > > aligns more with the characteristic of
> > *unchecked
> > > > > > exceptions*
> > > > > > > > > > > >> instead
> > > > > > > > > > > >> > > > > of checked exceptions.
> > > > > > > > > > > >> > > > >
> > > > > > > > > > > >> > > > > To address these issues, we borrow the idea
> of
> > > > > > throwing
> > > > > > > > > > unchecked
> > > > > > > > > > > >> > > > > exceptions in Java Collection API and
> propose
> > the
> > > > > > following
> > > > > > > > > > > >> changes in
> > > > > > > > > > > >> > > > > state-related exceptions:
> > > > > > > > > > > >> > > > > 1. Introduction of specific unchecked
> > exception
> > > > > > types for
> > > > > > > > > > > >> different
> > > > > > > > > > > >> > > > > reasons, providing users with more precise
> > > > > > information about
> > > > > > > > > > the
> > > > > > > > > > > >> cause
> > > > > > > > > > > >> > > > > of the exception.
> > > > > > > > > > > >> > > > > 2. Removal of all checked exceptions from
> > > > interface
> > > > > > > > > > signatures
> > > > > > > > > > > >> and
> > > > > > > > > > > >> > > > > instead, throwing newly introduced unchecked
> > > > > > exceptions in the
> > > > > > > > > > > >> > > > > implementations.
> > > > > > > > > > > >> > > > >
> > > > > > > > > > > >> > > > > Please share your thoughts and suggestions
> > > > regarding
> > > > > > the
> > > > > > > > > > proposed
> > > > > > > > > > > >> > > > > changes. Thank you for your attention and
> > support.
> > > > > > > > > > > >> > > > >
> > > > > > > > > > > >> > > > >
> > > > > > > > > > > >> > > > > Best,
> > > > > > > > > > > >> > > > > Zakelly
> > > > > > > > > > > >> > > > >
> > > > > > > > > > > >> > > > > [1] FLIP-368: Reorganize the exceptions
> > thrown in
> > > > > > state
> > > > > > > > > > > >> interfaces,
> > > > > > > > > > > >> > > > >
> https://cwiki.apache.org/confluence/x/eZ2zDw
> > > > > > > > > > > >> > > > >
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > > --
> > > > > > > > > > > >> > > > Best,
> > > > > > > > > > > >> > > > Hangxiang.
> > > > > > > > > > > >>
> > > > > > > > > > > >
> > > > > > > > > >
> > > > > > >
> > > > > > > Unless otherwise stated above:
> > > > > > >
> > > > > > > IBM United Kingdom Limited
> > > > > > > Registered in England and Wales with number 741598
> > > > > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants.
> > PO6
> > > > 3AU
> > > > > > >
> > > > > > > Unless otherwise stated above:
> > > > > > >
> > > > > > > IBM United Kingdom Limited
> > > > > > > Registered in England and Wales with number 741598
> > > > > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants.
> > PO6
> > > > 3AU
> > > > > >
> > > > > > Unless otherwise stated above:
> > > > > >
> > > > > > IBM United Kingdom Limited
> > > > > > Registered in England and Wales with number 741598
> > > > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants.
> > PO6 3AU
> > > > > >
> > > >
> > >
> > > Unless otherwise stated above:
> > >
> > > IBM United Kingdom Limited
> > > Registered in England and Wales with number 741598
> > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
> >
>
Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Posted by Jing Ge <ji...@ververica.com.INVALID>.
Hi Zakelly,
Thanks for the clarification. But I have to agree with Yuan. It is a
breaking change as you mentioned that users who catch those exceptions will
be forced to change their code. Changing it during the minor releases
violates the backward compatibility promise of Flink public(Evolving) API.
I would suggest explicitly writing down the breaking change in the FLIP and
let the community decide. For me, it might not be an optimized solution to
introduce breaking changes between minor releases, but I personally won't
block your effort.
Best regards,
Jing
On Fri, Oct 13, 2023 at 9:37 PM Zakelly Lan <za...@gmail.com> wrote:
> Hi Jing,
>
> For jobs that catch exceptions, they will need to modify their code.
> If they still wish to catch the exception, they should catch the
> `Throwable` instead.
>
> As I explained, this use case is not common because:
> 1. The user-defined processElement function, where the state APIs are
> called, already declares that it throws `Exception`, so there is no
> need for user code to catch these exceptions. The compiler or IDE will
> not prompt them to do so.
> 2. These exceptions are fatal, and users can do very little or nothing
> to prevent a job failure. Also in this case, Flink already provides
> error logging for users.
>
> And for minor releases, APIs annotated with @PublicEvolving are
> allowed to have signature changes[1]. So I think the callers will
> expect this when migrating to a new version.
>
>
> Hi David,
>
> It is true that recompilation is necessary when migrating to the next
> minor release, as Flink is a rapidly evolving project. API
> compatibility is essential, but it should not hinder development.
> That's why the API compatibility guarantees of Flink[1] were created.
> If we expect stable binaries/sources across dot versions, then we
> should prohibit any breaking changes, not just this one. Therefore, we
> can discuss a new rule for this. However, for now, this proposal
> aligns with the current rules.
>
> Regarding Flink 2.0, if my estimate is correct, it will be released at
> least one year later. The API will undergo a significant redesign, and
> user code may need to be essentially rewritten. The main focus of
> Flink 2.0 will be on API design, not on reorganizing exceptions as
> proposed here. After Flink 2.0 is released, many existing users may
> continue using Flink 1.x for a long time. This work aims to provide a
> cleaner API for Flink 1.x before we stop updating it.
>
>
> Best,
> Zakelly
>
>
> [1]
> https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees
>
>
> On Fri, Oct 13, 2023 at 11:16 PM David Radley <da...@uk.ibm.com>
> wrote:
> >
> > Hi,
> > It seems that migrating to the next dot version and finding you need to
> recompile, would be frustrating and couse unexpected work, as I suspect a
> jar file using the old API will give an exception around not finding the
> method – I am not sure how this would surface in typical applications at
> runtime.
> >
> > Technically because this is tagged as @PublicEvolving then we can make
> this breaking change.
> >
> > So there will be migration issues if people are using this API, have we
> an idea on how many of our users are using it?
> >
> > If we use @PublicEvolving then maybe we should have stable binaries that
> only include public APIs and then another bleeding edge package containing
> @PublicEvolving content, so users can choose.
> >
> > Organisations I have worked with would not tend to want to or expect to
> have to recompile their applications on a dot version – as this would
> normally mean a lot more testing for them.
> >
> > On balance, as I am risk averse, I would suggest delaying this to v2 as
> Jing has proposed. This is a cleaner API, is there a demand for this in a
> dot version? If the community think this is too risk averse, then we could
> go with 1.19.
> > WDYT?
> >
> > Kind regards, David.
> >
> >
> >
> > From: Jing Ge <ji...@ververica.com.INVALID>
> > Date: Friday, 13 October 2023 at 14:30
> > To: dev@flink.apache.org <de...@flink.apache.org>
> > Subject: [EXTERNAL] Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the
> exceptions thrown in state interfaces
> > HI Zakelly,
> >
> > What about the jobs that catch those exceptions? Will these downstream
> > callers that expect this exception to be thrown break, since the
> exceptions
> > are part of the method's contract?
> >
> > Best regards,
> > Jing
> >
> > On Fri, Oct 13, 2023 at 11:01 AM Zakelly Lan <za...@gmail.com>
> wrote:
> >
> > > Hi Yuan,
> > >
> > > Thanks for sharing your thoughts!
> > >
> > > In Flink 2.0 I believe we could design brand-new state APIs, which is
> > > uncertain and may take some time. However, this proposal primarily
> > > focuses on improving the consistency of interface definitions and
> > > enhancing the user experience in error handling for the current APIs.
> > > Therefore, I would prefer to make it in version 1.19. Furthermore, the
> > > impact of this API change can be controlled since most Flink users do
> > > not actively catch these exceptions. For them, a simple code recompile
> > > is sufficient and acceptable when migrating to a new minor release.
> > > The change is not that big that we need a major version to apply.
> > >
> > >
> > > Best,
> > > Zakelly
> > >
> > > On Fri, Oct 13, 2023 at 3:03 PM Yuan Mei <yu...@gmail.com>
> wrote:
> > > >
> > > > +1 for the proposal
> > > >
> > > > But "Since the signature of the public state API has been changed",
> I was
> > > > wondering whether this would be more fittable in Flink 2.0, instead
> of
> > > 1.19?
> > > >
> > > > WDYT?
> > > >
> > > > Best
> > > > Yuan
> > > >
> > > > On Wed, Oct 11, 2023 at 4:34 PM David Radley <
> david_radley@uk.ibm.com>
> > > > wrote:
> > > >
> > > > > Hi Zakelly,
> > > > > Thanks for making this clear for me. We should document the
> impact on
> > > the
> > > > > user in the release notes, which will be a minimal rewrite and
> > > recompile of
> > > > > any java using the old APIs.
> > > > > I think it is a good point you make about if there are future
> > > > > implementations that are
> > > > > worth retrying (such as network access) – then there could be
> retries.
> > > I
> > > > > agree we should not be trying to create code now for an
> implementation
> > > > > consideration that is not there yet,
> > > > >
> > > > > +1 from me ,
> > > > > Kind regards, David.
> > > > >
> > > > > From: Zakelly Lan <za...@gmail.com>
> > > > > Date: Wednesday, 11 October 2023 at 04:25
> > > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > > Subject: [EXTERNAL] Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the
> > > > > exceptions thrown in state interfaces
> > > > > Hi David,
> > > > >
> > > > > Thanks for your response.
> > > > >
> > > > > The exceptions thrown by state interfaces are NOT retriable. For
> > > > > example, there may be some elements sent to the wrong subtask due
> to a
> > > > > non-deterministic hashCode() algorithm and the key group is not
> > > > > matching. Or the rocksdb may fail to read a file if it has been
> > > > > deleted by the user. If there are future implementations that are
> > > > > worth retrying (such as network access), it would be better to let
> the
> > > > > implementation itself handle the retries and provide a
> configuration
> > > > > for this, rather than requiring users to catch these exceptions.
> > > > >
> > > > > Regarding the release and documentation, I have mentioned that this
> > > > > change is targeted for version 1.19 with proper documentation. You
> may
> > > > > have noticed that state interfaces are annotated with
> @PublicEvolving,
> > > > > which means these interfaces may change across versions. The
> changes
> > > > > are suitable for a minor release (1.18.0 currently to 1.19.0 in the
> > > > > future) as defined by the API compatibility guarantees of Flink[1].
> > > > >
> > > > >
> > > > >
> > > > > Best,
> > > > > Zakelly
> > > > >
> > > > >
> > > > > [1]
> > > > >
> > >
> https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees
> > > > >
> > > > > On Tue, Oct 10, 2023 at 6:19 PM David Radley <
> david_radley@uk.ibm.com>
> > > > > wrote:
> > > > > >
> > > > > > Hi,
> > > > > > I notice
> > > > >
> > >
> https://nightlies.apache.org/flink/flink-docs-master/api/java/org/apache/flink/api/common/state/ValueState.html
> > > > > is an external API. I am concerned that this change will break
> existing
> > > > > applications using the old interface, they are likely to have
> catches /
> > > > > throws around the existing checked Exceptions.
> > > > > >
> > > > > > If we go with RunTimeException, I would suggest that this sort of
> > > > > breaking change should be done on a Flink version change, where it
> is
> > > > > appropriate to make breaking changes to the API with associated
> > > > > documentation.
> > > > > >
> > > > > > If we want this change on a minor release, we could create a new
> > > class
> > > > > ValueState2– that is used internally with the cleaned up
> Exceptions,
> > > but
> > > > > still expose the old class and Exceptions for existing external
> > > > > applications. I guess new applications could use the new
> ValueState2 .
> > > > > >
> > > > > > What do you think?
> > > > > > Kind regards, David.
> > > > > >
> > > > > >
> > > > > > From: David Radley <da...@uk.ibm.com>
> > > > > > Date: Tuesday, 10 October 2023 at 09:49
> > > > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > > > Subject: [EXTERNAL] RE: [DISCUSS] FLIP-368 Reorganize the
> exceptions
> > > > > thrown in state interfaces
> > > > > > Hi ,
> > > > > > The argument seems to be that the errors cannot be acted on so
> > > should be
> > > > > runtime exceptions. I want to confirm that none of these errors
> could /
> > > > > should be retriable. If there is a possibility that the state is
> > > available
> > > > > at some time later then I assume a checked retriable Exception
> would be
> > > > > appropriate for those cases; and be part of the contract with the
> > > caller.
> > > > > Can we be sure that there is no possibility that the state will
> become
> > > > > available; if so then I agree that a runtime Exception is
> appropriate.
> > > What
> > > > > do you think?
> > > > > >
> > > > > >
> > > > > >
> > > > > > Kind regards, David.
> > > > > >
> > > > > >
> > > > > > From: Zakelly Lan <za...@gmail.com>
> > > > > > Date: Monday, 9 October 2023 at 18:12
> > > > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > > > Subject: [EXTERNAL] Re: [DISCUSS] FLIP-368 Reorganize the
> exceptions
> > > > > thrown in state interfaces
> > > > > > Hi everyone,
> > > > > >
> > > > > > It seems we're gradually reaching a consensus. So I would like to
> > > > > > start a vote after 72 hours if there are no further discussions.
> > > > > >
> > > > > > Please let me know if you have any concerns, thanks!
> > > > > >
> > > > > >
> > > > > > Best,
> > > > > > Zakelly
> > > > > >
> > > > > >
> > > > > > On Sat, Oct 7, 2023 at 4:07 PM Zakelly Lan <
> zakelly.lan@gmail.com>
> > > > > wrote:
> > > > > > >
> > > > > > > Hi Jing,
> > > > > > >
> > > > > > > Sorry for the late reply! I agree with you that we do not
> expect
> > > users
> > > > > > > to do anything with Flink and we won't "bother" them with those
> > > > > > > exceptions. However, users can still catch the `Throwable` and
> > > perform
> > > > > > > any necessary logging activities, similar to how they use Java
> > > > > > > Collection interfaces.
> > > > > > >
> > > > > > >
> > > > > > > Thanks for your insights!
> > > > > > >
> > > > > > > Best,
> > > > > > > Zakelly
> > > > > > >
> > > > > > > On Thu, Sep 21, 2023 at 8:43 PM Jing Ge
> <jing@ververica.com.invalid
> > > >
> > > > > wrote:
> > > > > > > >
> > > > > > > > Fair enough! Thanks Zakelly for the information. Afaic, even
> > > users
> > > > > can do
> > > > > > > > nothing with Flink, they still can do something in their
> > > territory,
> > > > > at
> > > > > > > > least doing some logging and metrics stuff, or triggering
> some
> > > other
> > > > > > > > services in their ecosystem. After all, the Flink jobs they
> build
> > > > > are part
> > > > > > > > of their service component. It didn't change the fact that
> we are
> > > > > going to
> > > > > > > > use the anti-pattern. Just because we didn't expect users to
> do
> > > > > > > > anything with Flink, does not mean users don't expect to do
> > > > > something with
> > > > > > > > the expected exception. Anyway, I am open to hearing
> different
> > > > > opinions.
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > > Jing
> > > > > > > >
> > > > > > > > On Thu, Sep 21, 2023 at 7:02 AM Zakelly Lan <
> > > zakelly.lan@gmail.com>
> > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi Martijn,
> > > > > > > > >
> > > > > > > > > Thanks for the reminder!
> > > > > > > > >
> > > > > > > > > This FLIP proposes a change to the state API that is
> annotated
> > > as
> > > > > > > > > @PublicEvolving and targets version 1.19. I have clarified
> > > this in
> > > > > > > > > the "Proposed Change" section of the FLIP.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Hi Jing,
> > > > > > > > >
> > > > > > > > > Thanks for sharing your thoughts! Here are my opinions:
> > > > > > > > >
> > > > > > > > > 1. The exceptions of the state API are usually treated as
> > > critical
> > > > > > > > > ones. In other words, if anything goes wrong with state
> > > accessing,
> > > > > the
> > > > > > > > > element processing cannot proceed and the job should fail.
> > > Flink
> > > > > users
> > > > > > > > > may not know what to do when they encounter these
> exceptions. I
> > > > > > > > > believe this is the main reason why we want to replace them
> > > with
> > > > > > > > > unchecked exceptions.
> > > > > > > > > 2. There have also been some further discussions[1][2] from
> > > Stephan
> > > > > > > > > and Shixiaogang below the one you pointed out [3], and it
> seems
> > > > > they
> > > > > > > > > come to an agreement to use unchecked exceptions. After
> > > reviewing
> > > > > the
> > > > > > > > > entire discussion on that PR, I think their arguments are
> > > > > reasonable
> > > > > > > > > given the use case.
> > > > > > > > >
> > > > > > > > > Looking forward to your feedback.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Best,
> > > > > > > > > Zakelly
> > > > > > > > >
> > > > > > > > > [1]
> > > > > https://github.com/apache/flink/pull/3380#issuecomment-286807853
> > > > > > > > > [2]
> > > > > https://github.com/apache/flink/pull/3380#issuecomment-286932133
> > > > > > > > > [3]
> > > > > https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > > > > > >
> > > > > > > > > On Thu, Sep 21, 2023 at 1:27 AM Jing Ge
> > > <jing@ververica.com.invalid
> > > > > >
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > sorry, typo: It is a known "anti-pattern" instead of
> > > > > "ant-pattern"
> > > > > > > > > >
> > > > > > > > > > Best regards,
> > > > > > > > > > Jing
> > > > > > > > > >
> > > > > > > > > > On Wed, Sep 20, 2023 at 7:23 PM Jing Ge <
> jing@ververica.com>
> > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Hi Zakelly,
> > > > > > > > > > >
> > > > > > > > > > > Thanks for driving this topic. From good software
> > > engineering's
> > > > > > > > > > > perspective, I have different thoughts:
> > > > > > > > > > >
> > > > > > > > > > > 1. The idea to get rid of all checked Exceptions and
> > > replace
> > > > > them with
> > > > > > > > > > > unchecked Exceptions is a known ant-pattern: "Generally
> > > > > speaking, do
> > > > > > > > > not
> > > > > > > > > > > throw a RuntimeException or create a subclass of
> > > > > RuntimeException
> > > > > > > > > simply
> > > > > > > > > > > because you don't want to be bothered with specifying
> the
> > > > > exceptions
> > > > > > > > > your
> > > > > > > > > > > methods can throw." [1] Checked Exceptions mean
> expected
> > > > > exceptions
> > > > > > > > > that
> > > > > > > > > > > can help developers find a way to catch them and decide
> > > what
> > > > > to do. It
> > > > > > > > > is
> > > > > > > > > > > part of the public API signature that can help
> developers
> > > > > build robust
> > > > > > > > > > > systems. We should not mix concepts and build expected
> > > > > exceptions with
> > > > > > > > > > > unchecked Java Exception classes.
> > > > > > > > > > > 2. The comment Stephan left [2] clearly pointed out
> that we
> > > > > should
> > > > > > > > > avoid
> > > > > > > > > > > using generic Java Exceptions, and "find some more
> > > 'specific'
> > > > > > > > > exceptions
> > > > > > > > > > > for the signature, like throws IOException or throws
> > > > > > > > > StateAccessException."
> > > > > > > > > > > So, the idea is to define/use specific checked
> Exception
> > > > > classes
> > > > > > > > > instead of
> > > > > > > > > > > using unchecked Exceptions.
> > > > > > > > > > >
> > > > > > > > > > > Looking forward to your thoughts.
> > > > > > > > > > >
> > > > > > > > > > > Best regards,
> > > > > > > > > > > Jing
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > [1]
> > > > > > > > > > >
> > > > > > > > >
> > > > >
> > >
> https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
> > > > > > > > > > > [2]
> > > > > https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > > > > > > > >
> > > > > > > > > > > On Wed, Sep 20, 2023 at 4:52 PM Zakelly Lan <
> > > > > zakelly.lan@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > >> Hi Yanfei,
> > > > > > > > > > >>
> > > > > > > > > > >> Thanks for your reply!
> > > > > > > > > > >>
> > > > > > > > > > >> Yes, this FLIP aims to change all state-related
> > > exceptions to
> > > > > > > > > > >> unchecked exceptions and remove all exceptions from
> the
> > > > > signature. So
> > > > > > > > > > >> I believe we have come to an agreement to keep the
> > > interfaces
> > > > > simple.
> > > > > > > > > > >>
> > > > > > > > > > >>
> > > > > > > > > > >> Best regards,
> > > > > > > > > > >> Zakelly
> > > > > > > > > > >>
> > > > > > > > > > >> On Wed, Sep 20, 2023 at 2:26 PM Zakelly Lan <
> > > > > zakelly.lan@gmail.com>
> > > > > > > > > > >> wrote:
> > > > > > > > > > >> >
> > > > > > > > > > >> > Hi Hangxiang,
> > > > > > > > > > >> >
> > > > > > > > > > >> > Thank you for your response! Here are my thoughts:
> > > > > > > > > > >> >
> > > > > > > > > > >> > 1. Regarding the exceptions thrown by internal
> > > interfaces,
> > > > > I suggest
> > > > > > > > > > >> > keeping them as checked exceptions. Since these
> > > exceptions
> > > > > will be
> > > > > > > > > > >> > handled by the internal callers, it is meaningful to
> > > throw
> > > > > them as
> > > > > > > > > > >> > checked ones. If we need to make changes to these
> > > classes,
> > > > > we can
> > > > > > > > > > >> > create separate tickets alongside this FLIP. What
> are
> > > your
> > > > > thoughts
> > > > > > > > > on
> > > > > > > > > > >> > this?
> > > > > > > > > > >> > 2. StateIOException is primarily thrown by
> file-based
> > > state
> > > > > like
> > > > > > > > > > >> > RocksDB, while StateAccessException is more generic
> and
> > > can
> > > > > be
> > > > > > > > > thrown
> > > > > > > > > > >> > by heap states. Additionally, I believe there may be
> > > more
> > > > > subclasses
> > > > > > > > > > >> > of StateAccessException that we can add. We can
> consider
> > > > > this when
> > > > > > > > > > >> > implementing.
> > > > > > > > > > >> > 3. I would like to make this change in version
> 1.19. As
> > > > > mentioned in
> > > > > > > > > > >> > this FLIP, many users do not catch any exceptions
> since
> > > the
> > > > > element
> > > > > > > > > > >> > processing function exposes the exception to the
> upper
> > > > > layer.
> > > > > > > > > > >> > Therefore, the impact is manageable. And I
> completely
> > > agree
> > > > > that we
> > > > > > > > > > >> > should clearly document this change in the next
> release
> > > > > notes.
> > > > > > > > > > >> >
> > > > > > > > > > >> >
> > > > > > > > > > >> > Best regards,
> > > > > > > > > > >> > Zakelly
> > > > > > > > > > >> >
> > > > > > > > > > >> > On Wed, Sep 20, 2023 at 12:35 PM Yanfei Lei <
> > > > > fredialei@gmail.com>
> > > > > > > > > > >> wrote:
> > > > > > > > > > >> > >
> > > > > > > > > > >> > > Hi Zakelly,
> > > > > > > > > > >> > >
> > > > > > > > > > >> > > Thanks for bringing this up. +1 for reorganizing.
> > > > > > > > > > >> > >
> > > > > > > > > > >> > > IIUC, this proposal aims to change all
> state-related
> > > > > exceptions to
> > > > > > > > > > >> > > unchecked exceptions. If users have caught checked
> > > > > exceptions
> > > > > > > > > (such as
> > > > > > > > > > >> > > IOException ) in their code, leaving the code as
> is
> > > would
> > > > > also
> > > > > > > > > work.
> > > > > > > > > > >> > >
> > > > > > > > > > >> > > Is it possible not to put any exceptions in the
> > > signature
> > > > > of
> > > > > > > > > > >> > > user-facing interfaces? As the proposal mentioned,
> > > users
> > > > > can do a
> > > > > > > > > few
> > > > > > > > > > >> > > things even if they catch the exceptions. Keeping
> the
> > > > > interface
> > > > > > > > > simple
> > > > > > > > > > >> > > may be easier to understand.
> > > > > > > > > > >> > >
> > > > > > > > > > >> > >
> > > > > > > > > > >> > > Best,
> > > > > > > > > > >> > > Yanfei
> > > > > > > > > > >> > >
> > > > > > > > > > >> > > Hangxiang Yu <ma...@gmail.com> 于2023年9月19日周二
> > > 18:16写道：
> > > > > > > > > > >> > > >
> > > > > > > > > > >> > > > Hi, Zakelly.
> > > > > > > > > > >> > > >
> > > > > > > > > > >> > > > Thanks for the proposal.
> > > > > > > > > > >> > > >
> > > > > > > > > > >> > > > +1 for reorganizing exceptions of state
> interfaces
> > > > > which indeed
> > > > > > > > > > >> confuses me
> > > > > > > > > > >> > > > currently.
> > > > > > > > > > >> > > >
> > > > > > > > > > >> > > > From my experience, users usually omit these
> > > exceptions
> > > > > because
> > > > > > > > > > >> they cannot
> > > > > > > > > > >> > > > do much even if they catch the exceptions.
> > > > > > > > > > >> > > >
> > > > > > > > > > >> > > > I have some problems and suggestions, PTAL:
> > > > > > > > > > >> > > >
> > > > > > > > > > >> > > > 1. Could we also reorganize or add more state
> > > > > exceptions
> > > > > > > > > (may be
> > > > > > > > > > >> related
> > > > > > > > > > >> > > > to other state interfaces/classes e.g.
> > > > > KeyedStateBackend)
> > > > > > > > > into
> > > > > > > > > > >> the
> > > > > > > > > > >> > > > exception class diagrams ? Although these
> > > > > state-related
> > > > > > > > > classes
> > > > > > > > > > >> may not
> > > > > > > > > > >> > > > be public, it could be better to consider
> them
> > > > > together to
> > > > > > > > > make
> > > > > > > > > > >> all
> > > > > > > > > > >> > > > state-related exceptions clear. For example,
> we
> > > could
> > > > > > > > > reorganize
> > > > > > > > > > >> some
> > > > > > > > > > >> > > > existing exceptions such as
> > > StateMigrationException,
> > > > > add some
> > > > > > > > > > >> exceptions
> > > > > > > > > > >> > > > such as StateNotFoundException.
> > > > > > > > > > >> > > > 2. Could you clarify or give an example
> about the
> > > > > extended
> > > > > > > > > > >> relation
> > > > > > > > > > >> > > > "StateAccessException -- StateIOException" ?
> > > When do
> > > > > we just
> > > > > > > > > > >> return
> > > > > > > > > > >> > > > StateAccessException instead of
> StateIOException
> > > or
> > > > > others ?
> > > > > > > > > > >> > > > 3. Which version do you want to implement it
> in ?
> > > > > Since it
> > > > > > > > > has
> > > > > > > > > > >> to break
> > > > > > > > > > >> > > > changes for users who have catched the
> > > IOException,
> > > > > If we
> > > > > > > > > want
> > > > > > > > > > >> to implement
> > > > > > > > > > >> > > > it in 1.19, we must mark it very clearly in
> the
> > > > > release
> > > > > > > > > note, or
> > > > > > > > > > >> we should
> > > > > > > > > > >> > > > make it in 2.0.
> > > > > > > > > > >> > > >
> > > > > > > > > > >> > > >
> > > > > > > > > > >> > > > On Tue, Sep 19, 2023 at 5:08 PM Zakelly Lan <
> > > > > > > > > zakelly.lan@gmail.com>
> > > > > > > > > > >> wrote:
> > > > > > > > > > >> > > >
> > > > > > > > > > >> > > > > Hi everyone,
> > > > > > > > > > >> > > > >
> > > > > > > > > > >> > > > > I would like to initiate a discussion on
> FLIP-368,
> > > > > which
> > > > > > > > > focuses
> > > > > > > > > > >> on
> > > > > > > > > > >> > > > > reorganizing the exceptions thrown in state
> > > > > interfaces [1].
> > > > > > > > > > >> > > > >
> > > > > > > > > > >> > > > > Currently, we have identified several problems
> > > with
> > > > > the
> > > > > > > > > exceptions
> > > > > > > > > > >> > > > > thrown by state-related interfaces:
> > > > > > > > > > >> > > > > 1. The exception types thrown by each
> interface
> > > are
> > > > > > > > > > >> inconsistent.
> > > > > > > > > > >> > > > > While most of the interfaces claim to throw
> > > > > `Exception`, the
> > > > > > > > > > >> > > > > interfaces of `ValueState` throw
> `IOException`.
> > > > > Additionally,
> > > > > > > > > the
> > > > > > > > > > >> > > > > `State#clear()` never throws an exception.
> This
> > > can be
> > > > > > > > > confusing
> > > > > > > > > > >> for
> > > > > > > > > > >> > > > > users.
> > > > > > > > > > >> > > > > 2. The use of `Exception` or `IOException`
> as
> > > the
> > > > > thrown
> > > > > > > > > > >> exception
> > > > > > > > > > >> > > > > type is too generic and lacks specificity.
> > > > > > > > > > >> > > > > 3. Users may not be able to handle these
> > > > > exceptions. In
> > > > > > > > > cases
> > > > > > > > > > >> where
> > > > > > > > > > >> > > > > an exception occurs while accessing state,
> the job
> > > > > should
> > > > > > > > > fail.
> > > > > > > > > > >> This
> > > > > > > > > > >> > > > > aligns more with the characteristic of
> *unchecked
> > > > > exceptions*
> > > > > > > > > > >> instead
> > > > > > > > > > >> > > > > of checked exceptions.
> > > > > > > > > > >> > > > >
> > > > > > > > > > >> > > > > To address these issues, we borrow the idea of
> > > > > throwing
> > > > > > > > > unchecked
> > > > > > > > > > >> > > > > exceptions in Java Collection API and propose
> the
> > > > > following
> > > > > > > > > > >> changes in
> > > > > > > > > > >> > > > > state-related exceptions:
> > > > > > > > > > >> > > > > 1. Introduction of specific unchecked
> exception
> > > > > types for
> > > > > > > > > > >> different
> > > > > > > > > > >> > > > > reasons, providing users with more precise
> > > > > information about
> > > > > > > > > the
> > > > > > > > > > >> cause
> > > > > > > > > > >> > > > > of the exception.
> > > > > > > > > > >> > > > > 2. Removal of all checked exceptions from
> > > interface
> > > > > > > > > signatures
> > > > > > > > > > >> and
> > > > > > > > > > >> > > > > instead, throwing newly introduced unchecked
> > > > > exceptions in the
> > > > > > > > > > >> > > > > implementations.
> > > > > > > > > > >> > > > >
> > > > > > > > > > >> > > > > Please share your thoughts and suggestions
> > > regarding
> > > > > the
> > > > > > > > > proposed
> > > > > > > > > > >> > > > > changes. Thank you for your attention and
> support.
> > > > > > > > > > >> > > > >
> > > > > > > > > > >> > > > >
> > > > > > > > > > >> > > > > Best,
> > > > > > > > > > >> > > > > Zakelly
> > > > > > > > > > >> > > > >
> > > > > > > > > > >> > > > > [1] FLIP-368: Reorganize the exceptions
> thrown in
> > > > > state
> > > > > > > > > > >> interfaces,
> > > > > > > > > > >> > > > > https://cwiki.apache.org/confluence/x/eZ2zDw
> > > > > > > > > > >> > > > >
> > > > > > > > > > >> > > >
> > > > > > > > > > >> > > >
> > > > > > > > > > >> > > > --
> > > > > > > > > > >> > > > Best,
> > > > > > > > > > >> > > > Hangxiang.
> > > > > > > > > > >>
> > > > > > > > > > >
> > > > > > > > >
> > > > > >
> > > > > > Unless otherwise stated above:
> > > > > >
> > > > > > IBM United Kingdom Limited
> > > > > > Registered in England and Wales with number 741598
> > > > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants.
> PO6
> > > 3AU
> > > > > >
> > > > > > Unless otherwise stated above:
> > > > > >
> > > > > > IBM United Kingdom Limited
> > > > > > Registered in England and Wales with number 741598
> > > > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants.
> PO6
> > > 3AU
> > > > >
> > > > > Unless otherwise stated above:
> > > > >
> > > > > IBM United Kingdom Limited
> > > > > Registered in England and Wales with number 741598
> > > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants.
> PO6 3AU
> > > > >
> > >
> >
> > Unless otherwise stated above:
> >
> > IBM United Kingdom Limited
> > Registered in England and Wales with number 741598
> > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
>
Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Posted by Zakelly Lan <za...@gmail.com>.
Hi Jing,
For jobs that catch exceptions, they will need to modify their code.
If they still wish to catch the exception, they should catch the
`Throwable` instead.
As I explained, this use case is not common because:
1. The user-defined processElement function, where the state APIs are
called, already declares that it throws `Exception`, so there is no
need for user code to catch these exceptions. The compiler or IDE will
not prompt them to do so.
2. These exceptions are fatal, and users can do very little or nothing
to prevent a job failure. Also in this case, Flink already provides
error logging for users.
And for minor releases, APIs annotated with @PublicEvolving are
allowed to have signature changes[1]. So I think the callers will
expect this when migrating to a new version.
Hi David,
It is true that recompilation is necessary when migrating to the next
minor release, as Flink is a rapidly evolving project. API
compatibility is essential, but it should not hinder development.
That's why the API compatibility guarantees of Flink[1] were created.
If we expect stable binaries/sources across dot versions, then we
should prohibit any breaking changes, not just this one. Therefore, we
can discuss a new rule for this. However, for now, this proposal
aligns with the current rules.
Regarding Flink 2.0, if my estimate is correct, it will be released at
least one year later. The API will undergo a significant redesign, and
user code may need to be essentially rewritten. The main focus of
Flink 2.0 will be on API design, not on reorganizing exceptions as
proposed here. After Flink 2.0 is released, many existing users may
continue using Flink 1.x for a long time. This work aims to provide a
cleaner API for Flink 1.x before we stop updating it.
Best,
Zakelly
[1] https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees
On Fri, Oct 13, 2023 at 11:16 PM David Radley <da...@uk.ibm.com> wrote:
>
> Hi,
> It seems that migrating to the next dot version and finding you need to recompile, would be frustrating and couse unexpected work, as I suspect a jar file using the old API will give an exception around not finding the method – I am not sure how this would surface in typical applications at runtime.
>
> Technically because this is tagged as @PublicEvolving then we can make this breaking change.
>
> So there will be migration issues if people are using this API, have we an idea on how many of our users are using it?
>
> If we use @PublicEvolving then maybe we should have stable binaries that only include public APIs and then another bleeding edge package containing @PublicEvolving content, so users can choose.
>
> Organisations I have worked with would not tend to want to or expect to have to recompile their applications on a dot version – as this would normally mean a lot more testing for them.
>
> On balance, as I am risk averse, I would suggest delaying this to v2 as Jing has proposed. This is a cleaner API, is there a demand for this in a dot version? If the community think this is too risk averse, then we could go with 1.19.
> WDYT?
>
> Kind regards, David.
>
>
>
> From: Jing Ge <ji...@ververica.com.INVALID>
> Date: Friday, 13 October 2023 at 14:30
> To: dev@flink.apache.org <de...@flink.apache.org>
> Subject: [EXTERNAL] Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
> HI Zakelly,
>
> What about the jobs that catch those exceptions? Will these downstream
> callers that expect this exception to be thrown break, since the exceptions
> are part of the method's contract?
>
> Best regards,
> Jing
>
> On Fri, Oct 13, 2023 at 11:01 AM Zakelly Lan <za...@gmail.com> wrote:
>
> > Hi Yuan,
> >
> > Thanks for sharing your thoughts!
> >
> > In Flink 2.0 I believe we could design brand-new state APIs, which is
> > uncertain and may take some time. However, this proposal primarily
> > focuses on improving the consistency of interface definitions and
> > enhancing the user experience in error handling for the current APIs.
> > Therefore, I would prefer to make it in version 1.19. Furthermore, the
> > impact of this API change can be controlled since most Flink users do
> > not actively catch these exceptions. For them, a simple code recompile
> > is sufficient and acceptable when migrating to a new minor release.
> > The change is not that big that we need a major version to apply.
> >
> >
> > Best,
> > Zakelly
> >
> > On Fri, Oct 13, 2023 at 3:03 PM Yuan Mei <yu...@gmail.com> wrote:
> > >
> > > +1 for the proposal
> > >
> > > But "Since the signature of the public state API has been changed", I was
> > > wondering whether this would be more fittable in Flink 2.0, instead of
> > 1.19?
> > >
> > > WDYT?
> > >
> > > Best
> > > Yuan
> > >
> > > On Wed, Oct 11, 2023 at 4:34 PM David Radley <da...@uk.ibm.com>
> > > wrote:
> > >
> > > > Hi Zakelly,
> > > > Thanks for making this clear for me. We should document the impact on
> > the
> > > > user in the release notes, which will be a minimal rewrite and
> > recompile of
> > > > any java using the old APIs.
> > > > I think it is a good point you make about if there are future
> > > > implementations that are
> > > > worth retrying (such as network access) – then there could be retries.
> > I
> > > > agree we should not be trying to create code now for an implementation
> > > > consideration that is not there yet,
> > > >
> > > > +1 from me ,
> > > > Kind regards, David.
> > > >
> > > > From: Zakelly Lan <za...@gmail.com>
> > > > Date: Wednesday, 11 October 2023 at 04:25
> > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > Subject: [EXTERNAL] Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the
> > > > exceptions thrown in state interfaces
> > > > Hi David,
> > > >
> > > > Thanks for your response.
> > > >
> > > > The exceptions thrown by state interfaces are NOT retriable. For
> > > > example, there may be some elements sent to the wrong subtask due to a
> > > > non-deterministic hashCode() algorithm and the key group is not
> > > > matching. Or the rocksdb may fail to read a file if it has been
> > > > deleted by the user. If there are future implementations that are
> > > > worth retrying (such as network access), it would be better to let the
> > > > implementation itself handle the retries and provide a configuration
> > > > for this, rather than requiring users to catch these exceptions.
> > > >
> > > > Regarding the release and documentation, I have mentioned that this
> > > > change is targeted for version 1.19 with proper documentation. You may
> > > > have noticed that state interfaces are annotated with @PublicEvolving,
> > > > which means these interfaces may change across versions. The changes
> > > > are suitable for a minor release (1.18.0 currently to 1.19.0 in the
> > > > future) as defined by the API compatibility guarantees of Flink[1].
> > > >
> > > >
> > > >
> > > > Best,
> > > > Zakelly
> > > >
> > > >
> > > > [1]
> > > >
> > https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees
> > > >
> > > > On Tue, Oct 10, 2023 at 6:19 PM David Radley <da...@uk.ibm.com>
> > > > wrote:
> > > > >
> > > > > Hi,
> > > > > I notice
> > > >
> > https://nightlies.apache.org/flink/flink-docs-master/api/java/org/apache/flink/api/common/state/ValueState.html
> > > > is an external API. I am concerned that this change will break existing
> > > > applications using the old interface, they are likely to have catches /
> > > > throws around the existing checked Exceptions.
> > > > >
> > > > > If we go with RunTimeException, I would suggest that this sort of
> > > > breaking change should be done on a Flink version change, where it is
> > > > appropriate to make breaking changes to the API with associated
> > > > documentation.
> > > > >
> > > > > If we want this change on a minor release, we could create a new
> > class
> > > > ValueState2– that is used internally with the cleaned up Exceptions,
> > but
> > > > still expose the old class and Exceptions for existing external
> > > > applications. I guess new applications could use the new ValueState2 .
> > > > >
> > > > > What do you think?
> > > > > Kind regards, David.
> > > > >
> > > > >
> > > > > From: David Radley <da...@uk.ibm.com>
> > > > > Date: Tuesday, 10 October 2023 at 09:49
> > > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > > Subject: [EXTERNAL] RE: [DISCUSS] FLIP-368 Reorganize the exceptions
> > > > thrown in state interfaces
> > > > > Hi ,
> > > > > The argument seems to be that the errors cannot be acted on so
> > should be
> > > > runtime exceptions. I want to confirm that none of these errors could /
> > > > should be retriable. If there is a possibility that the state is
> > available
> > > > at some time later then I assume a checked retriable Exception would be
> > > > appropriate for those cases; and be part of the contract with the
> > caller.
> > > > Can we be sure that there is no possibility that the state will become
> > > > available; if so then I agree that a runtime Exception is appropriate.
> > What
> > > > do you think?
> > > > >
> > > > >
> > > > >
> > > > > Kind regards, David.
> > > > >
> > > > >
> > > > > From: Zakelly Lan <za...@gmail.com>
> > > > > Date: Monday, 9 October 2023 at 18:12
> > > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > > Subject: [EXTERNAL] Re: [DISCUSS] FLIP-368 Reorganize the exceptions
> > > > thrown in state interfaces
> > > > > Hi everyone,
> > > > >
> > > > > It seems we're gradually reaching a consensus. So I would like to
> > > > > start a vote after 72 hours if there are no further discussions.
> > > > >
> > > > > Please let me know if you have any concerns, thanks!
> > > > >
> > > > >
> > > > > Best,
> > > > > Zakelly
> > > > >
> > > > >
> > > > > On Sat, Oct 7, 2023 at 4:07 PM Zakelly Lan <za...@gmail.com>
> > > > wrote:
> > > > > >
> > > > > > Hi Jing,
> > > > > >
> > > > > > Sorry for the late reply! I agree with you that we do not expect
> > users
> > > > > > to do anything with Flink and we won't "bother" them with those
> > > > > > exceptions. However, users can still catch the `Throwable` and
> > perform
> > > > > > any necessary logging activities, similar to how they use Java
> > > > > > Collection interfaces.
> > > > > >
> > > > > >
> > > > > > Thanks for your insights!
> > > > > >
> > > > > > Best,
> > > > > > Zakelly
> > > > > >
> > > > > > On Thu, Sep 21, 2023 at 8:43 PM Jing Ge <jing@ververica.com.invalid
> > >
> > > > wrote:
> > > > > > >
> > > > > > > Fair enough! Thanks Zakelly for the information. Afaic, even
> > users
> > > > can do
> > > > > > > nothing with Flink, they still can do something in their
> > territory,
> > > > at
> > > > > > > least doing some logging and metrics stuff, or triggering some
> > other
> > > > > > > services in their ecosystem. After all, the Flink jobs they build
> > > > are part
> > > > > > > of their service component. It didn't change the fact that we are
> > > > going to
> > > > > > > use the anti-pattern. Just because we didn't expect users to do
> > > > > > > anything with Flink, does not mean users don't expect to do
> > > > something with
> > > > > > > the expected exception. Anyway, I am open to hearing different
> > > > opinions.
> > > > > > >
> > > > > > > Best regards,
> > > > > > > Jing
> > > > > > >
> > > > > > > On Thu, Sep 21, 2023 at 7:02 AM Zakelly Lan <
> > zakelly.lan@gmail.com>
> > > > wrote:
> > > > > > >
> > > > > > > > Hi Martijn,
> > > > > > > >
> > > > > > > > Thanks for the reminder!
> > > > > > > >
> > > > > > > > This FLIP proposes a change to the state API that is annotated
> > as
> > > > > > > > @PublicEvolving and targets version 1.19. I have clarified
> > this in
> > > > > > > > the "Proposed Change" section of the FLIP.
> > > > > > > >
> > > > > > > >
> > > > > > > > Hi Jing,
> > > > > > > >
> > > > > > > > Thanks for sharing your thoughts! Here are my opinions:
> > > > > > > >
> > > > > > > > 1. The exceptions of the state API are usually treated as
> > critical
> > > > > > > > ones. In other words, if anything goes wrong with state
> > accessing,
> > > > the
> > > > > > > > element processing cannot proceed and the job should fail.
> > Flink
> > > > users
> > > > > > > > may not know what to do when they encounter these exceptions. I
> > > > > > > > believe this is the main reason why we want to replace them
> > with
> > > > > > > > unchecked exceptions.
> > > > > > > > 2. There have also been some further discussions[1][2] from
> > Stephan
> > > > > > > > and Shixiaogang below the one you pointed out [3], and it seems
> > > > they
> > > > > > > > come to an agreement to use unchecked exceptions. After
> > reviewing
> > > > the
> > > > > > > > entire discussion on that PR, I think their arguments are
> > > > reasonable
> > > > > > > > given the use case.
> > > > > > > >
> > > > > > > > Looking forward to your feedback.
> > > > > > > >
> > > > > > > >
> > > > > > > > Best,
> > > > > > > > Zakelly
> > > > > > > >
> > > > > > > > [1]
> > > > https://github.com/apache/flink/pull/3380#issuecomment-286807853
> > > > > > > > [2]
> > > > https://github.com/apache/flink/pull/3380#issuecomment-286932133
> > > > > > > > [3]
> > > > https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > > > > >
> > > > > > > > On Thu, Sep 21, 2023 at 1:27 AM Jing Ge
> > <jing@ververica.com.invalid
> > > > >
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > sorry, typo: It is a known "anti-pattern" instead of
> > > > "ant-pattern"
> > > > > > > > >
> > > > > > > > > Best regards,
> > > > > > > > > Jing
> > > > > > > > >
> > > > > > > > > On Wed, Sep 20, 2023 at 7:23 PM Jing Ge <ji...@ververica.com>
> > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi Zakelly,
> > > > > > > > > >
> > > > > > > > > > Thanks for driving this topic. From good software
> > engineering's
> > > > > > > > > > perspective, I have different thoughts:
> > > > > > > > > >
> > > > > > > > > > 1. The idea to get rid of all checked Exceptions and
> > replace
> > > > them with
> > > > > > > > > > unchecked Exceptions is a known ant-pattern: "Generally
> > > > speaking, do
> > > > > > > > not
> > > > > > > > > > throw a RuntimeException or create a subclass of
> > > > RuntimeException
> > > > > > > > simply
> > > > > > > > > > because you don't want to be bothered with specifying the
> > > > exceptions
> > > > > > > > your
> > > > > > > > > > methods can throw." [1] Checked Exceptions mean expected
> > > > exceptions
> > > > > > > > that
> > > > > > > > > > can help developers find a way to catch them and decide
> > what
> > > > to do. It
> > > > > > > > is
> > > > > > > > > > part of the public API signature that can help developers
> > > > build robust
> > > > > > > > > > systems. We should not mix concepts and build expected
> > > > exceptions with
> > > > > > > > > > unchecked Java Exception classes.
> > > > > > > > > > 2. The comment Stephan left [2] clearly pointed out that we
> > > > should
> > > > > > > > avoid
> > > > > > > > > > using generic Java Exceptions, and "find some more
> > 'specific'
> > > > > > > > exceptions
> > > > > > > > > > for the signature, like throws IOException or throws
> > > > > > > > StateAccessException."
> > > > > > > > > > So, the idea is to define/use specific checked Exception
> > > > classes
> > > > > > > > instead of
> > > > > > > > > > using unchecked Exceptions.
> > > > > > > > > >
> > > > > > > > > > Looking forward to your thoughts.
> > > > > > > > > >
> > > > > > > > > > Best regards,
> > > > > > > > > > Jing
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > [1]
> > > > > > > > > >
> > > > > > > >
> > > >
> > https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
> > > > > > > > > > [2]
> > > > https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > > > > > > >
> > > > > > > > > > On Wed, Sep 20, 2023 at 4:52 PM Zakelly Lan <
> > > > zakelly.lan@gmail.com>
> > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > >> Hi Yanfei,
> > > > > > > > > >>
> > > > > > > > > >> Thanks for your reply!
> > > > > > > > > >>
> > > > > > > > > >> Yes, this FLIP aims to change all state-related
> > exceptions to
> > > > > > > > > >> unchecked exceptions and remove all exceptions from the
> > > > signature. So
> > > > > > > > > >> I believe we have come to an agreement to keep the
> > interfaces
> > > > simple.
> > > > > > > > > >>
> > > > > > > > > >>
> > > > > > > > > >> Best regards,
> > > > > > > > > >> Zakelly
> > > > > > > > > >>
> > > > > > > > > >> On Wed, Sep 20, 2023 at 2:26 PM Zakelly Lan <
> > > > zakelly.lan@gmail.com>
> > > > > > > > > >> wrote:
> > > > > > > > > >> >
> > > > > > > > > >> > Hi Hangxiang,
> > > > > > > > > >> >
> > > > > > > > > >> > Thank you for your response! Here are my thoughts:
> > > > > > > > > >> >
> > > > > > > > > >> > 1. Regarding the exceptions thrown by internal
> > interfaces,
> > > > I suggest
> > > > > > > > > >> > keeping them as checked exceptions. Since these
> > exceptions
> > > > will be
> > > > > > > > > >> > handled by the internal callers, it is meaningful to
> > throw
> > > > them as
> > > > > > > > > >> > checked ones. If we need to make changes to these
> > classes,
> > > > we can
> > > > > > > > > >> > create separate tickets alongside this FLIP. What are
> > your
> > > > thoughts
> > > > > > > > on
> > > > > > > > > >> > this?
> > > > > > > > > >> > 2. StateIOException is primarily thrown by file-based
> > state
> > > > like
> > > > > > > > > >> > RocksDB, while StateAccessException is more generic and
> > can
> > > > be
> > > > > > > > thrown
> > > > > > > > > >> > by heap states. Additionally, I believe there may be
> > more
> > > > subclasses
> > > > > > > > > >> > of StateAccessException that we can add. We can consider
> > > > this when
> > > > > > > > > >> > implementing.
> > > > > > > > > >> > 3. I would like to make this change in version 1.19. As
> > > > mentioned in
> > > > > > > > > >> > this FLIP, many users do not catch any exceptions since
> > the
> > > > element
> > > > > > > > > >> > processing function exposes the exception to the upper
> > > > layer.
> > > > > > > > > >> > Therefore, the impact is manageable. And I completely
> > agree
> > > > that we
> > > > > > > > > >> > should clearly document this change in the next release
> > > > notes.
> > > > > > > > > >> >
> > > > > > > > > >> >
> > > > > > > > > >> > Best regards,
> > > > > > > > > >> > Zakelly
> > > > > > > > > >> >
> > > > > > > > > >> > On Wed, Sep 20, 2023 at 12:35 PM Yanfei Lei <
> > > > fredialei@gmail.com>
> > > > > > > > > >> wrote:
> > > > > > > > > >> > >
> > > > > > > > > >> > > Hi Zakelly,
> > > > > > > > > >> > >
> > > > > > > > > >> > > Thanks for bringing this up. +1 for reorganizing.
> > > > > > > > > >> > >
> > > > > > > > > >> > > IIUC, this proposal aims to change all state-related
> > > > exceptions to
> > > > > > > > > >> > > unchecked exceptions. If users have caught checked
> > > > exceptions
> > > > > > > > (such as
> > > > > > > > > >> > > IOException ) in their code, leaving the code as is
> > would
> > > > also
> > > > > > > > work.
> > > > > > > > > >> > >
> > > > > > > > > >> > > Is it possible not to put any exceptions in the
> > signature
> > > > of
> > > > > > > > > >> > > user-facing interfaces? As the proposal mentioned,
> > users
> > > > can do a
> > > > > > > > few
> > > > > > > > > >> > > things even if they catch the exceptions. Keeping the
> > > > interface
> > > > > > > > simple
> > > > > > > > > >> > > may be easier to understand.
> > > > > > > > > >> > >
> > > > > > > > > >> > >
> > > > > > > > > >> > > Best,
> > > > > > > > > >> > > Yanfei
> > > > > > > > > >> > >
> > > > > > > > > >> > > Hangxiang Yu <ma...@gmail.com> 于2023年9月19日周二
> > 18:16写道：
> > > > > > > > > >> > > >
> > > > > > > > > >> > > > Hi, Zakelly.
> > > > > > > > > >> > > >
> > > > > > > > > >> > > > Thanks for the proposal.
> > > > > > > > > >> > > >
> > > > > > > > > >> > > > +1 for reorganizing exceptions of state interfaces
> > > > which indeed
> > > > > > > > > >> confuses me
> > > > > > > > > >> > > > currently.
> > > > > > > > > >> > > >
> > > > > > > > > >> > > > From my experience, users usually omit these
> > exceptions
> > > > because
> > > > > > > > > >> they cannot
> > > > > > > > > >> > > > do much even if they catch the exceptions.
> > > > > > > > > >> > > >
> > > > > > > > > >> > > > I have some problems and suggestions, PTAL:
> > > > > > > > > >> > > >
> > > > > > > > > >> > > > 1. Could we also reorganize or add more state
> > > > exceptions
> > > > > > > > (may be
> > > > > > > > > >> related
> > > > > > > > > >> > > > to other state interfaces/classes e.g.
> > > > KeyedStateBackend)
> > > > > > > > into
> > > > > > > > > >> the
> > > > > > > > > >> > > > exception class diagrams ? Although these
> > > > state-related
> > > > > > > > classes
> > > > > > > > > >> may not
> > > > > > > > > >> > > > be public, it could be better to consider them
> > > > together to
> > > > > > > > make
> > > > > > > > > >> all
> > > > > > > > > >> > > > state-related exceptions clear. For example, we
> > could
> > > > > > > > reorganize
> > > > > > > > > >> some
> > > > > > > > > >> > > > existing exceptions such as
> > StateMigrationException,
> > > > add some
> > > > > > > > > >> exceptions
> > > > > > > > > >> > > > such as StateNotFoundException.
> > > > > > > > > >> > > > 2. Could you clarify or give an example about the
> > > > extended
> > > > > > > > > >> relation
> > > > > > > > > >> > > > "StateAccessException -- StateIOException" ?
> > When do
> > > > we just
> > > > > > > > > >> return
> > > > > > > > > >> > > > StateAccessException instead of StateIOException
> > or
> > > > others ?
> > > > > > > > > >> > > > 3. Which version do you want to implement it in ?
> > > > Since it
> > > > > > > > has
> > > > > > > > > >> to break
> > > > > > > > > >> > > > changes for users who have catched the
> > IOException,
> > > > If we
> > > > > > > > want
> > > > > > > > > >> to implement
> > > > > > > > > >> > > > it in 1.19, we must mark it very clearly in the
> > > > release
> > > > > > > > note, or
> > > > > > > > > >> we should
> > > > > > > > > >> > > > make it in 2.0.
> > > > > > > > > >> > > >
> > > > > > > > > >> > > >
> > > > > > > > > >> > > > On Tue, Sep 19, 2023 at 5:08 PM Zakelly Lan <
> > > > > > > > zakelly.lan@gmail.com>
> > > > > > > > > >> wrote:
> > > > > > > > > >> > > >
> > > > > > > > > >> > > > > Hi everyone,
> > > > > > > > > >> > > > >
> > > > > > > > > >> > > > > I would like to initiate a discussion on FLIP-368,
> > > > which
> > > > > > > > focuses
> > > > > > > > > >> on
> > > > > > > > > >> > > > > reorganizing the exceptions thrown in state
> > > > interfaces [1].
> > > > > > > > > >> > > > >
> > > > > > > > > >> > > > > Currently, we have identified several problems
> > with
> > > > the
> > > > > > > > exceptions
> > > > > > > > > >> > > > > thrown by state-related interfaces:
> > > > > > > > > >> > > > > 1. The exception types thrown by each interface
> > are
> > > > > > > > > >> inconsistent.
> > > > > > > > > >> > > > > While most of the interfaces claim to throw
> > > > `Exception`, the
> > > > > > > > > >> > > > > interfaces of `ValueState` throw `IOException`.
> > > > Additionally,
> > > > > > > > the
> > > > > > > > > >> > > > > `State#clear()` never throws an exception. This
> > can be
> > > > > > > > confusing
> > > > > > > > > >> for
> > > > > > > > > >> > > > > users.
> > > > > > > > > >> > > > > 2. The use of `Exception` or `IOException` as
> > the
> > > > thrown
> > > > > > > > > >> exception
> > > > > > > > > >> > > > > type is too generic and lacks specificity.
> > > > > > > > > >> > > > > 3. Users may not be able to handle these
> > > > exceptions. In
> > > > > > > > cases
> > > > > > > > > >> where
> > > > > > > > > >> > > > > an exception occurs while accessing state, the job
> > > > should
> > > > > > > > fail.
> > > > > > > > > >> This
> > > > > > > > > >> > > > > aligns more with the characteristic of *unchecked
> > > > exceptions*
> > > > > > > > > >> instead
> > > > > > > > > >> > > > > of checked exceptions.
> > > > > > > > > >> > > > >
> > > > > > > > > >> > > > > To address these issues, we borrow the idea of
> > > > throwing
> > > > > > > > unchecked
> > > > > > > > > >> > > > > exceptions in Java Collection API and propose the
> > > > following
> > > > > > > > > >> changes in
> > > > > > > > > >> > > > > state-related exceptions:
> > > > > > > > > >> > > > > 1. Introduction of specific unchecked exception
> > > > types for
> > > > > > > > > >> different
> > > > > > > > > >> > > > > reasons, providing users with more precise
> > > > information about
> > > > > > > > the
> > > > > > > > > >> cause
> > > > > > > > > >> > > > > of the exception.
> > > > > > > > > >> > > > > 2. Removal of all checked exceptions from
> > interface
> > > > > > > > signatures
> > > > > > > > > >> and
> > > > > > > > > >> > > > > instead, throwing newly introduced unchecked
> > > > exceptions in the
> > > > > > > > > >> > > > > implementations.
> > > > > > > > > >> > > > >
> > > > > > > > > >> > > > > Please share your thoughts and suggestions
> > regarding
> > > > the
> > > > > > > > proposed
> > > > > > > > > >> > > > > changes. Thank you for your attention and support.
> > > > > > > > > >> > > > >
> > > > > > > > > >> > > > >
> > > > > > > > > >> > > > > Best,
> > > > > > > > > >> > > > > Zakelly
> > > > > > > > > >> > > > >
> > > > > > > > > >> > > > > [1] FLIP-368: Reorganize the exceptions thrown in
> > > > state
> > > > > > > > > >> interfaces,
> > > > > > > > > >> > > > > https://cwiki.apache.org/confluence/x/eZ2zDw
> > > > > > > > > >> > > > >
> > > > > > > > > >> > > >
> > > > > > > > > >> > > >
> > > > > > > > > >> > > > --
> > > > > > > > > >> > > > Best,
> > > > > > > > > >> > > > Hangxiang.
> > > > > > > > > >>
> > > > > > > > > >
> > > > > > > >
> > > > >
> > > > > Unless otherwise stated above:
> > > > >
> > > > > IBM United Kingdom Limited
> > > > > Registered in England and Wales with number 741598
> > > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6
> > 3AU
> > > > >
> > > > > Unless otherwise stated above:
> > > > >
> > > > > IBM United Kingdom Limited
> > > > > Registered in England and Wales with number 741598
> > > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6
> > 3AU
> > > >
> > > > Unless otherwise stated above:
> > > >
> > > > IBM United Kingdom Limited
> > > > Registered in England and Wales with number 741598
> > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
> > > >
> >
>
> Unless otherwise stated above:
>
> IBM United Kingdom Limited
> Registered in England and Wales with number 741598
> Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
RE: FW: RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Posted by David Radley <da...@uk.ibm.com>.
Hi,
It seems that migrating to the next dot version and finding you need to recompile, would be frustrating and couse unexpected work, as I suspect a jar file using the old API will give an exception around not finding the method – I am not sure how this would surface in typical applications at runtime.
Technically because this is tagged as @PublicEvolving then we can make this breaking change.
So there will be migration issues if people are using this API, have we an idea on how many of our users are using it?
If we use @PublicEvolving then maybe we should have stable binaries that only include public APIs and then another bleeding edge package containing @PublicEvolving content, so users can choose.
Organisations I have worked with would not tend to want to or expect to have to recompile their applications on a dot version – as this would normally mean a lot more testing for them.
On balance, as I am risk averse, I would suggest delaying this to v2 as Jing has proposed. This is a cleaner API, is there a demand for this in a dot version? If the community think this is too risk averse, then we could go with 1.19.
WDYT?
Kind regards, David.
From: Jing Ge <ji...@ververica.com.INVALID>
Date: Friday, 13 October 2023 at 14:30
To: dev@flink.apache.org <de...@flink.apache.org>
Subject: [EXTERNAL] Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
HI Zakelly,
What about the jobs that catch those exceptions? Will these downstream
callers that expect this exception to be thrown break, since the exceptions
are part of the method's contract?
Best regards,
Jing
On Fri, Oct 13, 2023 at 11:01 AM Zakelly Lan <za...@gmail.com> wrote:
> Hi Yuan,
>
> Thanks for sharing your thoughts!
>
> In Flink 2.0 I believe we could design brand-new state APIs, which is
> uncertain and may take some time. However, this proposal primarily
> focuses on improving the consistency of interface definitions and
> enhancing the user experience in error handling for the current APIs.
> Therefore, I would prefer to make it in version 1.19. Furthermore, the
> impact of this API change can be controlled since most Flink users do
> not actively catch these exceptions. For them, a simple code recompile
> is sufficient and acceptable when migrating to a new minor release.
> The change is not that big that we need a major version to apply.
>
>
> Best,
> Zakelly
>
> On Fri, Oct 13, 2023 at 3:03 PM Yuan Mei <yu...@gmail.com> wrote:
> >
> > +1 for the proposal
> >
> > But "Since the signature of the public state API has been changed", I was
> > wondering whether this would be more fittable in Flink 2.0, instead of
> 1.19?
> >
> > WDYT?
> >
> > Best
> > Yuan
> >
> > On Wed, Oct 11, 2023 at 4:34 PM David Radley <da...@uk.ibm.com>
> > wrote:
> >
> > > Hi Zakelly,
> > > Thanks for making this clear for me. We should document the impact on
> the
> > > user in the release notes, which will be a minimal rewrite and
> recompile of
> > > any java using the old APIs.
> > > I think it is a good point you make about if there are future
> > > implementations that are
> > > worth retrying (such as network access) – then there could be retries.
> I
> > > agree we should not be trying to create code now for an implementation
> > > consideration that is not there yet,
> > >
> > > +1 from me ,
> > > Kind regards, David.
> > >
> > > From: Zakelly Lan <za...@gmail.com>
> > > Date: Wednesday, 11 October 2023 at 04:25
> > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > Subject: [EXTERNAL] Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the
> > > exceptions thrown in state interfaces
> > > Hi David,
> > >
> > > Thanks for your response.
> > >
> > > The exceptions thrown by state interfaces are NOT retriable. For
> > > example, there may be some elements sent to the wrong subtask due to a
> > > non-deterministic hashCode() algorithm and the key group is not
> > > matching. Or the rocksdb may fail to read a file if it has been
> > > deleted by the user. If there are future implementations that are
> > > worth retrying (such as network access), it would be better to let the
> > > implementation itself handle the retries and provide a configuration
> > > for this, rather than requiring users to catch these exceptions.
> > >
> > > Regarding the release and documentation, I have mentioned that this
> > > change is targeted for version 1.19 with proper documentation. You may
> > > have noticed that state interfaces are annotated with @PublicEvolving,
> > > which means these interfaces may change across versions. The changes
> > > are suitable for a minor release (1.18.0 currently to 1.19.0 in the
> > > future) as defined by the API compatibility guarantees of Flink[1].
> > >
> > >
> > >
> > > Best,
> > > Zakelly
> > >
> > >
> > > [1]
> > >
> https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees
> > >
> > > On Tue, Oct 10, 2023 at 6:19 PM David Radley <da...@uk.ibm.com>
> > > wrote:
> > > >
> > > > Hi,
> > > > I notice
> > >
> https://nightlies.apache.org/flink/flink-docs-master/api/java/org/apache/flink/api/common/state/ValueState.html
> > > is an external API. I am concerned that this change will break existing
> > > applications using the old interface, they are likely to have catches /
> > > throws around the existing checked Exceptions.
> > > >
> > > > If we go with RunTimeException, I would suggest that this sort of
> > > breaking change should be done on a Flink version change, where it is
> > > appropriate to make breaking changes to the API with associated
> > > documentation.
> > > >
> > > > If we want this change on a minor release, we could create a new
> class
> > > ValueState2– that is used internally with the cleaned up Exceptions,
> but
> > > still expose the old class and Exceptions for existing external
> > > applications. I guess new applications could use the new ValueState2 .
> > > >
> > > > What do you think?
> > > > Kind regards, David.
> > > >
> > > >
> > > > From: David Radley <da...@uk.ibm.com>
> > > > Date: Tuesday, 10 October 2023 at 09:49
> > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > Subject: [EXTERNAL] RE: [DISCUSS] FLIP-368 Reorganize the exceptions
> > > thrown in state interfaces
> > > > Hi ,
> > > > The argument seems to be that the errors cannot be acted on so
> should be
> > > runtime exceptions. I want to confirm that none of these errors could /
> > > should be retriable. If there is a possibility that the state is
> available
> > > at some time later then I assume a checked retriable Exception would be
> > > appropriate for those cases; and be part of the contract with the
> caller.
> > > Can we be sure that there is no possibility that the state will become
> > > available; if so then I agree that a runtime Exception is appropriate.
> What
> > > do you think?
> > > >
> > > >
> > > >
> > > > Kind regards, David.
> > > >
> > > >
> > > > From: Zakelly Lan <za...@gmail.com>
> > > > Date: Monday, 9 October 2023 at 18:12
> > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > Subject: [EXTERNAL] Re: [DISCUSS] FLIP-368 Reorganize the exceptions
> > > thrown in state interfaces
> > > > Hi everyone,
> > > >
> > > > It seems we're gradually reaching a consensus. So I would like to
> > > > start a vote after 72 hours if there are no further discussions.
> > > >
> > > > Please let me know if you have any concerns, thanks!
> > > >
> > > >
> > > > Best,
> > > > Zakelly
> > > >
> > > >
> > > > On Sat, Oct 7, 2023 at 4:07 PM Zakelly Lan <za...@gmail.com>
> > > wrote:
> > > > >
> > > > > Hi Jing,
> > > > >
> > > > > Sorry for the late reply! I agree with you that we do not expect
> users
> > > > > to do anything with Flink and we won't "bother" them with those
> > > > > exceptions. However, users can still catch the `Throwable` and
> perform
> > > > > any necessary logging activities, similar to how they use Java
> > > > > Collection interfaces.
> > > > >
> > > > >
> > > > > Thanks for your insights!
> > > > >
> > > > > Best,
> > > > > Zakelly
> > > > >
> > > > > On Thu, Sep 21, 2023 at 8:43 PM Jing Ge <jing@ververica.com.invalid
> >
> > > wrote:
> > > > > >
> > > > > > Fair enough! Thanks Zakelly for the information. Afaic, even
> users
> > > can do
> > > > > > nothing with Flink, they still can do something in their
> territory,
> > > at
> > > > > > least doing some logging and metrics stuff, or triggering some
> other
> > > > > > services in their ecosystem. After all, the Flink jobs they build
> > > are part
> > > > > > of their service component. It didn't change the fact that we are
> > > going to
> > > > > > use the anti-pattern. Just because we didn't expect users to do
> > > > > > anything with Flink, does not mean users don't expect to do
> > > something with
> > > > > > the expected exception. Anyway, I am open to hearing different
> > > opinions.
> > > > > >
> > > > > > Best regards,
> > > > > > Jing
> > > > > >
> > > > > > On Thu, Sep 21, 2023 at 7:02 AM Zakelly Lan <
> zakelly.lan@gmail.com>
> > > wrote:
> > > > > >
> > > > > > > Hi Martijn,
> > > > > > >
> > > > > > > Thanks for the reminder!
> > > > > > >
> > > > > > > This FLIP proposes a change to the state API that is annotated
> as
> > > > > > > @PublicEvolving and targets version 1.19. I have clarified
> this in
> > > > > > > the "Proposed Change" section of the FLIP.
> > > > > > >
> > > > > > >
> > > > > > > Hi Jing,
> > > > > > >
> > > > > > > Thanks for sharing your thoughts! Here are my opinions:
> > > > > > >
> > > > > > > 1. The exceptions of the state API are usually treated as
> critical
> > > > > > > ones. In other words, if anything goes wrong with state
> accessing,
> > > the
> > > > > > > element processing cannot proceed and the job should fail.
> Flink
> > > users
> > > > > > > may not know what to do when they encounter these exceptions. I
> > > > > > > believe this is the main reason why we want to replace them
> with
> > > > > > > unchecked exceptions.
> > > > > > > 2. There have also been some further discussions[1][2] from
> Stephan
> > > > > > > and Shixiaogang below the one you pointed out [3], and it seems
> > > they
> > > > > > > come to an agreement to use unchecked exceptions. After
> reviewing
> > > the
> > > > > > > entire discussion on that PR, I think their arguments are
> > > reasonable
> > > > > > > given the use case.
> > > > > > >
> > > > > > > Looking forward to your feedback.
> > > > > > >
> > > > > > >
> > > > > > > Best,
> > > > > > > Zakelly
> > > > > > >
> > > > > > > [1]
> > > https://github.com/apache/flink/pull/3380#issuecomment-286807853
> > > > > > > [2]
> > > https://github.com/apache/flink/pull/3380#issuecomment-286932133
> > > > > > > [3]
> > > https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > > > >
> > > > > > > On Thu, Sep 21, 2023 at 1:27 AM Jing Ge
> <jing@ververica.com.invalid
> > > >
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > sorry, typo: It is a known "anti-pattern" instead of
> > > "ant-pattern"
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > > Jing
> > > > > > > >
> > > > > > > > On Wed, Sep 20, 2023 at 7:23 PM Jing Ge <ji...@ververica.com>
> > > wrote:
> > > > > > > >
> > > > > > > > > Hi Zakelly,
> > > > > > > > >
> > > > > > > > > Thanks for driving this topic. From good software
> engineering's
> > > > > > > > > perspective, I have different thoughts:
> > > > > > > > >
> > > > > > > > > 1. The idea to get rid of all checked Exceptions and
> replace
> > > them with
> > > > > > > > > unchecked Exceptions is a known ant-pattern: "Generally
> > > speaking, do
> > > > > > > not
> > > > > > > > > throw a RuntimeException or create a subclass of
> > > RuntimeException
> > > > > > > simply
> > > > > > > > > because you don't want to be bothered with specifying the
> > > exceptions
> > > > > > > your
> > > > > > > > > methods can throw." [1] Checked Exceptions mean expected
> > > exceptions
> > > > > > > that
> > > > > > > > > can help developers find a way to catch them and decide
> what
> > > to do. It
> > > > > > > is
> > > > > > > > > part of the public API signature that can help developers
> > > build robust
> > > > > > > > > systems. We should not mix concepts and build expected
> > > exceptions with
> > > > > > > > > unchecked Java Exception classes.
> > > > > > > > > 2. The comment Stephan left [2] clearly pointed out that we
> > > should
> > > > > > > avoid
> > > > > > > > > using generic Java Exceptions, and "find some more
> 'specific'
> > > > > > > exceptions
> > > > > > > > > for the signature, like throws IOException or throws
> > > > > > > StateAccessException."
> > > > > > > > > So, the idea is to define/use specific checked Exception
> > > classes
> > > > > > > instead of
> > > > > > > > > using unchecked Exceptions.
> > > > > > > > >
> > > > > > > > > Looking forward to your thoughts.
> > > > > > > > >
> > > > > > > > > Best regards,
> > > > > > > > > Jing
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > [1]
> > > > > > > > >
> > > > > > >
> > >
> https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
> > > > > > > > > [2]
> > > https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > > > > > >
> > > > > > > > > On Wed, Sep 20, 2023 at 4:52 PM Zakelly Lan <
> > > zakelly.lan@gmail.com>
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > >> Hi Yanfei,
> > > > > > > > >>
> > > > > > > > >> Thanks for your reply!
> > > > > > > > >>
> > > > > > > > >> Yes, this FLIP aims to change all state-related
> exceptions to
> > > > > > > > >> unchecked exceptions and remove all exceptions from the
> > > signature. So
> > > > > > > > >> I believe we have come to an agreement to keep the
> interfaces
> > > simple.
> > > > > > > > >>
> > > > > > > > >>
> > > > > > > > >> Best regards,
> > > > > > > > >> Zakelly
> > > > > > > > >>
> > > > > > > > >> On Wed, Sep 20, 2023 at 2:26 PM Zakelly Lan <
> > > zakelly.lan@gmail.com>
> > > > > > > > >> wrote:
> > > > > > > > >> >
> > > > > > > > >> > Hi Hangxiang,
> > > > > > > > >> >
> > > > > > > > >> > Thank you for your response! Here are my thoughts:
> > > > > > > > >> >
> > > > > > > > >> > 1. Regarding the exceptions thrown by internal
> interfaces,
> > > I suggest
> > > > > > > > >> > keeping them as checked exceptions. Since these
> exceptions
> > > will be
> > > > > > > > >> > handled by the internal callers, it is meaningful to
> throw
> > > them as
> > > > > > > > >> > checked ones. If we need to make changes to these
> classes,
> > > we can
> > > > > > > > >> > create separate tickets alongside this FLIP. What are
> your
> > > thoughts
> > > > > > > on
> > > > > > > > >> > this?
> > > > > > > > >> > 2. StateIOException is primarily thrown by file-based
> state
> > > like
> > > > > > > > >> > RocksDB, while StateAccessException is more generic and
> can
> > > be
> > > > > > > thrown
> > > > > > > > >> > by heap states. Additionally, I believe there may be
> more
> > > subclasses
> > > > > > > > >> > of StateAccessException that we can add. We can consider
> > > this when
> > > > > > > > >> > implementing.
> > > > > > > > >> > 3. I would like to make this change in version 1.19. As
> > > mentioned in
> > > > > > > > >> > this FLIP, many users do not catch any exceptions since
> the
> > > element
> > > > > > > > >> > processing function exposes the exception to the upper
> > > layer.
> > > > > > > > >> > Therefore, the impact is manageable. And I completely
> agree
> > > that we
> > > > > > > > >> > should clearly document this change in the next release
> > > notes.
> > > > > > > > >> >
> > > > > > > > >> >
> > > > > > > > >> > Best regards,
> > > > > > > > >> > Zakelly
> > > > > > > > >> >
> > > > > > > > >> > On Wed, Sep 20, 2023 at 12:35 PM Yanfei Lei <
> > > fredialei@gmail.com>
> > > > > > > > >> wrote:
> > > > > > > > >> > >
> > > > > > > > >> > > Hi Zakelly,
> > > > > > > > >> > >
> > > > > > > > >> > > Thanks for bringing this up. +1 for reorganizing.
> > > > > > > > >> > >
> > > > > > > > >> > > IIUC, this proposal aims to change all state-related
> > > exceptions to
> > > > > > > > >> > > unchecked exceptions. If users have caught checked
> > > exceptions
> > > > > > > (such as
> > > > > > > > >> > > IOException ) in their code, leaving the code as is
> would
> > > also
> > > > > > > work.
> > > > > > > > >> > >
> > > > > > > > >> > > Is it possible not to put any exceptions in the
> signature
> > > of
> > > > > > > > >> > > user-facing interfaces? As the proposal mentioned,
> users
> > > can do a
> > > > > > > few
> > > > > > > > >> > > things even if they catch the exceptions. Keeping the
> > > interface
> > > > > > > simple
> > > > > > > > >> > > may be easier to understand.
> > > > > > > > >> > >
> > > > > > > > >> > >
> > > > > > > > >> > > Best,
> > > > > > > > >> > > Yanfei
> > > > > > > > >> > >
> > > > > > > > >> > > Hangxiang Yu <ma...@gmail.com> 于2023年9月19日周二
> 18:16写道：
> > > > > > > > >> > > >
> > > > > > > > >> > > > Hi, Zakelly.
> > > > > > > > >> > > >
> > > > > > > > >> > > > Thanks for the proposal.
> > > > > > > > >> > > >
> > > > > > > > >> > > > +1 for reorganizing exceptions of state interfaces
> > > which indeed
> > > > > > > > >> confuses me
> > > > > > > > >> > > > currently.
> > > > > > > > >> > > >
> > > > > > > > >> > > > From my experience, users usually omit these
> exceptions
> > > because
> > > > > > > > >> they cannot
> > > > > > > > >> > > > do much even if they catch the exceptions.
> > > > > > > > >> > > >
> > > > > > > > >> > > > I have some problems and suggestions, PTAL:
> > > > > > > > >> > > >
> > > > > > > > >> > > > 1. Could we also reorganize or add more state
> > > exceptions
> > > > > > > (may be
> > > > > > > > >> related
> > > > > > > > >> > > > to other state interfaces/classes e.g.
> > > KeyedStateBackend)
> > > > > > > into
> > > > > > > > >> the
> > > > > > > > >> > > > exception class diagrams ? Although these
> > > state-related
> > > > > > > classes
> > > > > > > > >> may not
> > > > > > > > >> > > > be public, it could be better to consider them
> > > together to
> > > > > > > make
> > > > > > > > >> all
> > > > > > > > >> > > > state-related exceptions clear. For example, we
> could
> > > > > > > reorganize
> > > > > > > > >> some
> > > > > > > > >> > > > existing exceptions such as
> StateMigrationException,
> > > add some
> > > > > > > > >> exceptions
> > > > > > > > >> > > > such as StateNotFoundException.
> > > > > > > > >> > > > 2. Could you clarify or give an example about the
> > > extended
> > > > > > > > >> relation
> > > > > > > > >> > > > "StateAccessException -- StateIOException" ?
> When do
> > > we just
> > > > > > > > >> return
> > > > > > > > >> > > > StateAccessException instead of StateIOException
> or
> > > others ?
> > > > > > > > >> > > > 3. Which version do you want to implement it in ?
> > > Since it
> > > > > > > has
> > > > > > > > >> to break
> > > > > > > > >> > > > changes for users who have catched the
> IOException,
> > > If we
> > > > > > > want
> > > > > > > > >> to implement
> > > > > > > > >> > > > it in 1.19, we must mark it very clearly in the
> > > release
> > > > > > > note, or
> > > > > > > > >> we should
> > > > > > > > >> > > > make it in 2.0.
> > > > > > > > >> > > >
> > > > > > > > >> > > >
> > > > > > > > >> > > > On Tue, Sep 19, 2023 at 5:08 PM Zakelly Lan <
> > > > > > > zakelly.lan@gmail.com>
> > > > > > > > >> wrote:
> > > > > > > > >> > > >
> > > > > > > > >> > > > > Hi everyone,
> > > > > > > > >> > > > >
> > > > > > > > >> > > > > I would like to initiate a discussion on FLIP-368,
> > > which
> > > > > > > focuses
> > > > > > > > >> on
> > > > > > > > >> > > > > reorganizing the exceptions thrown in state
> > > interfaces [1].
> > > > > > > > >> > > > >
> > > > > > > > >> > > > > Currently, we have identified several problems
> with
> > > the
> > > > > > > exceptions
> > > > > > > > >> > > > > thrown by state-related interfaces:
> > > > > > > > >> > > > > 1. The exception types thrown by each interface
> are
> > > > > > > > >> inconsistent.
> > > > > > > > >> > > > > While most of the interfaces claim to throw
> > > `Exception`, the
> > > > > > > > >> > > > > interfaces of `ValueState` throw `IOException`.
> > > Additionally,
> > > > > > > the
> > > > > > > > >> > > > > `State#clear()` never throws an exception. This
> can be
> > > > > > > confusing
> > > > > > > > >> for
> > > > > > > > >> > > > > users.
> > > > > > > > >> > > > > 2. The use of `Exception` or `IOException` as
> the
> > > thrown
> > > > > > > > >> exception
> > > > > > > > >> > > > > type is too generic and lacks specificity.
> > > > > > > > >> > > > > 3. Users may not be able to handle these
> > > exceptions. In
> > > > > > > cases
> > > > > > > > >> where
> > > > > > > > >> > > > > an exception occurs while accessing state, the job
> > > should
> > > > > > > fail.
> > > > > > > > >> This
> > > > > > > > >> > > > > aligns more with the characteristic of *unchecked
> > > exceptions*
> > > > > > > > >> instead
> > > > > > > > >> > > > > of checked exceptions.
> > > > > > > > >> > > > >
> > > > > > > > >> > > > > To address these issues, we borrow the idea of
> > > throwing
> > > > > > > unchecked
> > > > > > > > >> > > > > exceptions in Java Collection API and propose the
> > > following
> > > > > > > > >> changes in
> > > > > > > > >> > > > > state-related exceptions:
> > > > > > > > >> > > > > 1. Introduction of specific unchecked exception
> > > types for
> > > > > > > > >> different
> > > > > > > > >> > > > > reasons, providing users with more precise
> > > information about
> > > > > > > the
> > > > > > > > >> cause
> > > > > > > > >> > > > > of the exception.
> > > > > > > > >> > > > > 2. Removal of all checked exceptions from
> interface
> > > > > > > signatures
> > > > > > > > >> and
> > > > > > > > >> > > > > instead, throwing newly introduced unchecked
> > > exceptions in the
> > > > > > > > >> > > > > implementations.
> > > > > > > > >> > > > >
> > > > > > > > >> > > > > Please share your thoughts and suggestions
> regarding
> > > the
> > > > > > > proposed
> > > > > > > > >> > > > > changes. Thank you for your attention and support.
> > > > > > > > >> > > > >
> > > > > > > > >> > > > >
> > > > > > > > >> > > > > Best,
> > > > > > > > >> > > > > Zakelly
> > > > > > > > >> > > > >
> > > > > > > > >> > > > > [1] FLIP-368: Reorganize the exceptions thrown in
> > > state
> > > > > > > > >> interfaces,
> > > > > > > > >> > > > > https://cwiki.apache.org/confluence/x/eZ2zDw
> > > > > > > > >> > > > >
> > > > > > > > >> > > >
> > > > > > > > >> > > >
> > > > > > > > >> > > > --
> > > > > > > > >> > > > Best,
> > > > > > > > >> > > > Hangxiang.
> > > > > > > > >>
> > > > > > > > >
> > > > > > >
> > > >
> > > > Unless otherwise stated above:
> > > >
> > > > IBM United Kingdom Limited
> > > > Registered in England and Wales with number 741598
> > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6
> 3AU
> > > >
> > > > Unless otherwise stated above:
> > > >
> > > > IBM United Kingdom Limited
> > > > Registered in England and Wales with number 741598
> > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6
> 3AU
> > >
> > > Unless otherwise stated above:
> > >
> > > IBM United Kingdom Limited
> > > Registered in England and Wales with number 741598
> > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
> > >
>
Unless otherwise stated above:
IBM United Kingdom Limited
Registered in England and Wales with number 741598
Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Posted by Jing Ge <ji...@ververica.com.INVALID>.
HI Zakelly,
What about the jobs that catch those exceptions? Will these downstream
callers that expect this exception to be thrown break, since the exceptions
are part of the method's contract?
Best regards,
Jing
On Fri, Oct 13, 2023 at 11:01 AM Zakelly Lan <za...@gmail.com> wrote:
> Hi Yuan,
>
> Thanks for sharing your thoughts!
>
> In Flink 2.0 I believe we could design brand-new state APIs, which is
> uncertain and may take some time. However, this proposal primarily
> focuses on improving the consistency of interface definitions and
> enhancing the user experience in error handling for the current APIs.
> Therefore, I would prefer to make it in version 1.19. Furthermore, the
> impact of this API change can be controlled since most Flink users do
> not actively catch these exceptions. For them, a simple code recompile
> is sufficient and acceptable when migrating to a new minor release.
> The change is not that big that we need a major version to apply.
>
>
> Best,
> Zakelly
>
> On Fri, Oct 13, 2023 at 3:03 PM Yuan Mei <yu...@gmail.com> wrote:
> >
> > +1 for the proposal
> >
> > But "Since the signature of the public state API has been changed", I was
> > wondering whether this would be more fittable in Flink 2.0, instead of
> 1.19?
> >
> > WDYT?
> >
> > Best
> > Yuan
> >
> > On Wed, Oct 11, 2023 at 4:34 PM David Radley <da...@uk.ibm.com>
> > wrote:
> >
> > > Hi Zakelly,
> > > Thanks for making this clear for me. We should document the impact on
> the
> > > user in the release notes, which will be a minimal rewrite and
> recompile of
> > > any java using the old APIs.
> > > I think it is a good point you make about if there are future
> > > implementations that are
> > > worth retrying (such as network access) – then there could be retries.
> I
> > > agree we should not be trying to create code now for an implementation
> > > consideration that is not there yet,
> > >
> > > +1 from me ,
> > > Kind regards, David.
> > >
> > > From: Zakelly Lan <za...@gmail.com>
> > > Date: Wednesday, 11 October 2023 at 04:25
> > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > Subject: [EXTERNAL] Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the
> > > exceptions thrown in state interfaces
> > > Hi David,
> > >
> > > Thanks for your response.
> > >
> > > The exceptions thrown by state interfaces are NOT retriable. For
> > > example, there may be some elements sent to the wrong subtask due to a
> > > non-deterministic hashCode() algorithm and the key group is not
> > > matching. Or the rocksdb may fail to read a file if it has been
> > > deleted by the user. If there are future implementations that are
> > > worth retrying (such as network access), it would be better to let the
> > > implementation itself handle the retries and provide a configuration
> > > for this, rather than requiring users to catch these exceptions.
> > >
> > > Regarding the release and documentation, I have mentioned that this
> > > change is targeted for version 1.19 with proper documentation. You may
> > > have noticed that state interfaces are annotated with @PublicEvolving,
> > > which means these interfaces may change across versions. The changes
> > > are suitable for a minor release (1.18.0 currently to 1.19.0 in the
> > > future) as defined by the API compatibility guarantees of Flink[1].
> > >
> > >
> > >
> > > Best,
> > > Zakelly
> > >
> > >
> > > [1]
> > >
> https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees
> > >
> > > On Tue, Oct 10, 2023 at 6:19 PM David Radley <da...@uk.ibm.com>
> > > wrote:
> > > >
> > > > Hi,
> > > > I notice
> > >
> https://nightlies.apache.org/flink/flink-docs-master/api/java/org/apache/flink/api/common/state/ValueState.html
> > > is an external API. I am concerned that this change will break existing
> > > applications using the old interface, they are likely to have catches /
> > > throws around the existing checked Exceptions.
> > > >
> > > > If we go with RunTimeException, I would suggest that this sort of
> > > breaking change should be done on a Flink version change, where it is
> > > appropriate to make breaking changes to the API with associated
> > > documentation.
> > > >
> > > > If we want this change on a minor release, we could create a new
> class
> > > ValueState2– that is used internally with the cleaned up Exceptions,
> but
> > > still expose the old class and Exceptions for existing external
> > > applications. I guess new applications could use the new ValueState2 .
> > > >
> > > > What do you think?
> > > > Kind regards, David.
> > > >
> > > >
> > > > From: David Radley <da...@uk.ibm.com>
> > > > Date: Tuesday, 10 October 2023 at 09:49
> > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > Subject: [EXTERNAL] RE: [DISCUSS] FLIP-368 Reorganize the exceptions
> > > thrown in state interfaces
> > > > Hi ,
> > > > The argument seems to be that the errors cannot be acted on so
> should be
> > > runtime exceptions. I want to confirm that none of these errors could /
> > > should be retriable. If there is a possibility that the state is
> available
> > > at some time later then I assume a checked retriable Exception would be
> > > appropriate for those cases; and be part of the contract with the
> caller.
> > > Can we be sure that there is no possibility that the state will become
> > > available; if so then I agree that a runtime Exception is appropriate.
> What
> > > do you think?
> > > >
> > > >
> > > >
> > > > Kind regards, David.
> > > >
> > > >
> > > > From: Zakelly Lan <za...@gmail.com>
> > > > Date: Monday, 9 October 2023 at 18:12
> > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > Subject: [EXTERNAL] Re: [DISCUSS] FLIP-368 Reorganize the exceptions
> > > thrown in state interfaces
> > > > Hi everyone,
> > > >
> > > > It seems we're gradually reaching a consensus. So I would like to
> > > > start a vote after 72 hours if there are no further discussions.
> > > >
> > > > Please let me know if you have any concerns, thanks!
> > > >
> > > >
> > > > Best,
> > > > Zakelly
> > > >
> > > >
> > > > On Sat, Oct 7, 2023 at 4:07 PM Zakelly Lan <za...@gmail.com>
> > > wrote:
> > > > >
> > > > > Hi Jing,
> > > > >
> > > > > Sorry for the late reply! I agree with you that we do not expect
> users
> > > > > to do anything with Flink and we won't "bother" them with those
> > > > > exceptions. However, users can still catch the `Throwable` and
> perform
> > > > > any necessary logging activities, similar to how they use Java
> > > > > Collection interfaces.
> > > > >
> > > > >
> > > > > Thanks for your insights!
> > > > >
> > > > > Best,
> > > > > Zakelly
> > > > >
> > > > > On Thu, Sep 21, 2023 at 8:43 PM Jing Ge <jing@ververica.com.invalid
> >
> > > wrote:
> > > > > >
> > > > > > Fair enough! Thanks Zakelly for the information. Afaic, even
> users
> > > can do
> > > > > > nothing with Flink, they still can do something in their
> territory,
> > > at
> > > > > > least doing some logging and metrics stuff, or triggering some
> other
> > > > > > services in their ecosystem. After all, the Flink jobs they build
> > > are part
> > > > > > of their service component. It didn't change the fact that we are
> > > going to
> > > > > > use the anti-pattern. Just because we didn't expect users to do
> > > > > > anything with Flink, does not mean users don't expect to do
> > > something with
> > > > > > the expected exception. Anyway, I am open to hearing different
> > > opinions.
> > > > > >
> > > > > > Best regards,
> > > > > > Jing
> > > > > >
> > > > > > On Thu, Sep 21, 2023 at 7:02 AM Zakelly Lan <
> zakelly.lan@gmail.com>
> > > wrote:
> > > > > >
> > > > > > > Hi Martijn,
> > > > > > >
> > > > > > > Thanks for the reminder!
> > > > > > >
> > > > > > > This FLIP proposes a change to the state API that is annotated
> as
> > > > > > > @PublicEvolving and targets version 1.19. I have clarified
> this in
> > > > > > > the "Proposed Change" section of the FLIP.
> > > > > > >
> > > > > > >
> > > > > > > Hi Jing,
> > > > > > >
> > > > > > > Thanks for sharing your thoughts! Here are my opinions:
> > > > > > >
> > > > > > > 1. The exceptions of the state API are usually treated as
> critical
> > > > > > > ones. In other words, if anything goes wrong with state
> accessing,
> > > the
> > > > > > > element processing cannot proceed and the job should fail.
> Flink
> > > users
> > > > > > > may not know what to do when they encounter these exceptions. I
> > > > > > > believe this is the main reason why we want to replace them
> with
> > > > > > > unchecked exceptions.
> > > > > > > 2. There have also been some further discussions[1][2] from
> Stephan
> > > > > > > and Shixiaogang below the one you pointed out [3], and it seems
> > > they
> > > > > > > come to an agreement to use unchecked exceptions. After
> reviewing
> > > the
> > > > > > > entire discussion on that PR, I think their arguments are
> > > reasonable
> > > > > > > given the use case.
> > > > > > >
> > > > > > > Looking forward to your feedback.
> > > > > > >
> > > > > > >
> > > > > > > Best,
> > > > > > > Zakelly
> > > > > > >
> > > > > > > [1]
> > > https://github.com/apache/flink/pull/3380#issuecomment-286807853
> > > > > > > [2]
> > > https://github.com/apache/flink/pull/3380#issuecomment-286932133
> > > > > > > [3]
> > > https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > > > >
> > > > > > > On Thu, Sep 21, 2023 at 1:27 AM Jing Ge
> <jing@ververica.com.invalid
> > > >
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > sorry, typo: It is a known "anti-pattern" instead of
> > > "ant-pattern"
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > > Jing
> > > > > > > >
> > > > > > > > On Wed, Sep 20, 2023 at 7:23 PM Jing Ge <ji...@ververica.com>
> > > wrote:
> > > > > > > >
> > > > > > > > > Hi Zakelly,
> > > > > > > > >
> > > > > > > > > Thanks for driving this topic. From good software
> engineering's
> > > > > > > > > perspective, I have different thoughts:
> > > > > > > > >
> > > > > > > > > 1. The idea to get rid of all checked Exceptions and
> replace
> > > them with
> > > > > > > > > unchecked Exceptions is a known ant-pattern: "Generally
> > > speaking, do
> > > > > > > not
> > > > > > > > > throw a RuntimeException or create a subclass of
> > > RuntimeException
> > > > > > > simply
> > > > > > > > > because you don't want to be bothered with specifying the
> > > exceptions
> > > > > > > your
> > > > > > > > > methods can throw." [1] Checked Exceptions mean expected
> > > exceptions
> > > > > > > that
> > > > > > > > > can help developers find a way to catch them and decide
> what
> > > to do. It
> > > > > > > is
> > > > > > > > > part of the public API signature that can help developers
> > > build robust
> > > > > > > > > systems. We should not mix concepts and build expected
> > > exceptions with
> > > > > > > > > unchecked Java Exception classes.
> > > > > > > > > 2. The comment Stephan left [2] clearly pointed out that we
> > > should
> > > > > > > avoid
> > > > > > > > > using generic Java Exceptions, and "find some more
> 'specific'
> > > > > > > exceptions
> > > > > > > > > for the signature, like throws IOException or throws
> > > > > > > StateAccessException."
> > > > > > > > > So, the idea is to define/use specific checked Exception
> > > classes
> > > > > > > instead of
> > > > > > > > > using unchecked Exceptions.
> > > > > > > > >
> > > > > > > > > Looking forward to your thoughts.
> > > > > > > > >
> > > > > > > > > Best regards,
> > > > > > > > > Jing
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > [1]
> > > > > > > > >
> > > > > > >
> > >
> https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
> > > > > > > > > [2]
> > > https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > > > > > >
> > > > > > > > > On Wed, Sep 20, 2023 at 4:52 PM Zakelly Lan <
> > > zakelly.lan@gmail.com>
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > >> Hi Yanfei,
> > > > > > > > >>
> > > > > > > > >> Thanks for your reply!
> > > > > > > > >>
> > > > > > > > >> Yes, this FLIP aims to change all state-related
> exceptions to
> > > > > > > > >> unchecked exceptions and remove all exceptions from the
> > > signature. So
> > > > > > > > >> I believe we have come to an agreement to keep the
> interfaces
> > > simple.
> > > > > > > > >>
> > > > > > > > >>
> > > > > > > > >> Best regards,
> > > > > > > > >> Zakelly
> > > > > > > > >>
> > > > > > > > >> On Wed, Sep 20, 2023 at 2:26 PM Zakelly Lan <
> > > zakelly.lan@gmail.com>
> > > > > > > > >> wrote:
> > > > > > > > >> >
> > > > > > > > >> > Hi Hangxiang,
> > > > > > > > >> >
> > > > > > > > >> > Thank you for your response! Here are my thoughts:
> > > > > > > > >> >
> > > > > > > > >> > 1. Regarding the exceptions thrown by internal
> interfaces,
> > > I suggest
> > > > > > > > >> > keeping them as checked exceptions. Since these
> exceptions
> > > will be
> > > > > > > > >> > handled by the internal callers, it is meaningful to
> throw
> > > them as
> > > > > > > > >> > checked ones. If we need to make changes to these
> classes,
> > > we can
> > > > > > > > >> > create separate tickets alongside this FLIP. What are
> your
> > > thoughts
> > > > > > > on
> > > > > > > > >> > this?
> > > > > > > > >> > 2. StateIOException is primarily thrown by file-based
> state
> > > like
> > > > > > > > >> > RocksDB, while StateAccessException is more generic and
> can
> > > be
> > > > > > > thrown
> > > > > > > > >> > by heap states. Additionally, I believe there may be
> more
> > > subclasses
> > > > > > > > >> > of StateAccessException that we can add. We can consider
> > > this when
> > > > > > > > >> > implementing.
> > > > > > > > >> > 3. I would like to make this change in version 1.19. As
> > > mentioned in
> > > > > > > > >> > this FLIP, many users do not catch any exceptions since
> the
> > > element
> > > > > > > > >> > processing function exposes the exception to the upper
> > > layer.
> > > > > > > > >> > Therefore, the impact is manageable. And I completely
> agree
> > > that we
> > > > > > > > >> > should clearly document this change in the next release
> > > notes.
> > > > > > > > >> >
> > > > > > > > >> >
> > > > > > > > >> > Best regards,
> > > > > > > > >> > Zakelly
> > > > > > > > >> >
> > > > > > > > >> > On Wed, Sep 20, 2023 at 12:35 PM Yanfei Lei <
> > > fredialei@gmail.com>
> > > > > > > > >> wrote:
> > > > > > > > >> > >
> > > > > > > > >> > > Hi Zakelly,
> > > > > > > > >> > >
> > > > > > > > >> > > Thanks for bringing this up. +1 for reorganizing.
> > > > > > > > >> > >
> > > > > > > > >> > > IIUC, this proposal aims to change all state-related
> > > exceptions to
> > > > > > > > >> > > unchecked exceptions. If users have caught checked
> > > exceptions
> > > > > > > (such as
> > > > > > > > >> > > IOException ) in their code, leaving the code as is
> would
> > > also
> > > > > > > work.
> > > > > > > > >> > >
> > > > > > > > >> > > Is it possible not to put any exceptions in the
> signature
> > > of
> > > > > > > > >> > > user-facing interfaces? As the proposal mentioned,
> users
> > > can do a
> > > > > > > few
> > > > > > > > >> > > things even if they catch the exceptions. Keeping the
> > > interface
> > > > > > > simple
> > > > > > > > >> > > may be easier to understand.
> > > > > > > > >> > >
> > > > > > > > >> > >
> > > > > > > > >> > > Best,
> > > > > > > > >> > > Yanfei
> > > > > > > > >> > >
> > > > > > > > >> > > Hangxiang Yu <ma...@gmail.com> 于2023年9月19日周二
> 18:16写道：
> > > > > > > > >> > > >
> > > > > > > > >> > > > Hi, Zakelly.
> > > > > > > > >> > > >
> > > > > > > > >> > > > Thanks for the proposal.
> > > > > > > > >> > > >
> > > > > > > > >> > > > +1 for reorganizing exceptions of state interfaces
> > > which indeed
> > > > > > > > >> confuses me
> > > > > > > > >> > > > currently.
> > > > > > > > >> > > >
> > > > > > > > >> > > > From my experience, users usually omit these
> exceptions
> > > because
> > > > > > > > >> they cannot
> > > > > > > > >> > > > do much even if they catch the exceptions.
> > > > > > > > >> > > >
> > > > > > > > >> > > > I have some problems and suggestions, PTAL:
> > > > > > > > >> > > >
> > > > > > > > >> > > > 1. Could we also reorganize or add more state
> > > exceptions
> > > > > > > (may be
> > > > > > > > >> related
> > > > > > > > >> > > > to other state interfaces/classes e.g.
> > > KeyedStateBackend)
> > > > > > > into
> > > > > > > > >> the
> > > > > > > > >> > > > exception class diagrams ? Although these
> > > state-related
> > > > > > > classes
> > > > > > > > >> may not
> > > > > > > > >> > > > be public, it could be better to consider them
> > > together to
> > > > > > > make
> > > > > > > > >> all
> > > > > > > > >> > > > state-related exceptions clear. For example, we
> could
> > > > > > > reorganize
> > > > > > > > >> some
> > > > > > > > >> > > > existing exceptions such as
> StateMigrationException,
> > > add some
> > > > > > > > >> exceptions
> > > > > > > > >> > > > such as StateNotFoundException.
> > > > > > > > >> > > > 2. Could you clarify or give an example about the
> > > extended
> > > > > > > > >> relation
> > > > > > > > >> > > > "StateAccessException -- StateIOException" ?
> When do
> > > we just
> > > > > > > > >> return
> > > > > > > > >> > > > StateAccessException instead of StateIOException
> or
> > > others ?
> > > > > > > > >> > > > 3. Which version do you want to implement it in ?
> > > Since it
> > > > > > > has
> > > > > > > > >> to break
> > > > > > > > >> > > > changes for users who have catched the
> IOException,
> > > If we
> > > > > > > want
> > > > > > > > >> to implement
> > > > > > > > >> > > > it in 1.19, we must mark it very clearly in the
> > > release
> > > > > > > note, or
> > > > > > > > >> we should
> > > > > > > > >> > > > make it in 2.0.
> > > > > > > > >> > > >
> > > > > > > > >> > > >
> > > > > > > > >> > > > On Tue, Sep 19, 2023 at 5:08 PM Zakelly Lan <
> > > > > > > zakelly.lan@gmail.com>
> > > > > > > > >> wrote:
> > > > > > > > >> > > >
> > > > > > > > >> > > > > Hi everyone,
> > > > > > > > >> > > > >
> > > > > > > > >> > > > > I would like to initiate a discussion on FLIP-368,
> > > which
> > > > > > > focuses
> > > > > > > > >> on
> > > > > > > > >> > > > > reorganizing the exceptions thrown in state
> > > interfaces [1].
> > > > > > > > >> > > > >
> > > > > > > > >> > > > > Currently, we have identified several problems
> with
> > > the
> > > > > > > exceptions
> > > > > > > > >> > > > > thrown by state-related interfaces:
> > > > > > > > >> > > > > 1. The exception types thrown by each interface
> are
> > > > > > > > >> inconsistent.
> > > > > > > > >> > > > > While most of the interfaces claim to throw
> > > `Exception`, the
> > > > > > > > >> > > > > interfaces of `ValueState` throw `IOException`.
> > > Additionally,
> > > > > > > the
> > > > > > > > >> > > > > `State#clear()` never throws an exception. This
> can be
> > > > > > > confusing
> > > > > > > > >> for
> > > > > > > > >> > > > > users.
> > > > > > > > >> > > > > 2. The use of `Exception` or `IOException` as
> the
> > > thrown
> > > > > > > > >> exception
> > > > > > > > >> > > > > type is too generic and lacks specificity.
> > > > > > > > >> > > > > 3. Users may not be able to handle these
> > > exceptions. In
> > > > > > > cases
> > > > > > > > >> where
> > > > > > > > >> > > > > an exception occurs while accessing state, the job
> > > should
> > > > > > > fail.
> > > > > > > > >> This
> > > > > > > > >> > > > > aligns more with the characteristic of *unchecked
> > > exceptions*
> > > > > > > > >> instead
> > > > > > > > >> > > > > of checked exceptions.
> > > > > > > > >> > > > >
> > > > > > > > >> > > > > To address these issues, we borrow the idea of
> > > throwing
> > > > > > > unchecked
> > > > > > > > >> > > > > exceptions in Java Collection API and propose the
> > > following
> > > > > > > > >> changes in
> > > > > > > > >> > > > > state-related exceptions:
> > > > > > > > >> > > > > 1. Introduction of specific unchecked exception
> > > types for
> > > > > > > > >> different
> > > > > > > > >> > > > > reasons, providing users with more precise
> > > information about
> > > > > > > the
> > > > > > > > >> cause
> > > > > > > > >> > > > > of the exception.
> > > > > > > > >> > > > > 2. Removal of all checked exceptions from
> interface
> > > > > > > signatures
> > > > > > > > >> and
> > > > > > > > >> > > > > instead, throwing newly introduced unchecked
> > > exceptions in the
> > > > > > > > >> > > > > implementations.
> > > > > > > > >> > > > >
> > > > > > > > >> > > > > Please share your thoughts and suggestions
> regarding
> > > the
> > > > > > > proposed
> > > > > > > > >> > > > > changes. Thank you for your attention and support.
> > > > > > > > >> > > > >
> > > > > > > > >> > > > >
> > > > > > > > >> > > > > Best,
> > > > > > > > >> > > > > Zakelly
> > > > > > > > >> > > > >
> > > > > > > > >> > > > > [1] FLIP-368: Reorganize the exceptions thrown in
> > > state
> > > > > > > > >> interfaces,
> > > > > > > > >> > > > > https://cwiki.apache.org/confluence/x/eZ2zDw
> > > > > > > > >> > > > >
> > > > > > > > >> > > >
> > > > > > > > >> > > >
> > > > > > > > >> > > > --
> > > > > > > > >> > > > Best,
> > > > > > > > >> > > > Hangxiang.
> > > > > > > > >>
> > > > > > > > >
> > > > > > >
> > > >
> > > > Unless otherwise stated above:
> > > >
> > > > IBM United Kingdom Limited
> > > > Registered in England and Wales with number 741598
> > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6
> 3AU
> > > >
> > > > Unless otherwise stated above:
> > > >
> > > > IBM United Kingdom Limited
> > > > Registered in England and Wales with number 741598
> > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6
> 3AU
> > >
> > > Unless otherwise stated above:
> > >
> > > IBM United Kingdom Limited
> > > Registered in England and Wales with number 741598
> > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
> > >
>
Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Posted by Zakelly Lan <za...@gmail.com>.
Hi Yuan,
Thanks for sharing your thoughts!
In Flink 2.0 I believe we could design brand-new state APIs, which is
uncertain and may take some time. However, this proposal primarily
focuses on improving the consistency of interface definitions and
enhancing the user experience in error handling for the current APIs.
Therefore, I would prefer to make it in version 1.19. Furthermore, the
impact of this API change can be controlled since most Flink users do
not actively catch these exceptions. For them, a simple code recompile
is sufficient and acceptable when migrating to a new minor release.
The change is not that big that we need a major version to apply.
Best,
Zakelly
On Fri, Oct 13, 2023 at 3:03 PM Yuan Mei <yu...@gmail.com> wrote:
>
> +1 for the proposal
>
> But "Since the signature of the public state API has been changed", I was
> wondering whether this would be more fittable in Flink 2.0, instead of 1.19?
>
> WDYT?
>
> Best
> Yuan
>
> On Wed, Oct 11, 2023 at 4:34 PM David Radley <da...@uk.ibm.com>
> wrote:
>
> > Hi Zakelly,
> > Thanks for making this clear for me. We should document the impact on the
> > user in the release notes, which will be a minimal rewrite and recompile of
> > any java using the old APIs.
> > I think it is a good point you make about if there are future
> > implementations that are
> > worth retrying (such as network access) – then there could be retries. I
> > agree we should not be trying to create code now for an implementation
> > consideration that is not there yet,
> >
> > +1 from me ,
> > Kind regards, David.
> >
> > From: Zakelly Lan <za...@gmail.com>
> > Date: Wednesday, 11 October 2023 at 04:25
> > To: dev@flink.apache.org <de...@flink.apache.org>
> > Subject: [EXTERNAL] Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the
> > exceptions thrown in state interfaces
> > Hi David,
> >
> > Thanks for your response.
> >
> > The exceptions thrown by state interfaces are NOT retriable. For
> > example, there may be some elements sent to the wrong subtask due to a
> > non-deterministic hashCode() algorithm and the key group is not
> > matching. Or the rocksdb may fail to read a file if it has been
> > deleted by the user. If there are future implementations that are
> > worth retrying (such as network access), it would be better to let the
> > implementation itself handle the retries and provide a configuration
> > for this, rather than requiring users to catch these exceptions.
> >
> > Regarding the release and documentation, I have mentioned that this
> > change is targeted for version 1.19 with proper documentation. You may
> > have noticed that state interfaces are annotated with @PublicEvolving,
> > which means these interfaces may change across versions. The changes
> > are suitable for a minor release (1.18.0 currently to 1.19.0 in the
> > future) as defined by the API compatibility guarantees of Flink[1].
> >
> >
> >
> > Best,
> > Zakelly
> >
> >
> > [1]
> > https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees
> >
> > On Tue, Oct 10, 2023 at 6:19 PM David Radley <da...@uk.ibm.com>
> > wrote:
> > >
> > > Hi,
> > > I notice
> > https://nightlies.apache.org/flink/flink-docs-master/api/java/org/apache/flink/api/common/state/ValueState.html
> > is an external API. I am concerned that this change will break existing
> > applications using the old interface, they are likely to have catches /
> > throws around the existing checked Exceptions.
> > >
> > > If we go with RunTimeException, I would suggest that this sort of
> > breaking change should be done on a Flink version change, where it is
> > appropriate to make breaking changes to the API with associated
> > documentation.
> > >
> > > If we want this change on a minor release, we could create a new class
> > ValueState2– that is used internally with the cleaned up Exceptions, but
> > still expose the old class and Exceptions for existing external
> > applications. I guess new applications could use the new ValueState2 .
> > >
> > > What do you think?
> > > Kind regards, David.
> > >
> > >
> > > From: David Radley <da...@uk.ibm.com>
> > > Date: Tuesday, 10 October 2023 at 09:49
> > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > Subject: [EXTERNAL] RE: [DISCUSS] FLIP-368 Reorganize the exceptions
> > thrown in state interfaces
> > > Hi ,
> > > The argument seems to be that the errors cannot be acted on so should be
> > runtime exceptions. I want to confirm that none of these errors could /
> > should be retriable. If there is a possibility that the state is available
> > at some time later then I assume a checked retriable Exception would be
> > appropriate for those cases; and be part of the contract with the caller.
> > Can we be sure that there is no possibility that the state will become
> > available; if so then I agree that a runtime Exception is appropriate. What
> > do you think?
> > >
> > >
> > >
> > > Kind regards, David.
> > >
> > >
> > > From: Zakelly Lan <za...@gmail.com>
> > > Date: Monday, 9 October 2023 at 18:12
> > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > Subject: [EXTERNAL] Re: [DISCUSS] FLIP-368 Reorganize the exceptions
> > thrown in state interfaces
> > > Hi everyone,
> > >
> > > It seems we're gradually reaching a consensus. So I would like to
> > > start a vote after 72 hours if there are no further discussions.
> > >
> > > Please let me know if you have any concerns, thanks!
> > >
> > >
> > > Best,
> > > Zakelly
> > >
> > >
> > > On Sat, Oct 7, 2023 at 4:07 PM Zakelly Lan <za...@gmail.com>
> > wrote:
> > > >
> > > > Hi Jing,
> > > >
> > > > Sorry for the late reply! I agree with you that we do not expect users
> > > > to do anything with Flink and we won't "bother" them with those
> > > > exceptions. However, users can still catch the `Throwable` and perform
> > > > any necessary logging activities, similar to how they use Java
> > > > Collection interfaces.
> > > >
> > > >
> > > > Thanks for your insights!
> > > >
> > > > Best,
> > > > Zakelly
> > > >
> > > > On Thu, Sep 21, 2023 at 8:43 PM Jing Ge <ji...@ververica.com.invalid>
> > wrote:
> > > > >
> > > > > Fair enough! Thanks Zakelly for the information. Afaic, even users
> > can do
> > > > > nothing with Flink, they still can do something in their territory,
> > at
> > > > > least doing some logging and metrics stuff, or triggering some other
> > > > > services in their ecosystem. After all, the Flink jobs they build
> > are part
> > > > > of their service component. It didn't change the fact that we are
> > going to
> > > > > use the anti-pattern. Just because we didn't expect users to do
> > > > > anything with Flink, does not mean users don't expect to do
> > something with
> > > > > the expected exception. Anyway, I am open to hearing different
> > opinions.
> > > > >
> > > > > Best regards,
> > > > > Jing
> > > > >
> > > > > On Thu, Sep 21, 2023 at 7:02 AM Zakelly Lan <za...@gmail.com>
> > wrote:
> > > > >
> > > > > > Hi Martijn,
> > > > > >
> > > > > > Thanks for the reminder!
> > > > > >
> > > > > > This FLIP proposes a change to the state API that is annotated as
> > > > > > @PublicEvolving and targets version 1.19. I have clarified this in
> > > > > > the "Proposed Change" section of the FLIP.
> > > > > >
> > > > > >
> > > > > > Hi Jing,
> > > > > >
> > > > > > Thanks for sharing your thoughts! Here are my opinions:
> > > > > >
> > > > > > 1. The exceptions of the state API are usually treated as critical
> > > > > > ones. In other words, if anything goes wrong with state accessing,
> > the
> > > > > > element processing cannot proceed and the job should fail. Flink
> > users
> > > > > > may not know what to do when they encounter these exceptions. I
> > > > > > believe this is the main reason why we want to replace them with
> > > > > > unchecked exceptions.
> > > > > > 2. There have also been some further discussions[1][2] from Stephan
> > > > > > and Shixiaogang below the one you pointed out [3], and it seems
> > they
> > > > > > come to an agreement to use unchecked exceptions. After reviewing
> > the
> > > > > > entire discussion on that PR, I think their arguments are
> > reasonable
> > > > > > given the use case.
> > > > > >
> > > > > > Looking forward to your feedback.
> > > > > >
> > > > > >
> > > > > > Best,
> > > > > > Zakelly
> > > > > >
> > > > > > [1]
> > https://github.com/apache/flink/pull/3380#issuecomment-286807853
> > > > > > [2]
> > https://github.com/apache/flink/pull/3380#issuecomment-286932133
> > > > > > [3]
> > https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > > >
> > > > > > On Thu, Sep 21, 2023 at 1:27 AM Jing Ge <jing@ververica.com.invalid
> > >
> > > > > > wrote:
> > > > > > >
> > > > > > > sorry, typo: It is a known "anti-pattern" instead of
> > "ant-pattern"
> > > > > > >
> > > > > > > Best regards,
> > > > > > > Jing
> > > > > > >
> > > > > > > On Wed, Sep 20, 2023 at 7:23 PM Jing Ge <ji...@ververica.com>
> > wrote:
> > > > > > >
> > > > > > > > Hi Zakelly,
> > > > > > > >
> > > > > > > > Thanks for driving this topic. From good software engineering's
> > > > > > > > perspective, I have different thoughts:
> > > > > > > >
> > > > > > > > 1. The idea to get rid of all checked Exceptions and replace
> > them with
> > > > > > > > unchecked Exceptions is a known ant-pattern: "Generally
> > speaking, do
> > > > > > not
> > > > > > > > throw a RuntimeException or create a subclass of
> > RuntimeException
> > > > > > simply
> > > > > > > > because you don't want to be bothered with specifying the
> > exceptions
> > > > > > your
> > > > > > > > methods can throw." [1] Checked Exceptions mean expected
> > exceptions
> > > > > > that
> > > > > > > > can help developers find a way to catch them and decide what
> > to do. It
> > > > > > is
> > > > > > > > part of the public API signature that can help developers
> > build robust
> > > > > > > > systems. We should not mix concepts and build expected
> > exceptions with
> > > > > > > > unchecked Java Exception classes.
> > > > > > > > 2. The comment Stephan left [2] clearly pointed out that we
> > should
> > > > > > avoid
> > > > > > > > using generic Java Exceptions, and "find some more 'specific'
> > > > > > exceptions
> > > > > > > > for the signature, like throws IOException or throws
> > > > > > StateAccessException."
> > > > > > > > So, the idea is to define/use specific checked Exception
> > classes
> > > > > > instead of
> > > > > > > > using unchecked Exceptions.
> > > > > > > >
> > > > > > > > Looking forward to your thoughts.
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > > Jing
> > > > > > > >
> > > > > > > >
> > > > > > > > [1]
> > > > > > > >
> > > > > >
> > https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
> > > > > > > > [2]
> > https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > > > > >
> > > > > > > > On Wed, Sep 20, 2023 at 4:52 PM Zakelly Lan <
> > zakelly.lan@gmail.com>
> > > > > > wrote:
> > > > > > > >
> > > > > > > >> Hi Yanfei,
> > > > > > > >>
> > > > > > > >> Thanks for your reply!
> > > > > > > >>
> > > > > > > >> Yes, this FLIP aims to change all state-related exceptions to
> > > > > > > >> unchecked exceptions and remove all exceptions from the
> > signature. So
> > > > > > > >> I believe we have come to an agreement to keep the interfaces
> > simple.
> > > > > > > >>
> > > > > > > >>
> > > > > > > >> Best regards,
> > > > > > > >> Zakelly
> > > > > > > >>
> > > > > > > >> On Wed, Sep 20, 2023 at 2:26 PM Zakelly Lan <
> > zakelly.lan@gmail.com>
> > > > > > > >> wrote:
> > > > > > > >> >
> > > > > > > >> > Hi Hangxiang,
> > > > > > > >> >
> > > > > > > >> > Thank you for your response! Here are my thoughts:
> > > > > > > >> >
> > > > > > > >> > 1. Regarding the exceptions thrown by internal interfaces,
> > I suggest
> > > > > > > >> > keeping them as checked exceptions. Since these exceptions
> > will be
> > > > > > > >> > handled by the internal callers, it is meaningful to throw
> > them as
> > > > > > > >> > checked ones. If we need to make changes to these classes,
> > we can
> > > > > > > >> > create separate tickets alongside this FLIP. What are your
> > thoughts
> > > > > > on
> > > > > > > >> > this?
> > > > > > > >> > 2. StateIOException is primarily thrown by file-based state
> > like
> > > > > > > >> > RocksDB, while StateAccessException is more generic and can
> > be
> > > > > > thrown
> > > > > > > >> > by heap states. Additionally, I believe there may be more
> > subclasses
> > > > > > > >> > of StateAccessException that we can add. We can consider
> > this when
> > > > > > > >> > implementing.
> > > > > > > >> > 3. I would like to make this change in version 1.19. As
> > mentioned in
> > > > > > > >> > this FLIP, many users do not catch any exceptions since the
> > element
> > > > > > > >> > processing function exposes the exception to the upper
> > layer.
> > > > > > > >> > Therefore, the impact is manageable. And I completely agree
> > that we
> > > > > > > >> > should clearly document this change in the next release
> > notes.
> > > > > > > >> >
> > > > > > > >> >
> > > > > > > >> > Best regards,
> > > > > > > >> > Zakelly
> > > > > > > >> >
> > > > > > > >> > On Wed, Sep 20, 2023 at 12:35 PM Yanfei Lei <
> > fredialei@gmail.com>
> > > > > > > >> wrote:
> > > > > > > >> > >
> > > > > > > >> > > Hi Zakelly,
> > > > > > > >> > >
> > > > > > > >> > > Thanks for bringing this up. +1 for reorganizing.
> > > > > > > >> > >
> > > > > > > >> > > IIUC, this proposal aims to change all state-related
> > exceptions to
> > > > > > > >> > > unchecked exceptions. If users have caught checked
> > exceptions
> > > > > > (such as
> > > > > > > >> > > IOException ) in their code, leaving the code as is would
> > also
> > > > > > work.
> > > > > > > >> > >
> > > > > > > >> > > Is it possible not to put any exceptions in the signature
> > of
> > > > > > > >> > > user-facing interfaces? As the proposal mentioned, users
> > can do a
> > > > > > few
> > > > > > > >> > > things even if they catch the exceptions. Keeping the
> > interface
> > > > > > simple
> > > > > > > >> > > may be easier to understand.
> > > > > > > >> > >
> > > > > > > >> > >
> > > > > > > >> > > Best,
> > > > > > > >> > > Yanfei
> > > > > > > >> > >
> > > > > > > >> > > Hangxiang Yu <ma...@gmail.com> 于2023年9月19日周二 18:16写道：
> > > > > > > >> > > >
> > > > > > > >> > > > Hi, Zakelly.
> > > > > > > >> > > >
> > > > > > > >> > > > Thanks for the proposal.
> > > > > > > >> > > >
> > > > > > > >> > > > +1 for reorganizing exceptions of state interfaces
> > which indeed
> > > > > > > >> confuses me
> > > > > > > >> > > > currently.
> > > > > > > >> > > >
> > > > > > > >> > > > From my experience, users usually omit these exceptions
> > because
> > > > > > > >> they cannot
> > > > > > > >> > > > do much even if they catch the exceptions.
> > > > > > > >> > > >
> > > > > > > >> > > > I have some problems and suggestions, PTAL:
> > > > > > > >> > > >
> > > > > > > >> > > > 1. Could we also reorganize or add more state
> > exceptions
> > > > > > (may be
> > > > > > > >> related
> > > > > > > >> > > > to other state interfaces/classes e.g.
> > KeyedStateBackend)
> > > > > > into
> > > > > > > >> the
> > > > > > > >> > > > exception class diagrams ? Although these
> > state-related
> > > > > > classes
> > > > > > > >> may not
> > > > > > > >> > > > be public, it could be better to consider them
> > together to
> > > > > > make
> > > > > > > >> all
> > > > > > > >> > > > state-related exceptions clear. For example, we could
> > > > > > reorganize
> > > > > > > >> some
> > > > > > > >> > > > existing exceptions such as StateMigrationException,
> > add some
> > > > > > > >> exceptions
> > > > > > > >> > > > such as StateNotFoundException.
> > > > > > > >> > > > 2. Could you clarify or give an example about the
> > extended
> > > > > > > >> relation
> > > > > > > >> > > > "StateAccessException -- StateIOException" ? When do
> > we just
> > > > > > > >> return
> > > > > > > >> > > > StateAccessException instead of StateIOException or
> > others ?
> > > > > > > >> > > > 3. Which version do you want to implement it in ?
> > Since it
> > > > > > has
> > > > > > > >> to break
> > > > > > > >> > > > changes for users who have catched the IOException,
> > If we
> > > > > > want
> > > > > > > >> to implement
> > > > > > > >> > > > it in 1.19, we must mark it very clearly in the
> > release
> > > > > > note, or
> > > > > > > >> we should
> > > > > > > >> > > > make it in 2.0.
> > > > > > > >> > > >
> > > > > > > >> > > >
> > > > > > > >> > > > On Tue, Sep 19, 2023 at 5:08 PM Zakelly Lan <
> > > > > > zakelly.lan@gmail.com>
> > > > > > > >> wrote:
> > > > > > > >> > > >
> > > > > > > >> > > > > Hi everyone,
> > > > > > > >> > > > >
> > > > > > > >> > > > > I would like to initiate a discussion on FLIP-368,
> > which
> > > > > > focuses
> > > > > > > >> on
> > > > > > > >> > > > > reorganizing the exceptions thrown in state
> > interfaces [1].
> > > > > > > >> > > > >
> > > > > > > >> > > > > Currently, we have identified several problems with
> > the
> > > > > > exceptions
> > > > > > > >> > > > > thrown by state-related interfaces:
> > > > > > > >> > > > > 1. The exception types thrown by each interface are
> > > > > > > >> inconsistent.
> > > > > > > >> > > > > While most of the interfaces claim to throw
> > `Exception`, the
> > > > > > > >> > > > > interfaces of `ValueState` throw `IOException`.
> > Additionally,
> > > > > > the
> > > > > > > >> > > > > `State#clear()` never throws an exception. This can be
> > > > > > confusing
> > > > > > > >> for
> > > > > > > >> > > > > users.
> > > > > > > >> > > > > 2. The use of `Exception` or `IOException` as the
> > thrown
> > > > > > > >> exception
> > > > > > > >> > > > > type is too generic and lacks specificity.
> > > > > > > >> > > > > 3. Users may not be able to handle these
> > exceptions. In
> > > > > > cases
> > > > > > > >> where
> > > > > > > >> > > > > an exception occurs while accessing state, the job
> > should
> > > > > > fail.
> > > > > > > >> This
> > > > > > > >> > > > > aligns more with the characteristic of *unchecked
> > exceptions*
> > > > > > > >> instead
> > > > > > > >> > > > > of checked exceptions.
> > > > > > > >> > > > >
> > > > > > > >> > > > > To address these issues, we borrow the idea of
> > throwing
> > > > > > unchecked
> > > > > > > >> > > > > exceptions in Java Collection API and propose the
> > following
> > > > > > > >> changes in
> > > > > > > >> > > > > state-related exceptions:
> > > > > > > >> > > > > 1. Introduction of specific unchecked exception
> > types for
> > > > > > > >> different
> > > > > > > >> > > > > reasons, providing users with more precise
> > information about
> > > > > > the
> > > > > > > >> cause
> > > > > > > >> > > > > of the exception.
> > > > > > > >> > > > > 2. Removal of all checked exceptions from interface
> > > > > > signatures
> > > > > > > >> and
> > > > > > > >> > > > > instead, throwing newly introduced unchecked
> > exceptions in the
> > > > > > > >> > > > > implementations.
> > > > > > > >> > > > >
> > > > > > > >> > > > > Please share your thoughts and suggestions regarding
> > the
> > > > > > proposed
> > > > > > > >> > > > > changes. Thank you for your attention and support.
> > > > > > > >> > > > >
> > > > > > > >> > > > >
> > > > > > > >> > > > > Best,
> > > > > > > >> > > > > Zakelly
> > > > > > > >> > > > >
> > > > > > > >> > > > > [1] FLIP-368: Reorganize the exceptions thrown in
> > state
> > > > > > > >> interfaces,
> > > > > > > >> > > > > https://cwiki.apache.org/confluence/x/eZ2zDw
> > > > > > > >> > > > >
> > > > > > > >> > > >
> > > > > > > >> > > >
> > > > > > > >> > > > --
> > > > > > > >> > > > Best,
> > > > > > > >> > > > Hangxiang.
> > > > > > > >>
> > > > > > > >
> > > > > >
> > >
> > > Unless otherwise stated above:
> > >
> > > IBM United Kingdom Limited
> > > Registered in England and Wales with number 741598
> > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
> > >
> > > Unless otherwise stated above:
> > >
> > > IBM United Kingdom Limited
> > > Registered in England and Wales with number 741598
> > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
> >
> > Unless otherwise stated above:
> >
> > IBM United Kingdom Limited
> > Registered in England and Wales with number 741598
> > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
> >
Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Posted by Yuan Mei <yu...@gmail.com>.
+1 for the proposal
But "Since the signature of the public state API has been changed", I was
wondering whether this would be more fittable in Flink 2.0, instead of 1.19?
WDYT?
Best
Yuan
On Wed, Oct 11, 2023 at 4:34 PM David Radley <da...@uk.ibm.com>
wrote:
> Hi Zakelly,
> Thanks for making this clear for me. We should document the impact on the
> user in the release notes, which will be a minimal rewrite and recompile of
> any java using the old APIs.
> I think it is a good point you make about if there are future
> implementations that are
> worth retrying (such as network access) – then there could be retries. I
> agree we should not be trying to create code now for an implementation
> consideration that is not there yet,
>
> +1 from me ,
> Kind regards, David.
>
> From: Zakelly Lan <za...@gmail.com>
> Date: Wednesday, 11 October 2023 at 04:25
> To: dev@flink.apache.org <de...@flink.apache.org>
> Subject: [EXTERNAL] Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the
> exceptions thrown in state interfaces
> Hi David,
>
> Thanks for your response.
>
> The exceptions thrown by state interfaces are NOT retriable. For
> example, there may be some elements sent to the wrong subtask due to a
> non-deterministic hashCode() algorithm and the key group is not
> matching. Or the rocksdb may fail to read a file if it has been
> deleted by the user. If there are future implementations that are
> worth retrying (such as network access), it would be better to let the
> implementation itself handle the retries and provide a configuration
> for this, rather than requiring users to catch these exceptions.
>
> Regarding the release and documentation, I have mentioned that this
> change is targeted for version 1.19 with proper documentation. You may
> have noticed that state interfaces are annotated with @PublicEvolving,
> which means these interfaces may change across versions. The changes
> are suitable for a minor release (1.18.0 currently to 1.19.0 in the
> future) as defined by the API compatibility guarantees of Flink[1].
>
>
>
> Best,
> Zakelly
>
>
> [1]
> https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees
>
> On Tue, Oct 10, 2023 at 6:19 PM David Radley <da...@uk.ibm.com>
> wrote:
> >
> > Hi,
> > I notice
> https://nightlies.apache.org/flink/flink-docs-master/api/java/org/apache/flink/api/common/state/ValueState.html
> is an external API. I am concerned that this change will break existing
> applications using the old interface, they are likely to have catches /
> throws around the existing checked Exceptions.
> >
> > If we go with RunTimeException, I would suggest that this sort of
> breaking change should be done on a Flink version change, where it is
> appropriate to make breaking changes to the API with associated
> documentation.
> >
> > If we want this change on a minor release, we could create a new class
> ValueState2– that is used internally with the cleaned up Exceptions, but
> still expose the old class and Exceptions for existing external
> applications. I guess new applications could use the new ValueState2 .
> >
> > What do you think?
> > Kind regards, David.
> >
> >
> > From: David Radley <da...@uk.ibm.com>
> > Date: Tuesday, 10 October 2023 at 09:49
> > To: dev@flink.apache.org <de...@flink.apache.org>
> > Subject: [EXTERNAL] RE: [DISCUSS] FLIP-368 Reorganize the exceptions
> thrown in state interfaces
> > Hi ,
> > The argument seems to be that the errors cannot be acted on so should be
> runtime exceptions. I want to confirm that none of these errors could /
> should be retriable. If there is a possibility that the state is available
> at some time later then I assume a checked retriable Exception would be
> appropriate for those cases; and be part of the contract with the caller.
> Can we be sure that there is no possibility that the state will become
> available; if so then I agree that a runtime Exception is appropriate. What
> do you think?
> >
> >
> >
> > Kind regards, David.
> >
> >
> > From: Zakelly Lan <za...@gmail.com>
> > Date: Monday, 9 October 2023 at 18:12
> > To: dev@flink.apache.org <de...@flink.apache.org>
> > Subject: [EXTERNAL] Re: [DISCUSS] FLIP-368 Reorganize the exceptions
> thrown in state interfaces
> > Hi everyone,
> >
> > It seems we're gradually reaching a consensus. So I would like to
> > start a vote after 72 hours if there are no further discussions.
> >
> > Please let me know if you have any concerns, thanks!
> >
> >
> > Best,
> > Zakelly
> >
> >
> > On Sat, Oct 7, 2023 at 4:07 PM Zakelly Lan <za...@gmail.com>
> wrote:
> > >
> > > Hi Jing,
> > >
> > > Sorry for the late reply! I agree with you that we do not expect users
> > > to do anything with Flink and we won't "bother" them with those
> > > exceptions. However, users can still catch the `Throwable` and perform
> > > any necessary logging activities, similar to how they use Java
> > > Collection interfaces.
> > >
> > >
> > > Thanks for your insights!
> > >
> > > Best,
> > > Zakelly
> > >
> > > On Thu, Sep 21, 2023 at 8:43 PM Jing Ge <ji...@ververica.com.invalid>
> wrote:
> > > >
> > > > Fair enough! Thanks Zakelly for the information. Afaic, even users
> can do
> > > > nothing with Flink, they still can do something in their territory,
> at
> > > > least doing some logging and metrics stuff, or triggering some other
> > > > services in their ecosystem. After all, the Flink jobs they build
> are part
> > > > of their service component. It didn't change the fact that we are
> going to
> > > > use the anti-pattern. Just because we didn't expect users to do
> > > > anything with Flink, does not mean users don't expect to do
> something with
> > > > the expected exception. Anyway, I am open to hearing different
> opinions.
> > > >
> > > > Best regards,
> > > > Jing
> > > >
> > > > On Thu, Sep 21, 2023 at 7:02 AM Zakelly Lan <za...@gmail.com>
> wrote:
> > > >
> > > > > Hi Martijn,
> > > > >
> > > > > Thanks for the reminder!
> > > > >
> > > > > This FLIP proposes a change to the state API that is annotated as
> > > > > @PublicEvolving and targets version 1.19. I have clarified this in
> > > > > the "Proposed Change" section of the FLIP.
> > > > >
> > > > >
> > > > > Hi Jing,
> > > > >
> > > > > Thanks for sharing your thoughts! Here are my opinions:
> > > > >
> > > > > 1. The exceptions of the state API are usually treated as critical
> > > > > ones. In other words, if anything goes wrong with state accessing,
> the
> > > > > element processing cannot proceed and the job should fail. Flink
> users
> > > > > may not know what to do when they encounter these exceptions. I
> > > > > believe this is the main reason why we want to replace them with
> > > > > unchecked exceptions.
> > > > > 2. There have also been some further discussions[1][2] from Stephan
> > > > > and Shixiaogang below the one you pointed out [3], and it seems
> they
> > > > > come to an agreement to use unchecked exceptions. After reviewing
> the
> > > > > entire discussion on that PR, I think their arguments are
> reasonable
> > > > > given the use case.
> > > > >
> > > > > Looking forward to your feedback.
> > > > >
> > > > >
> > > > > Best,
> > > > > Zakelly
> > > > >
> > > > > [1]
> https://github.com/apache/flink/pull/3380#issuecomment-286807853
> > > > > [2]
> https://github.com/apache/flink/pull/3380#issuecomment-286932133
> > > > > [3]
> https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > >
> > > > > On Thu, Sep 21, 2023 at 1:27 AM Jing Ge <jing@ververica.com.invalid
> >
> > > > > wrote:
> > > > > >
> > > > > > sorry, typo: It is a known "anti-pattern" instead of
> "ant-pattern"
> > > > > >
> > > > > > Best regards,
> > > > > > Jing
> > > > > >
> > > > > > On Wed, Sep 20, 2023 at 7:23 PM Jing Ge <ji...@ververica.com>
> wrote:
> > > > > >
> > > > > > > Hi Zakelly,
> > > > > > >
> > > > > > > Thanks for driving this topic. From good software engineering's
> > > > > > > perspective, I have different thoughts:
> > > > > > >
> > > > > > > 1. The idea to get rid of all checked Exceptions and replace
> them with
> > > > > > > unchecked Exceptions is a known ant-pattern: "Generally
> speaking, do
> > > > > not
> > > > > > > throw a RuntimeException or create a subclass of
> RuntimeException
> > > > > simply
> > > > > > > because you don't want to be bothered with specifying the
> exceptions
> > > > > your
> > > > > > > methods can throw." [1] Checked Exceptions mean expected
> exceptions
> > > > > that
> > > > > > > can help developers find a way to catch them and decide what
> to do. It
> > > > > is
> > > > > > > part of the public API signature that can help developers
> build robust
> > > > > > > systems. We should not mix concepts and build expected
> exceptions with
> > > > > > > unchecked Java Exception classes.
> > > > > > > 2. The comment Stephan left [2] clearly pointed out that we
> should
> > > > > avoid
> > > > > > > using generic Java Exceptions, and "find some more 'specific'
> > > > > exceptions
> > > > > > > for the signature, like throws IOException or throws
> > > > > StateAccessException."
> > > > > > > So, the idea is to define/use specific checked Exception
> classes
> > > > > instead of
> > > > > > > using unchecked Exceptions.
> > > > > > >
> > > > > > > Looking forward to your thoughts.
> > > > > > >
> > > > > > > Best regards,
> > > > > > > Jing
> > > > > > >
> > > > > > >
> > > > > > > [1]
> > > > > > >
> > > > >
> https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
> > > > > > > [2]
> https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > > > >
> > > > > > > On Wed, Sep 20, 2023 at 4:52 PM Zakelly Lan <
> zakelly.lan@gmail.com>
> > > > > wrote:
> > > > > > >
> > > > > > >> Hi Yanfei,
> > > > > > >>
> > > > > > >> Thanks for your reply!
> > > > > > >>
> > > > > > >> Yes, this FLIP aims to change all state-related exceptions to
> > > > > > >> unchecked exceptions and remove all exceptions from the
> signature. So
> > > > > > >> I believe we have come to an agreement to keep the interfaces
> simple.
> > > > > > >>
> > > > > > >>
> > > > > > >> Best regards,
> > > > > > >> Zakelly
> > > > > > >>
> > > > > > >> On Wed, Sep 20, 2023 at 2:26 PM Zakelly Lan <
> zakelly.lan@gmail.com>
> > > > > > >> wrote:
> > > > > > >> >
> > > > > > >> > Hi Hangxiang,
> > > > > > >> >
> > > > > > >> > Thank you for your response! Here are my thoughts:
> > > > > > >> >
> > > > > > >> > 1. Regarding the exceptions thrown by internal interfaces,
> I suggest
> > > > > > >> > keeping them as checked exceptions. Since these exceptions
> will be
> > > > > > >> > handled by the internal callers, it is meaningful to throw
> them as
> > > > > > >> > checked ones. If we need to make changes to these classes,
> we can
> > > > > > >> > create separate tickets alongside this FLIP. What are your
> thoughts
> > > > > on
> > > > > > >> > this?
> > > > > > >> > 2. StateIOException is primarily thrown by file-based state
> like
> > > > > > >> > RocksDB, while StateAccessException is more generic and can
> be
> > > > > thrown
> > > > > > >> > by heap states. Additionally, I believe there may be more
> subclasses
> > > > > > >> > of StateAccessException that we can add. We can consider
> this when
> > > > > > >> > implementing.
> > > > > > >> > 3. I would like to make this change in version 1.19. As
> mentioned in
> > > > > > >> > this FLIP, many users do not catch any exceptions since the
> element
> > > > > > >> > processing function exposes the exception to the upper
> layer.
> > > > > > >> > Therefore, the impact is manageable. And I completely agree
> that we
> > > > > > >> > should clearly document this change in the next release
> notes.
> > > > > > >> >
> > > > > > >> >
> > > > > > >> > Best regards,
> > > > > > >> > Zakelly
> > > > > > >> >
> > > > > > >> > On Wed, Sep 20, 2023 at 12:35 PM Yanfei Lei <
> fredialei@gmail.com>
> > > > > > >> wrote:
> > > > > > >> > >
> > > > > > >> > > Hi Zakelly,
> > > > > > >> > >
> > > > > > >> > > Thanks for bringing this up. +1 for reorganizing.
> > > > > > >> > >
> > > > > > >> > > IIUC, this proposal aims to change all state-related
> exceptions to
> > > > > > >> > > unchecked exceptions. If users have caught checked
> exceptions
> > > > > (such as
> > > > > > >> > > IOException ) in their code, leaving the code as is would
> also
> > > > > work.
> > > > > > >> > >
> > > > > > >> > > Is it possible not to put any exceptions in the signature
> of
> > > > > > >> > > user-facing interfaces? As the proposal mentioned, users
> can do a
> > > > > few
> > > > > > >> > > things even if they catch the exceptions. Keeping the
> interface
> > > > > simple
> > > > > > >> > > may be easier to understand.
> > > > > > >> > >
> > > > > > >> > >
> > > > > > >> > > Best,
> > > > > > >> > > Yanfei
> > > > > > >> > >
> > > > > > >> > > Hangxiang Yu <ma...@gmail.com> 于2023年9月19日周二 18:16写道：
> > > > > > >> > > >
> > > > > > >> > > > Hi, Zakelly.
> > > > > > >> > > >
> > > > > > >> > > > Thanks for the proposal.
> > > > > > >> > > >
> > > > > > >> > > > +1 for reorganizing exceptions of state interfaces
> which indeed
> > > > > > >> confuses me
> > > > > > >> > > > currently.
> > > > > > >> > > >
> > > > > > >> > > > From my experience, users usually omit these exceptions
> because
> > > > > > >> they cannot
> > > > > > >> > > > do much even if they catch the exceptions.
> > > > > > >> > > >
> > > > > > >> > > > I have some problems and suggestions, PTAL:
> > > > > > >> > > >
> > > > > > >> > > > 1. Could we also reorganize or add more state
> exceptions
> > > > > (may be
> > > > > > >> related
> > > > > > >> > > > to other state interfaces/classes e.g.
> KeyedStateBackend)
> > > > > into
> > > > > > >> the
> > > > > > >> > > > exception class diagrams ? Although these
> state-related
> > > > > classes
> > > > > > >> may not
> > > > > > >> > > > be public, it could be better to consider them
> together to
> > > > > make
> > > > > > >> all
> > > > > > >> > > > state-related exceptions clear. For example, we could
> > > > > reorganize
> > > > > > >> some
> > > > > > >> > > > existing exceptions such as StateMigrationException,
> add some
> > > > > > >> exceptions
> > > > > > >> > > > such as StateNotFoundException.
> > > > > > >> > > > 2. Could you clarify or give an example about the
> extended
> > > > > > >> relation
> > > > > > >> > > > "StateAccessException -- StateIOException" ? When do
> we just
> > > > > > >> return
> > > > > > >> > > > StateAccessException instead of StateIOException or
> others ?
> > > > > > >> > > > 3. Which version do you want to implement it in ?
> Since it
> > > > > has
> > > > > > >> to break
> > > > > > >> > > > changes for users who have catched the IOException,
> If we
> > > > > want
> > > > > > >> to implement
> > > > > > >> > > > it in 1.19, we must mark it very clearly in the
> release
> > > > > note, or
> > > > > > >> we should
> > > > > > >> > > > make it in 2.0.
> > > > > > >> > > >
> > > > > > >> > > >
> > > > > > >> > > > On Tue, Sep 19, 2023 at 5:08 PM Zakelly Lan <
> > > > > zakelly.lan@gmail.com>
> > > > > > >> wrote:
> > > > > > >> > > >
> > > > > > >> > > > > Hi everyone,
> > > > > > >> > > > >
> > > > > > >> > > > > I would like to initiate a discussion on FLIP-368,
> which
> > > > > focuses
> > > > > > >> on
> > > > > > >> > > > > reorganizing the exceptions thrown in state
> interfaces [1].
> > > > > > >> > > > >
> > > > > > >> > > > > Currently, we have identified several problems with
> the
> > > > > exceptions
> > > > > > >> > > > > thrown by state-related interfaces:
> > > > > > >> > > > > 1. The exception types thrown by each interface are
> > > > > > >> inconsistent.
> > > > > > >> > > > > While most of the interfaces claim to throw
> `Exception`, the
> > > > > > >> > > > > interfaces of `ValueState` throw `IOException`.
> Additionally,
> > > > > the
> > > > > > >> > > > > `State#clear()` never throws an exception. This can be
> > > > > confusing
> > > > > > >> for
> > > > > > >> > > > > users.
> > > > > > >> > > > > 2. The use of `Exception` or `IOException` as the
> thrown
> > > > > > >> exception
> > > > > > >> > > > > type is too generic and lacks specificity.
> > > > > > >> > > > > 3. Users may not be able to handle these
> exceptions. In
> > > > > cases
> > > > > > >> where
> > > > > > >> > > > > an exception occurs while accessing state, the job
> should
> > > > > fail.
> > > > > > >> This
> > > > > > >> > > > > aligns more with the characteristic of *unchecked
> exceptions*
> > > > > > >> instead
> > > > > > >> > > > > of checked exceptions.
> > > > > > >> > > > >
> > > > > > >> > > > > To address these issues, we borrow the idea of
> throwing
> > > > > unchecked
> > > > > > >> > > > > exceptions in Java Collection API and propose the
> following
> > > > > > >> changes in
> > > > > > >> > > > > state-related exceptions:
> > > > > > >> > > > > 1. Introduction of specific unchecked exception
> types for
> > > > > > >> different
> > > > > > >> > > > > reasons, providing users with more precise
> information about
> > > > > the
> > > > > > >> cause
> > > > > > >> > > > > of the exception.
> > > > > > >> > > > > 2. Removal of all checked exceptions from interface
> > > > > signatures
> > > > > > >> and
> > > > > > >> > > > > instead, throwing newly introduced unchecked
> exceptions in the
> > > > > > >> > > > > implementations.
> > > > > > >> > > > >
> > > > > > >> > > > > Please share your thoughts and suggestions regarding
> the
> > > > > proposed
> > > > > > >> > > > > changes. Thank you for your attention and support.
> > > > > > >> > > > >
> > > > > > >> > > > >
> > > > > > >> > > > > Best,
> > > > > > >> > > > > Zakelly
> > > > > > >> > > > >
> > > > > > >> > > > > [1] FLIP-368: Reorganize the exceptions thrown in
> state
> > > > > > >> interfaces,
> > > > > > >> > > > > https://cwiki.apache.org/confluence/x/eZ2zDw
> > > > > > >> > > > >
> > > > > > >> > > >
> > > > > > >> > > >
> > > > > > >> > > > --
> > > > > > >> > > > Best,
> > > > > > >> > > > Hangxiang.
> > > > > > >>
> > > > > > >
> > > > >
> >
> > Unless otherwise stated above:
> >
> > IBM United Kingdom Limited
> > Registered in England and Wales with number 741598
> > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
> >
> > Unless otherwise stated above:
> >
> > IBM United Kingdom Limited
> > Registered in England and Wales with number 741598
> > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
>
> Unless otherwise stated above:
>
> IBM United Kingdom Limited
> Registered in England and Wales with number 741598
> Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
>
RE: FW: RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Posted by David Radley <da...@uk.ibm.com>.
Hi Zakelly,
Thanks for making this clear for me. We should document the impact on the user in the release notes, which will be a minimal rewrite and recompile of any java using the old APIs.
I think it is a good point you make about if there are future implementations that are
worth retrying (such as network access) – then there could be retries. I agree we should not be trying to create code now for an implementation consideration that is not there yet,
+1 from me ,
Kind regards, David.
From: Zakelly Lan <za...@gmail.com>
Date: Wednesday, 11 October 2023 at 04:25
To: dev@flink.apache.org <de...@flink.apache.org>
Subject: [EXTERNAL] Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Hi David,
Thanks for your response.
The exceptions thrown by state interfaces are NOT retriable. For
example, there may be some elements sent to the wrong subtask due to a
non-deterministic hashCode() algorithm and the key group is not
matching. Or the rocksdb may fail to read a file if it has been
deleted by the user. If there are future implementations that are
worth retrying (such as network access), it would be better to let the
implementation itself handle the retries and provide a configuration
for this, rather than requiring users to catch these exceptions.
Regarding the release and documentation, I have mentioned that this
change is targeted for version 1.19 with proper documentation. You may
have noticed that state interfaces are annotated with @PublicEvolving,
which means these interfaces may change across versions. The changes
are suitable for a minor release (1.18.0 currently to 1.19.0 in the
future) as defined by the API compatibility guarantees of Flink[1].
Best,
Zakelly
[1] https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees
On Tue, Oct 10, 2023 at 6:19 PM David Radley <da...@uk.ibm.com> wrote:
>
> Hi,
> I notice https://nightlies.apache.org/flink/flink-docs-master/api/java/org/apache/flink/api/common/state/ValueState.html is an external API. I am concerned that this change will break existing applications using the old interface, they are likely to have catches / throws around the existing checked Exceptions.
>
> If we go with RunTimeException, I would suggest that this sort of breaking change should be done on a Flink version change, where it is appropriate to make breaking changes to the API with associated documentation.
>
> If we want this change on a minor release, we could create a new class ValueState2– that is used internally with the cleaned up Exceptions, but still expose the old class and Exceptions for existing external applications. I guess new applications could use the new ValueState2 .
>
> What do you think?
> Kind regards, David.
>
>
> From: David Radley <da...@uk.ibm.com>
> Date: Tuesday, 10 October 2023 at 09:49
> To: dev@flink.apache.org <de...@flink.apache.org>
> Subject: [EXTERNAL] RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
> Hi ,
> The argument seems to be that the errors cannot be acted on so should be runtime exceptions. I want to confirm that none of these errors could / should be retriable. If there is a possibility that the state is available at some time later then I assume a checked retriable Exception would be appropriate for those cases; and be part of the contract with the caller. Can we be sure that there is no possibility that the state will become available; if so then I agree that a runtime Exception is appropriate. What do you think?
>
>
>
> Kind regards, David.
>
>
> From: Zakelly Lan <za...@gmail.com>
> Date: Monday, 9 October 2023 at 18:12
> To: dev@flink.apache.org <de...@flink.apache.org>
> Subject: [EXTERNAL] Re: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
> Hi everyone,
>
> It seems we're gradually reaching a consensus. So I would like to
> start a vote after 72 hours if there are no further discussions.
>
> Please let me know if you have any concerns, thanks!
>
>
> Best,
> Zakelly
>
>
> On Sat, Oct 7, 2023 at 4:07 PM Zakelly Lan <za...@gmail.com> wrote:
> >
> > Hi Jing,
> >
> > Sorry for the late reply! I agree with you that we do not expect users
> > to do anything with Flink and we won't "bother" them with those
> > exceptions. However, users can still catch the `Throwable` and perform
> > any necessary logging activities, similar to how they use Java
> > Collection interfaces.
> >
> >
> > Thanks for your insights!
> >
> > Best,
> > Zakelly
> >
> > On Thu, Sep 21, 2023 at 8:43 PM Jing Ge <ji...@ververica.com.invalid> wrote:
> > >
> > > Fair enough! Thanks Zakelly for the information. Afaic, even users can do
> > > nothing with Flink, they still can do something in their territory, at
> > > least doing some logging and metrics stuff, or triggering some other
> > > services in their ecosystem. After all, the Flink jobs they build are part
> > > of their service component. It didn't change the fact that we are going to
> > > use the anti-pattern. Just because we didn't expect users to do
> > > anything with Flink, does not mean users don't expect to do something with
> > > the expected exception. Anyway, I am open to hearing different opinions.
> > >
> > > Best regards,
> > > Jing
> > >
> > > On Thu, Sep 21, 2023 at 7:02 AM Zakelly Lan <za...@gmail.com> wrote:
> > >
> > > > Hi Martijn,
> > > >
> > > > Thanks for the reminder!
> > > >
> > > > This FLIP proposes a change to the state API that is annotated as
> > > > @PublicEvolving and targets version 1.19. I have clarified this in
> > > > the "Proposed Change" section of the FLIP.
> > > >
> > > >
> > > > Hi Jing,
> > > >
> > > > Thanks for sharing your thoughts! Here are my opinions:
> > > >
> > > > 1. The exceptions of the state API are usually treated as critical
> > > > ones. In other words, if anything goes wrong with state accessing, the
> > > > element processing cannot proceed and the job should fail. Flink users
> > > > may not know what to do when they encounter these exceptions. I
> > > > believe this is the main reason why we want to replace them with
> > > > unchecked exceptions.
> > > > 2. There have also been some further discussions[1][2] from Stephan
> > > > and Shixiaogang below the one you pointed out [3], and it seems they
> > > > come to an agreement to use unchecked exceptions. After reviewing the
> > > > entire discussion on that PR, I think their arguments are reasonable
> > > > given the use case.
> > > >
> > > > Looking forward to your feedback.
> > > >
> > > >
> > > > Best,
> > > > Zakelly
> > > >
> > > > [1] https://github.com/apache/flink/pull/3380#issuecomment-286807853
> > > > [2] https://github.com/apache/flink/pull/3380#issuecomment-286932133
> > > > [3] https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > >
> > > > On Thu, Sep 21, 2023 at 1:27 AM Jing Ge <ji...@ververica.com.invalid>
> > > > wrote:
> > > > >
> > > > > sorry, typo: It is a known "anti-pattern" instead of "ant-pattern"
> > > > >
> > > > > Best regards,
> > > > > Jing
> > > > >
> > > > > On Wed, Sep 20, 2023 at 7:23 PM Jing Ge <ji...@ververica.com> wrote:
> > > > >
> > > > > > Hi Zakelly,
> > > > > >
> > > > > > Thanks for driving this topic. From good software engineering's
> > > > > > perspective, I have different thoughts:
> > > > > >
> > > > > > 1. The idea to get rid of all checked Exceptions and replace them with
> > > > > > unchecked Exceptions is a known ant-pattern: "Generally speaking, do
> > > > not
> > > > > > throw a RuntimeException or create a subclass of RuntimeException
> > > > simply
> > > > > > because you don't want to be bothered with specifying the exceptions
> > > > your
> > > > > > methods can throw." [1] Checked Exceptions mean expected exceptions
> > > > that
> > > > > > can help developers find a way to catch them and decide what to do. It
> > > > is
> > > > > > part of the public API signature that can help developers build robust
> > > > > > systems. We should not mix concepts and build expected exceptions with
> > > > > > unchecked Java Exception classes.
> > > > > > 2. The comment Stephan left [2] clearly pointed out that we should
> > > > avoid
> > > > > > using generic Java Exceptions, and "find some more 'specific'
> > > > exceptions
> > > > > > for the signature, like throws IOException or throws
> > > > StateAccessException."
> > > > > > So, the idea is to define/use specific checked Exception classes
> > > > instead of
> > > > > > using unchecked Exceptions.
> > > > > >
> > > > > > Looking forward to your thoughts.
> > > > > >
> > > > > > Best regards,
> > > > > > Jing
> > > > > >
> > > > > >
> > > > > > [1]
> > > > > >
> > > > https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
> > > > > > [2] https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > > >
> > > > > > On Wed, Sep 20, 2023 at 4:52 PM Zakelly Lan <za...@gmail.com>
> > > > wrote:
> > > > > >
> > > > > >> Hi Yanfei,
> > > > > >>
> > > > > >> Thanks for your reply!
> > > > > >>
> > > > > >> Yes, this FLIP aims to change all state-related exceptions to
> > > > > >> unchecked exceptions and remove all exceptions from the signature. So
> > > > > >> I believe we have come to an agreement to keep the interfaces simple.
> > > > > >>
> > > > > >>
> > > > > >> Best regards,
> > > > > >> Zakelly
> > > > > >>
> > > > > >> On Wed, Sep 20, 2023 at 2:26 PM Zakelly Lan <za...@gmail.com>
> > > > > >> wrote:
> > > > > >> >
> > > > > >> > Hi Hangxiang,
> > > > > >> >
> > > > > >> > Thank you for your response! Here are my thoughts:
> > > > > >> >
> > > > > >> > 1. Regarding the exceptions thrown by internal interfaces, I suggest
> > > > > >> > keeping them as checked exceptions. Since these exceptions will be
> > > > > >> > handled by the internal callers, it is meaningful to throw them as
> > > > > >> > checked ones. If we need to make changes to these classes, we can
> > > > > >> > create separate tickets alongside this FLIP. What are your thoughts
> > > > on
> > > > > >> > this?
> > > > > >> > 2. StateIOException is primarily thrown by file-based state like
> > > > > >> > RocksDB, while StateAccessException is more generic and can be
> > > > thrown
> > > > > >> > by heap states. Additionally, I believe there may be more subclasses
> > > > > >> > of StateAccessException that we can add. We can consider this when
> > > > > >> > implementing.
> > > > > >> > 3. I would like to make this change in version 1.19. As mentioned in
> > > > > >> > this FLIP, many users do not catch any exceptions since the element
> > > > > >> > processing function exposes the exception to the upper layer.
> > > > > >> > Therefore, the impact is manageable. And I completely agree that we
> > > > > >> > should clearly document this change in the next release notes.
> > > > > >> >
> > > > > >> >
> > > > > >> > Best regards,
> > > > > >> > Zakelly
> > > > > >> >
> > > > > >> > On Wed, Sep 20, 2023 at 12:35 PM Yanfei Lei <fr...@gmail.com>
> > > > > >> wrote:
> > > > > >> > >
> > > > > >> > > Hi Zakelly,
> > > > > >> > >
> > > > > >> > > Thanks for bringing this up. +1 for reorganizing.
> > > > > >> > >
> > > > > >> > > IIUC, this proposal aims to change all state-related exceptions to
> > > > > >> > > unchecked exceptions. If users have caught checked exceptions
> > > > (such as
> > > > > >> > > IOException ) in their code, leaving the code as is would also
> > > > work.
> > > > > >> > >
> > > > > >> > > Is it possible not to put any exceptions in the signature of
> > > > > >> > > user-facing interfaces? As the proposal mentioned, users can do a
> > > > few
> > > > > >> > > things even if they catch the exceptions. Keeping the interface
> > > > simple
> > > > > >> > > may be easier to understand.
> > > > > >> > >
> > > > > >> > >
> > > > > >> > > Best,
> > > > > >> > > Yanfei
> > > > > >> > >
> > > > > >> > > Hangxiang Yu <ma...@gmail.com> 于2023年9月19日周二 18:16写道：
> > > > > >> > > >
> > > > > >> > > > Hi, Zakelly.
> > > > > >> > > >
> > > > > >> > > > Thanks for the proposal.
> > > > > >> > > >
> > > > > >> > > > +1 for reorganizing exceptions of state interfaces which indeed
> > > > > >> confuses me
> > > > > >> > > > currently.
> > > > > >> > > >
> > > > > >> > > > From my experience, users usually omit these exceptions because
> > > > > >> they cannot
> > > > > >> > > > do much even if they catch the exceptions.
> > > > > >> > > >
> > > > > >> > > > I have some problems and suggestions, PTAL:
> > > > > >> > > >
> > > > > >> > > > 1. Could we also reorganize or add more state exceptions
> > > > (may be
> > > > > >> related
> > > > > >> > > > to other state interfaces/classes e.g. KeyedStateBackend)
> > > > into
> > > > > >> the
> > > > > >> > > > exception class diagrams ? Although these state-related
> > > > classes
> > > > > >> may not
> > > > > >> > > > be public, it could be better to consider them together to
> > > > make
> > > > > >> all
> > > > > >> > > > state-related exceptions clear. For example, we could
> > > > reorganize
> > > > > >> some
> > > > > >> > > > existing exceptions such as StateMigrationException, add some
> > > > > >> exceptions
> > > > > >> > > > such as StateNotFoundException.
> > > > > >> > > > 2. Could you clarify or give an example about the extended
> > > > > >> relation
> > > > > >> > > > "StateAccessException -- StateIOException" ? When do we just
> > > > > >> return
> > > > > >> > > > StateAccessException instead of StateIOException or others ?
> > > > > >> > > > 3. Which version do you want to implement it in ? Since it
> > > > has
> > > > > >> to break
> > > > > >> > > > changes for users who have catched the IOException, If we
> > > > want
> > > > > >> to implement
> > > > > >> > > > it in 1.19, we must mark it very clearly in the release
> > > > note, or
> > > > > >> we should
> > > > > >> > > > make it in 2.0.
> > > > > >> > > >
> > > > > >> > > >
> > > > > >> > > > On Tue, Sep 19, 2023 at 5:08 PM Zakelly Lan <
> > > > zakelly.lan@gmail.com>
> > > > > >> wrote:
> > > > > >> > > >
> > > > > >> > > > > Hi everyone,
> > > > > >> > > > >
> > > > > >> > > > > I would like to initiate a discussion on FLIP-368, which
> > > > focuses
> > > > > >> on
> > > > > >> > > > > reorganizing the exceptions thrown in state interfaces [1].
> > > > > >> > > > >
> > > > > >> > > > > Currently, we have identified several problems with the
> > > > exceptions
> > > > > >> > > > > thrown by state-related interfaces:
> > > > > >> > > > > 1. The exception types thrown by each interface are
> > > > > >> inconsistent.
> > > > > >> > > > > While most of the interfaces claim to throw `Exception`, the
> > > > > >> > > > > interfaces of `ValueState` throw `IOException`. Additionally,
> > > > the
> > > > > >> > > > > `State#clear()` never throws an exception. This can be
> > > > confusing
> > > > > >> for
> > > > > >> > > > > users.
> > > > > >> > > > > 2. The use of `Exception` or `IOException` as the thrown
> > > > > >> exception
> > > > > >> > > > > type is too generic and lacks specificity.
> > > > > >> > > > > 3. Users may not be able to handle these exceptions. In
> > > > cases
> > > > > >> where
> > > > > >> > > > > an exception occurs while accessing state, the job should
> > > > fail.
> > > > > >> This
> > > > > >> > > > > aligns more with the characteristic of *unchecked exceptions*
> > > > > >> instead
> > > > > >> > > > > of checked exceptions.
> > > > > >> > > > >
> > > > > >> > > > > To address these issues, we borrow the idea of throwing
> > > > unchecked
> > > > > >> > > > > exceptions in Java Collection API and propose the following
> > > > > >> changes in
> > > > > >> > > > > state-related exceptions:
> > > > > >> > > > > 1. Introduction of specific unchecked exception types for
> > > > > >> different
> > > > > >> > > > > reasons, providing users with more precise information about
> > > > the
> > > > > >> cause
> > > > > >> > > > > of the exception.
> > > > > >> > > > > 2. Removal of all checked exceptions from interface
> > > > signatures
> > > > > >> and
> > > > > >> > > > > instead, throwing newly introduced unchecked exceptions in the
> > > > > >> > > > > implementations.
> > > > > >> > > > >
> > > > > >> > > > > Please share your thoughts and suggestions regarding the
> > > > proposed
> > > > > >> > > > > changes. Thank you for your attention and support.
> > > > > >> > > > >
> > > > > >> > > > >
> > > > > >> > > > > Best,
> > > > > >> > > > > Zakelly
> > > > > >> > > > >
> > > > > >> > > > > [1] FLIP-368: Reorganize the exceptions thrown in state
> > > > > >> interfaces,
> > > > > >> > > > > https://cwiki.apache.org/confluence/x/eZ2zDw
> > > > > >> > > > >
> > > > > >> > > >
> > > > > >> > > >
> > > > > >> > > > --
> > > > > >> > > > Best,
> > > > > >> > > > Hangxiang.
> > > > > >>
> > > > > >
> > > >
>
> Unless otherwise stated above:
>
> IBM United Kingdom Limited
> Registered in England and Wales with number 741598
> Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
>
> Unless otherwise stated above:
>
> IBM United Kingdom Limited
> Registered in England and Wales with number 741598
> Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
Unless otherwise stated above:
IBM United Kingdom Limited
Registered in England and Wales with number 741598
Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
Re: FW: RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Posted by Zakelly Lan <za...@gmail.com>.
Hi David,
Thanks for your response.
The exceptions thrown by state interfaces are NOT retriable. For
example, there may be some elements sent to the wrong subtask due to a
non-deterministic hashCode() algorithm and the key group is not
matching. Or the rocksdb may fail to read a file if it has been
deleted by the user. If there are future implementations that are
worth retrying (such as network access), it would be better to let the
implementation itself handle the retries and provide a configuration
for this, rather than requiring users to catch these exceptions.
Regarding the release and documentation, I have mentioned that this
change is targeted for version 1.19 with proper documentation. You may
have noticed that state interfaces are annotated with @PublicEvolving,
which means these interfaces may change across versions. The changes
are suitable for a minor release (1.18.0 currently to 1.19.0 in the
future) as defined by the API compatibility guarantees of Flink[1].
Best,
Zakelly
[1] https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees
On Tue, Oct 10, 2023 at 6:19 PM David Radley <da...@uk.ibm.com> wrote:
>
> Hi,
> I notice https://nightlies.apache.org/flink/flink-docs-master/api/java/org/apache/flink/api/common/state/ValueState.html is an external API. I am concerned that this change will break existing applications using the old interface, they are likely to have catches / throws around the existing checked Exceptions.
>
> If we go with RunTimeException, I would suggest that this sort of breaking change should be done on a Flink version change, where it is appropriate to make breaking changes to the API with associated documentation.
>
> If we want this change on a minor release, we could create a new class ValueState2– that is used internally with the cleaned up Exceptions, but still expose the old class and Exceptions for existing external applications. I guess new applications could use the new ValueState2 .
>
> What do you think?
> Kind regards, David.
>
>
> From: David Radley <da...@uk.ibm.com>
> Date: Tuesday, 10 October 2023 at 09:49
> To: dev@flink.apache.org <de...@flink.apache.org>
> Subject: [EXTERNAL] RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
> Hi ,
> The argument seems to be that the errors cannot be acted on so should be runtime exceptions. I want to confirm that none of these errors could / should be retriable. If there is a possibility that the state is available at some time later then I assume a checked retriable Exception would be appropriate for those cases; and be part of the contract with the caller. Can we be sure that there is no possibility that the state will become available; if so then I agree that a runtime Exception is appropriate. What do you think?
>
>
>
> Kind regards, David.
>
>
> From: Zakelly Lan <za...@gmail.com>
> Date: Monday, 9 October 2023 at 18:12
> To: dev@flink.apache.org <de...@flink.apache.org>
> Subject: [EXTERNAL] Re: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
> Hi everyone,
>
> It seems we're gradually reaching a consensus. So I would like to
> start a vote after 72 hours if there are no further discussions.
>
> Please let me know if you have any concerns, thanks!
>
>
> Best,
> Zakelly
>
>
> On Sat, Oct 7, 2023 at 4:07 PM Zakelly Lan <za...@gmail.com> wrote:
> >
> > Hi Jing,
> >
> > Sorry for the late reply! I agree with you that we do not expect users
> > to do anything with Flink and we won't "bother" them with those
> > exceptions. However, users can still catch the `Throwable` and perform
> > any necessary logging activities, similar to how they use Java
> > Collection interfaces.
> >
> >
> > Thanks for your insights!
> >
> > Best,
> > Zakelly
> >
> > On Thu, Sep 21, 2023 at 8:43 PM Jing Ge <ji...@ververica.com.invalid> wrote:
> > >
> > > Fair enough! Thanks Zakelly for the information. Afaic, even users can do
> > > nothing with Flink, they still can do something in their territory, at
> > > least doing some logging and metrics stuff, or triggering some other
> > > services in their ecosystem. After all, the Flink jobs they build are part
> > > of their service component. It didn't change the fact that we are going to
> > > use the anti-pattern. Just because we didn't expect users to do
> > > anything with Flink, does not mean users don't expect to do something with
> > > the expected exception. Anyway, I am open to hearing different opinions.
> > >
> > > Best regards,
> > > Jing
> > >
> > > On Thu, Sep 21, 2023 at 7:02 AM Zakelly Lan <za...@gmail.com> wrote:
> > >
> > > > Hi Martijn,
> > > >
> > > > Thanks for the reminder!
> > > >
> > > > This FLIP proposes a change to the state API that is annotated as
> > > > @PublicEvolving and targets version 1.19. I have clarified this in
> > > > the "Proposed Change" section of the FLIP.
> > > >
> > > >
> > > > Hi Jing,
> > > >
> > > > Thanks for sharing your thoughts! Here are my opinions:
> > > >
> > > > 1. The exceptions of the state API are usually treated as critical
> > > > ones. In other words, if anything goes wrong with state accessing, the
> > > > element processing cannot proceed and the job should fail. Flink users
> > > > may not know what to do when they encounter these exceptions. I
> > > > believe this is the main reason why we want to replace them with
> > > > unchecked exceptions.
> > > > 2. There have also been some further discussions[1][2] from Stephan
> > > > and Shixiaogang below the one you pointed out [3], and it seems they
> > > > come to an agreement to use unchecked exceptions. After reviewing the
> > > > entire discussion on that PR, I think their arguments are reasonable
> > > > given the use case.
> > > >
> > > > Looking forward to your feedback.
> > > >
> > > >
> > > > Best,
> > > > Zakelly
> > > >
> > > > [1] https://github.com/apache/flink/pull/3380#issuecomment-286807853
> > > > [2] https://github.com/apache/flink/pull/3380#issuecomment-286932133
> > > > [3] https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > >
> > > > On Thu, Sep 21, 2023 at 1:27 AM Jing Ge <ji...@ververica.com.invalid>
> > > > wrote:
> > > > >
> > > > > sorry, typo: It is a known "anti-pattern" instead of "ant-pattern"
> > > > >
> > > > > Best regards,
> > > > > Jing
> > > > >
> > > > > On Wed, Sep 20, 2023 at 7:23 PM Jing Ge <ji...@ververica.com> wrote:
> > > > >
> > > > > > Hi Zakelly,
> > > > > >
> > > > > > Thanks for driving this topic. From good software engineering's
> > > > > > perspective, I have different thoughts:
> > > > > >
> > > > > > 1. The idea to get rid of all checked Exceptions and replace them with
> > > > > > unchecked Exceptions is a known ant-pattern: "Generally speaking, do
> > > > not
> > > > > > throw a RuntimeException or create a subclass of RuntimeException
> > > > simply
> > > > > > because you don't want to be bothered with specifying the exceptions
> > > > your
> > > > > > methods can throw." [1] Checked Exceptions mean expected exceptions
> > > > that
> > > > > > can help developers find a way to catch them and decide what to do. It
> > > > is
> > > > > > part of the public API signature that can help developers build robust
> > > > > > systems. We should not mix concepts and build expected exceptions with
> > > > > > unchecked Java Exception classes.
> > > > > > 2. The comment Stephan left [2] clearly pointed out that we should
> > > > avoid
> > > > > > using generic Java Exceptions, and "find some more 'specific'
> > > > exceptions
> > > > > > for the signature, like throws IOException or throws
> > > > StateAccessException."
> > > > > > So, the idea is to define/use specific checked Exception classes
> > > > instead of
> > > > > > using unchecked Exceptions.
> > > > > >
> > > > > > Looking forward to your thoughts.
> > > > > >
> > > > > > Best regards,
> > > > > > Jing
> > > > > >
> > > > > >
> > > > > > [1]
> > > > > >
> > > > https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
> > > > > > [2] https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > > >
> > > > > > On Wed, Sep 20, 2023 at 4:52 PM Zakelly Lan <za...@gmail.com>
> > > > wrote:
> > > > > >
> > > > > >> Hi Yanfei,
> > > > > >>
> > > > > >> Thanks for your reply!
> > > > > >>
> > > > > >> Yes, this FLIP aims to change all state-related exceptions to
> > > > > >> unchecked exceptions and remove all exceptions from the signature. So
> > > > > >> I believe we have come to an agreement to keep the interfaces simple.
> > > > > >>
> > > > > >>
> > > > > >> Best regards,
> > > > > >> Zakelly
> > > > > >>
> > > > > >> On Wed, Sep 20, 2023 at 2:26 PM Zakelly Lan <za...@gmail.com>
> > > > > >> wrote:
> > > > > >> >
> > > > > >> > Hi Hangxiang,
> > > > > >> >
> > > > > >> > Thank you for your response! Here are my thoughts:
> > > > > >> >
> > > > > >> > 1. Regarding the exceptions thrown by internal interfaces, I suggest
> > > > > >> > keeping them as checked exceptions. Since these exceptions will be
> > > > > >> > handled by the internal callers, it is meaningful to throw them as
> > > > > >> > checked ones. If we need to make changes to these classes, we can
> > > > > >> > create separate tickets alongside this FLIP. What are your thoughts
> > > > on
> > > > > >> > this?
> > > > > >> > 2. StateIOException is primarily thrown by file-based state like
> > > > > >> > RocksDB, while StateAccessException is more generic and can be
> > > > thrown
> > > > > >> > by heap states. Additionally, I believe there may be more subclasses
> > > > > >> > of StateAccessException that we can add. We can consider this when
> > > > > >> > implementing.
> > > > > >> > 3. I would like to make this change in version 1.19. As mentioned in
> > > > > >> > this FLIP, many users do not catch any exceptions since the element
> > > > > >> > processing function exposes the exception to the upper layer.
> > > > > >> > Therefore, the impact is manageable. And I completely agree that we
> > > > > >> > should clearly document this change in the next release notes.
> > > > > >> >
> > > > > >> >
> > > > > >> > Best regards,
> > > > > >> > Zakelly
> > > > > >> >
> > > > > >> > On Wed, Sep 20, 2023 at 12:35 PM Yanfei Lei <fr...@gmail.com>
> > > > > >> wrote:
> > > > > >> > >
> > > > > >> > > Hi Zakelly,
> > > > > >> > >
> > > > > >> > > Thanks for bringing this up. +1 for reorganizing.
> > > > > >> > >
> > > > > >> > > IIUC, this proposal aims to change all state-related exceptions to
> > > > > >> > > unchecked exceptions. If users have caught checked exceptions
> > > > (such as
> > > > > >> > > IOException ) in their code, leaving the code as is would also
> > > > work.
> > > > > >> > >
> > > > > >> > > Is it possible not to put any exceptions in the signature of
> > > > > >> > > user-facing interfaces? As the proposal mentioned, users can do a
> > > > few
> > > > > >> > > things even if they catch the exceptions. Keeping the interface
> > > > simple
> > > > > >> > > may be easier to understand.
> > > > > >> > >
> > > > > >> > >
> > > > > >> > > Best,
> > > > > >> > > Yanfei
> > > > > >> > >
> > > > > >> > > Hangxiang Yu <ma...@gmail.com> 于2023年9月19日周二 18:16写道：
> > > > > >> > > >
> > > > > >> > > > Hi, Zakelly.
> > > > > >> > > >
> > > > > >> > > > Thanks for the proposal.
> > > > > >> > > >
> > > > > >> > > > +1 for reorganizing exceptions of state interfaces which indeed
> > > > > >> confuses me
> > > > > >> > > > currently.
> > > > > >> > > >
> > > > > >> > > > From my experience, users usually omit these exceptions because
> > > > > >> they cannot
> > > > > >> > > > do much even if they catch the exceptions.
> > > > > >> > > >
> > > > > >> > > > I have some problems and suggestions, PTAL:
> > > > > >> > > >
> > > > > >> > > > 1. Could we also reorganize or add more state exceptions
> > > > (may be
> > > > > >> related
> > > > > >> > > > to other state interfaces/classes e.g. KeyedStateBackend)
> > > > into
> > > > > >> the
> > > > > >> > > > exception class diagrams ? Although these state-related
> > > > classes
> > > > > >> may not
> > > > > >> > > > be public, it could be better to consider them together to
> > > > make
> > > > > >> all
> > > > > >> > > > state-related exceptions clear. For example, we could
> > > > reorganize
> > > > > >> some
> > > > > >> > > > existing exceptions such as StateMigrationException, add some
> > > > > >> exceptions
> > > > > >> > > > such as StateNotFoundException.
> > > > > >> > > > 2. Could you clarify or give an example about the extended
> > > > > >> relation
> > > > > >> > > > "StateAccessException -- StateIOException" ? When do we just
> > > > > >> return
> > > > > >> > > > StateAccessException instead of StateIOException or others ?
> > > > > >> > > > 3. Which version do you want to implement it in ? Since it
> > > > has
> > > > > >> to break
> > > > > >> > > > changes for users who have catched the IOException, If we
> > > > want
> > > > > >> to implement
> > > > > >> > > > it in 1.19, we must mark it very clearly in the release
> > > > note, or
> > > > > >> we should
> > > > > >> > > > make it in 2.0.
> > > > > >> > > >
> > > > > >> > > >
> > > > > >> > > > On Tue, Sep 19, 2023 at 5:08 PM Zakelly Lan <
> > > > zakelly.lan@gmail.com>
> > > > > >> wrote:
> > > > > >> > > >
> > > > > >> > > > > Hi everyone,
> > > > > >> > > > >
> > > > > >> > > > > I would like to initiate a discussion on FLIP-368, which
> > > > focuses
> > > > > >> on
> > > > > >> > > > > reorganizing the exceptions thrown in state interfaces [1].
> > > > > >> > > > >
> > > > > >> > > > > Currently, we have identified several problems with the
> > > > exceptions
> > > > > >> > > > > thrown by state-related interfaces:
> > > > > >> > > > > 1. The exception types thrown by each interface are
> > > > > >> inconsistent.
> > > > > >> > > > > While most of the interfaces claim to throw `Exception`, the
> > > > > >> > > > > interfaces of `ValueState` throw `IOException`. Additionally,
> > > > the
> > > > > >> > > > > `State#clear()` never throws an exception. This can be
> > > > confusing
> > > > > >> for
> > > > > >> > > > > users.
> > > > > >> > > > > 2. The use of `Exception` or `IOException` as the thrown
> > > > > >> exception
> > > > > >> > > > > type is too generic and lacks specificity.
> > > > > >> > > > > 3. Users may not be able to handle these exceptions. In
> > > > cases
> > > > > >> where
> > > > > >> > > > > an exception occurs while accessing state, the job should
> > > > fail.
> > > > > >> This
> > > > > >> > > > > aligns more with the characteristic of *unchecked exceptions*
> > > > > >> instead
> > > > > >> > > > > of checked exceptions.
> > > > > >> > > > >
> > > > > >> > > > > To address these issues, we borrow the idea of throwing
> > > > unchecked
> > > > > >> > > > > exceptions in Java Collection API and propose the following
> > > > > >> changes in
> > > > > >> > > > > state-related exceptions:
> > > > > >> > > > > 1. Introduction of specific unchecked exception types for
> > > > > >> different
> > > > > >> > > > > reasons, providing users with more precise information about
> > > > the
> > > > > >> cause
> > > > > >> > > > > of the exception.
> > > > > >> > > > > 2. Removal of all checked exceptions from interface
> > > > signatures
> > > > > >> and
> > > > > >> > > > > instead, throwing newly introduced unchecked exceptions in the
> > > > > >> > > > > implementations.
> > > > > >> > > > >
> > > > > >> > > > > Please share your thoughts and suggestions regarding the
> > > > proposed
> > > > > >> > > > > changes. Thank you for your attention and support.
> > > > > >> > > > >
> > > > > >> > > > >
> > > > > >> > > > > Best,
> > > > > >> > > > > Zakelly
> > > > > >> > > > >
> > > > > >> > > > > [1] FLIP-368: Reorganize the exceptions thrown in state
> > > > > >> interfaces,
> > > > > >> > > > > https://cwiki.apache.org/confluence/x/eZ2zDw
> > > > > >> > > > >
> > > > > >> > > >
> > > > > >> > > >
> > > > > >> > > > --
> > > > > >> > > > Best,
> > > > > >> > > > Hangxiang.
> > > > > >>
> > > > > >
> > > >
>
> Unless otherwise stated above:
>
> IBM United Kingdom Limited
> Registered in England and Wales with number 741598
> Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
>
> Unless otherwise stated above:
>
> IBM United Kingdom Limited
> Registered in England and Wales with number 741598
> Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
FW: RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Posted by David Radley <da...@uk.ibm.com>.
Hi,
I notice https://nightlies.apache.org/flink/flink-docs-master/api/java/org/apache/flink/api/common/state/ValueState.html is an external API. I am concerned that this change will break existing applications using the old interface, they are likely to have catches / throws around the existing checked Exceptions.
If we go with RunTimeException, I would suggest that this sort of breaking change should be done on a Flink version change, where it is appropriate to make breaking changes to the API with associated documentation.
If we want this change on a minor release, we could create a new class ValueState2– that is used internally with the cleaned up Exceptions, but still expose the old class and Exceptions for existing external applications. I guess new applications could use the new ValueState2 .
What do you think?
Kind regards, David.
From: David Radley <da...@uk.ibm.com>
Date: Tuesday, 10 October 2023 at 09:49
To: dev@flink.apache.org <de...@flink.apache.org>
Subject: [EXTERNAL] RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Hi ,
The argument seems to be that the errors cannot be acted on so should be runtime exceptions. I want to confirm that none of these errors could / should be retriable. If there is a possibility that the state is available at some time later then I assume a checked retriable Exception would be appropriate for those cases; and be part of the contract with the caller. Can we be sure that there is no possibility that the state will become available; if so then I agree that a runtime Exception is appropriate. What do you think?
Kind regards, David.
From: Zakelly Lan <za...@gmail.com>
Date: Monday, 9 October 2023 at 18:12
To: dev@flink.apache.org <de...@flink.apache.org>
Subject: [EXTERNAL] Re: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Hi everyone,
It seems we're gradually reaching a consensus. So I would like to
start a vote after 72 hours if there are no further discussions.
Please let me know if you have any concerns, thanks!
Best,
Zakelly
On Sat, Oct 7, 2023 at 4:07 PM Zakelly Lan <za...@gmail.com> wrote:
>
> Hi Jing,
>
> Sorry for the late reply! I agree with you that we do not expect users
> to do anything with Flink and we won't "bother" them with those
> exceptions. However, users can still catch the `Throwable` and perform
> any necessary logging activities, similar to how they use Java
> Collection interfaces.
>
>
> Thanks for your insights!
>
> Best,
> Zakelly
>
> On Thu, Sep 21, 2023 at 8:43 PM Jing Ge <ji...@ververica.com.invalid> wrote:
> >
> > Fair enough! Thanks Zakelly for the information. Afaic, even users can do
> > nothing with Flink, they still can do something in their territory, at
> > least doing some logging and metrics stuff, or triggering some other
> > services in their ecosystem. After all, the Flink jobs they build are part
> > of their service component. It didn't change the fact that we are going to
> > use the anti-pattern. Just because we didn't expect users to do
> > anything with Flink, does not mean users don't expect to do something with
> > the expected exception. Anyway, I am open to hearing different opinions.
> >
> > Best regards,
> > Jing
> >
> > On Thu, Sep 21, 2023 at 7:02 AM Zakelly Lan <za...@gmail.com> wrote:
> >
> > > Hi Martijn,
> > >
> > > Thanks for the reminder!
> > >
> > > This FLIP proposes a change to the state API that is annotated as
> > > @PublicEvolving and targets version 1.19. I have clarified this in
> > > the "Proposed Change" section of the FLIP.
> > >
> > >
> > > Hi Jing,
> > >
> > > Thanks for sharing your thoughts! Here are my opinions:
> > >
> > > 1. The exceptions of the state API are usually treated as critical
> > > ones. In other words, if anything goes wrong with state accessing, the
> > > element processing cannot proceed and the job should fail. Flink users
> > > may not know what to do when they encounter these exceptions. I
> > > believe this is the main reason why we want to replace them with
> > > unchecked exceptions.
> > > 2. There have also been some further discussions[1][2] from Stephan
> > > and Shixiaogang below the one you pointed out [3], and it seems they
> > > come to an agreement to use unchecked exceptions. After reviewing the
> > > entire discussion on that PR, I think their arguments are reasonable
> > > given the use case.
> > >
> > > Looking forward to your feedback.
> > >
> > >
> > > Best,
> > > Zakelly
> > >
> > > [1] https://github.com/apache/flink/pull/3380#issuecomment-286807853
> > > [2] https://github.com/apache/flink/pull/3380#issuecomment-286932133
> > > [3] https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > >
> > > On Thu, Sep 21, 2023 at 1:27 AM Jing Ge <ji...@ververica.com.invalid>
> > > wrote:
> > > >
> > > > sorry, typo: It is a known "anti-pattern" instead of "ant-pattern"
> > > >
> > > > Best regards,
> > > > Jing
> > > >
> > > > On Wed, Sep 20, 2023 at 7:23 PM Jing Ge <ji...@ververica.com> wrote:
> > > >
> > > > > Hi Zakelly,
> > > > >
> > > > > Thanks for driving this topic. From good software engineering's
> > > > > perspective, I have different thoughts:
> > > > >
> > > > > 1. The idea to get rid of all checked Exceptions and replace them with
> > > > > unchecked Exceptions is a known ant-pattern: "Generally speaking, do
> > > not
> > > > > throw a RuntimeException or create a subclass of RuntimeException
> > > simply
> > > > > because you don't want to be bothered with specifying the exceptions
> > > your
> > > > > methods can throw." [1] Checked Exceptions mean expected exceptions
> > > that
> > > > > can help developers find a way to catch them and decide what to do. It
> > > is
> > > > > part of the public API signature that can help developers build robust
> > > > > systems. We should not mix concepts and build expected exceptions with
> > > > > unchecked Java Exception classes.
> > > > > 2. The comment Stephan left [2] clearly pointed out that we should
> > > avoid
> > > > > using generic Java Exceptions, and "find some more 'specific'
> > > exceptions
> > > > > for the signature, like throws IOException or throws
> > > StateAccessException."
> > > > > So, the idea is to define/use specific checked Exception classes
> > > instead of
> > > > > using unchecked Exceptions.
> > > > >
> > > > > Looking forward to your thoughts.
> > > > >
> > > > > Best regards,
> > > > > Jing
> > > > >
> > > > >
> > > > > [1]
> > > > >
> > > https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
> > > > > [2] https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > >
> > > > > On Wed, Sep 20, 2023 at 4:52 PM Zakelly Lan <za...@gmail.com>
> > > wrote:
> > > > >
> > > > >> Hi Yanfei,
> > > > >>
> > > > >> Thanks for your reply!
> > > > >>
> > > > >> Yes, this FLIP aims to change all state-related exceptions to
> > > > >> unchecked exceptions and remove all exceptions from the signature. So
> > > > >> I believe we have come to an agreement to keep the interfaces simple.
> > > > >>
> > > > >>
> > > > >> Best regards,
> > > > >> Zakelly
> > > > >>
> > > > >> On Wed, Sep 20, 2023 at 2:26 PM Zakelly Lan <za...@gmail.com>
> > > > >> wrote:
> > > > >> >
> > > > >> > Hi Hangxiang,
> > > > >> >
> > > > >> > Thank you for your response! Here are my thoughts:
> > > > >> >
> > > > >> > 1. Regarding the exceptions thrown by internal interfaces, I suggest
> > > > >> > keeping them as checked exceptions. Since these exceptions will be
> > > > >> > handled by the internal callers, it is meaningful to throw them as
> > > > >> > checked ones. If we need to make changes to these classes, we can
> > > > >> > create separate tickets alongside this FLIP. What are your thoughts
> > > on
> > > > >> > this?
> > > > >> > 2. StateIOException is primarily thrown by file-based state like
> > > > >> > RocksDB, while StateAccessException is more generic and can be
> > > thrown
> > > > >> > by heap states. Additionally, I believe there may be more subclasses
> > > > >> > of StateAccessException that we can add. We can consider this when
> > > > >> > implementing.
> > > > >> > 3. I would like to make this change in version 1.19. As mentioned in
> > > > >> > this FLIP, many users do not catch any exceptions since the element
> > > > >> > processing function exposes the exception to the upper layer.
> > > > >> > Therefore, the impact is manageable. And I completely agree that we
> > > > >> > should clearly document this change in the next release notes.
> > > > >> >
> > > > >> >
> > > > >> > Best regards,
> > > > >> > Zakelly
> > > > >> >
> > > > >> > On Wed, Sep 20, 2023 at 12:35 PM Yanfei Lei <fr...@gmail.com>
> > > > >> wrote:
> > > > >> > >
> > > > >> > > Hi Zakelly,
> > > > >> > >
> > > > >> > > Thanks for bringing this up. +1 for reorganizing.
> > > > >> > >
> > > > >> > > IIUC, this proposal aims to change all state-related exceptions to
> > > > >> > > unchecked exceptions. If users have caught checked exceptions
> > > (such as
> > > > >> > > IOException ) in their code, leaving the code as is would also
> > > work.
> > > > >> > >
> > > > >> > > Is it possible not to put any exceptions in the signature of
> > > > >> > > user-facing interfaces? As the proposal mentioned, users can do a
> > > few
> > > > >> > > things even if they catch the exceptions. Keeping the interface
> > > simple
> > > > >> > > may be easier to understand.
> > > > >> > >
> > > > >> > >
> > > > >> > > Best,
> > > > >> > > Yanfei
> > > > >> > >
> > > > >> > > Hangxiang Yu <ma...@gmail.com> 于2023年9月19日周二 18:16写道：
> > > > >> > > >
> > > > >> > > > Hi, Zakelly.
> > > > >> > > >
> > > > >> > > > Thanks for the proposal.
> > > > >> > > >
> > > > >> > > > +1 for reorganizing exceptions of state interfaces which indeed
> > > > >> confuses me
> > > > >> > > > currently.
> > > > >> > > >
> > > > >> > > > From my experience, users usually omit these exceptions because
> > > > >> they cannot
> > > > >> > > > do much even if they catch the exceptions.
> > > > >> > > >
> > > > >> > > > I have some problems and suggestions, PTAL:
> > > > >> > > >
> > > > >> > > > 1. Could we also reorganize or add more state exceptions
> > > (may be
> > > > >> related
> > > > >> > > > to other state interfaces/classes e.g. KeyedStateBackend)
> > > into
> > > > >> the
> > > > >> > > > exception class diagrams ? Although these state-related
> > > classes
> > > > >> may not
> > > > >> > > > be public, it could be better to consider them together to
> > > make
> > > > >> all
> > > > >> > > > state-related exceptions clear. For example, we could
> > > reorganize
> > > > >> some
> > > > >> > > > existing exceptions such as StateMigrationException, add some
> > > > >> exceptions
> > > > >> > > > such as StateNotFoundException.
> > > > >> > > > 2. Could you clarify or give an example about the extended
> > > > >> relation
> > > > >> > > > "StateAccessException -- StateIOException" ? When do we just
> > > > >> return
> > > > >> > > > StateAccessException instead of StateIOException or others ?
> > > > >> > > > 3. Which version do you want to implement it in ? Since it
> > > has
> > > > >> to break
> > > > >> > > > changes for users who have catched the IOException, If we
> > > want
> > > > >> to implement
> > > > >> > > > it in 1.19, we must mark it very clearly in the release
> > > note, or
> > > > >> we should
> > > > >> > > > make it in 2.0.
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > On Tue, Sep 19, 2023 at 5:08 PM Zakelly Lan <
> > > zakelly.lan@gmail.com>
> > > > >> wrote:
> > > > >> > > >
> > > > >> > > > > Hi everyone,
> > > > >> > > > >
> > > > >> > > > > I would like to initiate a discussion on FLIP-368, which
> > > focuses
> > > > >> on
> > > > >> > > > > reorganizing the exceptions thrown in state interfaces [1].
> > > > >> > > > >
> > > > >> > > > > Currently, we have identified several problems with the
> > > exceptions
> > > > >> > > > > thrown by state-related interfaces:
> > > > >> > > > > 1. The exception types thrown by each interface are
> > > > >> inconsistent.
> > > > >> > > > > While most of the interfaces claim to throw `Exception`, the
> > > > >> > > > > interfaces of `ValueState` throw `IOException`. Additionally,
> > > the
> > > > >> > > > > `State#clear()` never throws an exception. This can be
> > > confusing
> > > > >> for
> > > > >> > > > > users.
> > > > >> > > > > 2. The use of `Exception` or `IOException` as the thrown
> > > > >> exception
> > > > >> > > > > type is too generic and lacks specificity.
> > > > >> > > > > 3. Users may not be able to handle these exceptions. In
> > > cases
> > > > >> where
> > > > >> > > > > an exception occurs while accessing state, the job should
> > > fail.
> > > > >> This
> > > > >> > > > > aligns more with the characteristic of *unchecked exceptions*
> > > > >> instead
> > > > >> > > > > of checked exceptions.
> > > > >> > > > >
> > > > >> > > > > To address these issues, we borrow the idea of throwing
> > > unchecked
> > > > >> > > > > exceptions in Java Collection API and propose the following
> > > > >> changes in
> > > > >> > > > > state-related exceptions:
> > > > >> > > > > 1. Introduction of specific unchecked exception types for
> > > > >> different
> > > > >> > > > > reasons, providing users with more precise information about
> > > the
> > > > >> cause
> > > > >> > > > > of the exception.
> > > > >> > > > > 2. Removal of all checked exceptions from interface
> > > signatures
> > > > >> and
> > > > >> > > > > instead, throwing newly introduced unchecked exceptions in the
> > > > >> > > > > implementations.
> > > > >> > > > >
> > > > >> > > > > Please share your thoughts and suggestions regarding the
> > > proposed
> > > > >> > > > > changes. Thank you for your attention and support.
> > > > >> > > > >
> > > > >> > > > >
> > > > >> > > > > Best,
> > > > >> > > > > Zakelly
> > > > >> > > > >
> > > > >> > > > > [1] FLIP-368: Reorganize the exceptions thrown in state
> > > > >> interfaces,
> > > > >> > > > > https://cwiki.apache.org/confluence/x/eZ2zDw
> > > > >> > > > >
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > --
> > > > >> > > > Best,
> > > > >> > > > Hangxiang.
> > > > >>
> > > > >
> > >
Unless otherwise stated above:
IBM United Kingdom Limited
Registered in England and Wales with number 741598
Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
Unless otherwise stated above:
IBM United Kingdom Limited
Registered in England and Wales with number 741598
Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
RE: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Posted by David Radley <da...@uk.ibm.com>.
Hi ,
The argument seems to be that the errors cannot be acted on so should be runtime exceptions. I want to confirm that none of these errors could / should be retriable. If there is a possibility that the state is available at some time later then I assume a checked retriable Exception would be appropriate for those cases; and be part of the contract with the caller. Can we be sure that there is no possibility that the state will become available; if so then I agree that a runtime Exception is appropriate. What do you think?
Kind regards, David.
From: Zakelly Lan <za...@gmail.com>
Date: Monday, 9 October 2023 at 18:12
To: dev@flink.apache.org <de...@flink.apache.org>
Subject: [EXTERNAL] Re: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Hi everyone,
It seems we're gradually reaching a consensus. So I would like to
start a vote after 72 hours if there are no further discussions.
Please let me know if you have any concerns, thanks!
Best,
Zakelly
On Sat, Oct 7, 2023 at 4:07 PM Zakelly Lan <za...@gmail.com> wrote:
>
> Hi Jing,
>
> Sorry for the late reply! I agree with you that we do not expect users
> to do anything with Flink and we won't "bother" them with those
> exceptions. However, users can still catch the `Throwable` and perform
> any necessary logging activities, similar to how they use Java
> Collection interfaces.
>
>
> Thanks for your insights!
>
> Best,
> Zakelly
>
> On Thu, Sep 21, 2023 at 8:43 PM Jing Ge <ji...@ververica.com.invalid> wrote:
> >
> > Fair enough! Thanks Zakelly for the information. Afaic, even users can do
> > nothing with Flink, they still can do something in their territory, at
> > least doing some logging and metrics stuff, or triggering some other
> > services in their ecosystem. After all, the Flink jobs they build are part
> > of their service component. It didn't change the fact that we are going to
> > use the anti-pattern. Just because we didn't expect users to do
> > anything with Flink, does not mean users don't expect to do something with
> > the expected exception. Anyway, I am open to hearing different opinions.
> >
> > Best regards,
> > Jing
> >
> > On Thu, Sep 21, 2023 at 7:02 AM Zakelly Lan <za...@gmail.com> wrote:
> >
> > > Hi Martijn,
> > >
> > > Thanks for the reminder!
> > >
> > > This FLIP proposes a change to the state API that is annotated as
> > > @PublicEvolving and targets version 1.19. I have clarified this in
> > > the "Proposed Change" section of the FLIP.
> > >
> > >
> > > Hi Jing,
> > >
> > > Thanks for sharing your thoughts! Here are my opinions:
> > >
> > > 1. The exceptions of the state API are usually treated as critical
> > > ones. In other words, if anything goes wrong with state accessing, the
> > > element processing cannot proceed and the job should fail. Flink users
> > > may not know what to do when they encounter these exceptions. I
> > > believe this is the main reason why we want to replace them with
> > > unchecked exceptions.
> > > 2. There have also been some further discussions[1][2] from Stephan
> > > and Shixiaogang below the one you pointed out [3], and it seems they
> > > come to an agreement to use unchecked exceptions. After reviewing the
> > > entire discussion on that PR, I think their arguments are reasonable
> > > given the use case.
> > >
> > > Looking forward to your feedback.
> > >
> > >
> > > Best,
> > > Zakelly
> > >
> > > [1] https://github.com/apache/flink/pull/3380#issuecomment-286807853
> > > [2] https://github.com/apache/flink/pull/3380#issuecomment-286932133
> > > [3] https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > >
> > > On Thu, Sep 21, 2023 at 1:27 AM Jing Ge <ji...@ververica.com.invalid>
> > > wrote:
> > > >
> > > > sorry, typo: It is a known "anti-pattern" instead of "ant-pattern"
> > > >
> > > > Best regards,
> > > > Jing
> > > >
> > > > On Wed, Sep 20, 2023 at 7:23 PM Jing Ge <ji...@ververica.com> wrote:
> > > >
> > > > > Hi Zakelly,
> > > > >
> > > > > Thanks for driving this topic. From good software engineering's
> > > > > perspective, I have different thoughts:
> > > > >
> > > > > 1. The idea to get rid of all checked Exceptions and replace them with
> > > > > unchecked Exceptions is a known ant-pattern: "Generally speaking, do
> > > not
> > > > > throw a RuntimeException or create a subclass of RuntimeException
> > > simply
> > > > > because you don't want to be bothered with specifying the exceptions
> > > your
> > > > > methods can throw." [1] Checked Exceptions mean expected exceptions
> > > that
> > > > > can help developers find a way to catch them and decide what to do. It
> > > is
> > > > > part of the public API signature that can help developers build robust
> > > > > systems. We should not mix concepts and build expected exceptions with
> > > > > unchecked Java Exception classes.
> > > > > 2. The comment Stephan left [2] clearly pointed out that we should
> > > avoid
> > > > > using generic Java Exceptions, and "find some more 'specific'
> > > exceptions
> > > > > for the signature, like throws IOException or throws
> > > StateAccessException."
> > > > > So, the idea is to define/use specific checked Exception classes
> > > instead of
> > > > > using unchecked Exceptions.
> > > > >
> > > > > Looking forward to your thoughts.
> > > > >
> > > > > Best regards,
> > > > > Jing
> > > > >
> > > > >
> > > > > [1]
> > > > >
> > > https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
> > > > > [2] https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > >
> > > > > On Wed, Sep 20, 2023 at 4:52 PM Zakelly Lan <za...@gmail.com>
> > > wrote:
> > > > >
> > > > >> Hi Yanfei,
> > > > >>
> > > > >> Thanks for your reply!
> > > > >>
> > > > >> Yes, this FLIP aims to change all state-related exceptions to
> > > > >> unchecked exceptions and remove all exceptions from the signature. So
> > > > >> I believe we have come to an agreement to keep the interfaces simple.
> > > > >>
> > > > >>
> > > > >> Best regards,
> > > > >> Zakelly
> > > > >>
> > > > >> On Wed, Sep 20, 2023 at 2:26 PM Zakelly Lan <za...@gmail.com>
> > > > >> wrote:
> > > > >> >
> > > > >> > Hi Hangxiang,
> > > > >> >
> > > > >> > Thank you for your response! Here are my thoughts:
> > > > >> >
> > > > >> > 1. Regarding the exceptions thrown by internal interfaces, I suggest
> > > > >> > keeping them as checked exceptions. Since these exceptions will be
> > > > >> > handled by the internal callers, it is meaningful to throw them as
> > > > >> > checked ones. If we need to make changes to these classes, we can
> > > > >> > create separate tickets alongside this FLIP. What are your thoughts
> > > on
> > > > >> > this?
> > > > >> > 2. StateIOException is primarily thrown by file-based state like
> > > > >> > RocksDB, while StateAccessException is more generic and can be
> > > thrown
> > > > >> > by heap states. Additionally, I believe there may be more subclasses
> > > > >> > of StateAccessException that we can add. We can consider this when
> > > > >> > implementing.
> > > > >> > 3. I would like to make this change in version 1.19. As mentioned in
> > > > >> > this FLIP, many users do not catch any exceptions since the element
> > > > >> > processing function exposes the exception to the upper layer.
> > > > >> > Therefore, the impact is manageable. And I completely agree that we
> > > > >> > should clearly document this change in the next release notes.
> > > > >> >
> > > > >> >
> > > > >> > Best regards,
> > > > >> > Zakelly
> > > > >> >
> > > > >> > On Wed, Sep 20, 2023 at 12:35 PM Yanfei Lei <fr...@gmail.com>
> > > > >> wrote:
> > > > >> > >
> > > > >> > > Hi Zakelly,
> > > > >> > >
> > > > >> > > Thanks for bringing this up. +1 for reorganizing.
> > > > >> > >
> > > > >> > > IIUC, this proposal aims to change all state-related exceptions to
> > > > >> > > unchecked exceptions. If users have caught checked exceptions
> > > (such as
> > > > >> > > IOException ) in their code, leaving the code as is would also
> > > work.
> > > > >> > >
> > > > >> > > Is it possible not to put any exceptions in the signature of
> > > > >> > > user-facing interfaces? As the proposal mentioned, users can do a
> > > few
> > > > >> > > things even if they catch the exceptions. Keeping the interface
> > > simple
> > > > >> > > may be easier to understand.
> > > > >> > >
> > > > >> > >
> > > > >> > > Best,
> > > > >> > > Yanfei
> > > > >> > >
> > > > >> > > Hangxiang Yu <ma...@gmail.com> 于2023年9月19日周二 18:16写道：
> > > > >> > > >
> > > > >> > > > Hi, Zakelly.
> > > > >> > > >
> > > > >> > > > Thanks for the proposal.
> > > > >> > > >
> > > > >> > > > +1 for reorganizing exceptions of state interfaces which indeed
> > > > >> confuses me
> > > > >> > > > currently.
> > > > >> > > >
> > > > >> > > > From my experience, users usually omit these exceptions because
> > > > >> they cannot
> > > > >> > > > do much even if they catch the exceptions.
> > > > >> > > >
> > > > >> > > > I have some problems and suggestions, PTAL:
> > > > >> > > >
> > > > >> > > > 1. Could we also reorganize or add more state exceptions
> > > (may be
> > > > >> related
> > > > >> > > > to other state interfaces/classes e.g. KeyedStateBackend)
> > > into
> > > > >> the
> > > > >> > > > exception class diagrams ? Although these state-related
> > > classes
> > > > >> may not
> > > > >> > > > be public, it could be better to consider them together to
> > > make
> > > > >> all
> > > > >> > > > state-related exceptions clear. For example, we could
> > > reorganize
> > > > >> some
> > > > >> > > > existing exceptions such as StateMigrationException, add some
> > > > >> exceptions
> > > > >> > > > such as StateNotFoundException.
> > > > >> > > > 2. Could you clarify or give an example about the extended
> > > > >> relation
> > > > >> > > > "StateAccessException -- StateIOException" ? When do we just
> > > > >> return
> > > > >> > > > StateAccessException instead of StateIOException or others ?
> > > > >> > > > 3. Which version do you want to implement it in ? Since it
> > > has
> > > > >> to break
> > > > >> > > > changes for users who have catched the IOException, If we
> > > want
> > > > >> to implement
> > > > >> > > > it in 1.19, we must mark it very clearly in the release
> > > note, or
> > > > >> we should
> > > > >> > > > make it in 2.0.
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > On Tue, Sep 19, 2023 at 5:08 PM Zakelly Lan <
> > > zakelly.lan@gmail.com>
> > > > >> wrote:
> > > > >> > > >
> > > > >> > > > > Hi everyone,
> > > > >> > > > >
> > > > >> > > > > I would like to initiate a discussion on FLIP-368, which
> > > focuses
> > > > >> on
> > > > >> > > > > reorganizing the exceptions thrown in state interfaces [1].
> > > > >> > > > >
> > > > >> > > > > Currently, we have identified several problems with the
> > > exceptions
> > > > >> > > > > thrown by state-related interfaces:
> > > > >> > > > > 1. The exception types thrown by each interface are
> > > > >> inconsistent.
> > > > >> > > > > While most of the interfaces claim to throw `Exception`, the
> > > > >> > > > > interfaces of `ValueState` throw `IOException`. Additionally,
> > > the
> > > > >> > > > > `State#clear()` never throws an exception. This can be
> > > confusing
> > > > >> for
> > > > >> > > > > users.
> > > > >> > > > > 2. The use of `Exception` or `IOException` as the thrown
> > > > >> exception
> > > > >> > > > > type is too generic and lacks specificity.
> > > > >> > > > > 3. Users may not be able to handle these exceptions. In
> > > cases
> > > > >> where
> > > > >> > > > > an exception occurs while accessing state, the job should
> > > fail.
> > > > >> This
> > > > >> > > > > aligns more with the characteristic of *unchecked exceptions*
> > > > >> instead
> > > > >> > > > > of checked exceptions.
> > > > >> > > > >
> > > > >> > > > > To address these issues, we borrow the idea of throwing
> > > unchecked
> > > > >> > > > > exceptions in Java Collection API and propose the following
> > > > >> changes in
> > > > >> > > > > state-related exceptions:
> > > > >> > > > > 1. Introduction of specific unchecked exception types for
> > > > >> different
> > > > >> > > > > reasons, providing users with more precise information about
> > > the
> > > > >> cause
> > > > >> > > > > of the exception.
> > > > >> > > > > 2. Removal of all checked exceptions from interface
> > > signatures
> > > > >> and
> > > > >> > > > > instead, throwing newly introduced unchecked exceptions in the
> > > > >> > > > > implementations.
> > > > >> > > > >
> > > > >> > > > > Please share your thoughts and suggestions regarding the
> > > proposed
> > > > >> > > > > changes. Thank you for your attention and support.
> > > > >> > > > >
> > > > >> > > > >
> > > > >> > > > > Best,
> > > > >> > > > > Zakelly
> > > > >> > > > >
> > > > >> > > > > [1] FLIP-368: Reorganize the exceptions thrown in state
> > > > >> interfaces,
> > > > >> > > > > https://cwiki.apache.org/confluence/x/eZ2zDw
> > > > >> > > > >
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > --
> > > > >> > > > Best,
> > > > >> > > > Hangxiang.
> > > > >>
> > > > >
> > >
Unless otherwise stated above:
IBM United Kingdom Limited
Registered in England and Wales with number 741598
Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
Re: [DISCUSS] FLIP-368 Reorganize the exceptions thrown in state interfaces
Posted by Zakelly Lan <za...@gmail.com>.
Hi everyone,
It seems we're gradually reaching a consensus. So I would like to
start a vote after 72 hours if there are no further discussions.
Please let me know if you have any concerns, thanks!
Best,
Zakelly
On Sat, Oct 7, 2023 at 4:07 PM Zakelly Lan <za...@gmail.com> wrote:
>
> Hi Jing,
>
> Sorry for the late reply! I agree with you that we do not expect users
> to do anything with Flink and we won't "bother" them with those
> exceptions. However, users can still catch the `Throwable` and perform
> any necessary logging activities, similar to how they use Java
> Collection interfaces.
>
>
> Thanks for your insights!
>
> Best,
> Zakelly
>
> On Thu, Sep 21, 2023 at 8:43 PM Jing Ge <ji...@ververica.com.invalid> wrote:
> >
> > Fair enough! Thanks Zakelly for the information. Afaic, even users can do
> > nothing with Flink, they still can do something in their territory, at
> > least doing some logging and metrics stuff, or triggering some other
> > services in their ecosystem. After all, the Flink jobs they build are part
> > of their service component. It didn't change the fact that we are going to
> > use the anti-pattern. Just because we didn't expect users to do
> > anything with Flink, does not mean users don't expect to do something with
> > the expected exception. Anyway, I am open to hearing different opinions.
> >
> > Best regards,
> > Jing
> >
> > On Thu, Sep 21, 2023 at 7:02 AM Zakelly Lan <za...@gmail.com> wrote:
> >
> > > Hi Martijn,
> > >
> > > Thanks for the reminder!
> > >
> > > This FLIP proposes a change to the state API that is annotated as
> > > @PublicEvolving and targets version 1.19. I have clarified this in
> > > the "Proposed Change" section of the FLIP.
> > >
> > >
> > > Hi Jing,
> > >
> > > Thanks for sharing your thoughts! Here are my opinions:
> > >
> > > 1. The exceptions of the state API are usually treated as critical
> > > ones. In other words, if anything goes wrong with state accessing, the
> > > element processing cannot proceed and the job should fail. Flink users
> > > may not know what to do when they encounter these exceptions. I
> > > believe this is the main reason why we want to replace them with
> > > unchecked exceptions.
> > > 2. There have also been some further discussions[1][2] from Stephan
> > > and Shixiaogang below the one you pointed out [3], and it seems they
> > > come to an agreement to use unchecked exceptions. After reviewing the
> > > entire discussion on that PR, I think their arguments are reasonable
> > > given the use case.
> > >
> > > Looking forward to your feedback.
> > >
> > >
> > > Best,
> > > Zakelly
> > >
> > > [1] https://github.com/apache/flink/pull/3380#issuecomment-286807853
> > > [2] https://github.com/apache/flink/pull/3380#issuecomment-286932133
> > > [3] https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > >
> > > On Thu, Sep 21, 2023 at 1:27 AM Jing Ge <ji...@ververica.com.invalid>
> > > wrote:
> > > >
> > > > sorry, typo: It is a known "anti-pattern" instead of "ant-pattern"
> > > >
> > > > Best regards,
> > > > Jing
> > > >
> > > > On Wed, Sep 20, 2023 at 7:23 PM Jing Ge <ji...@ververica.com> wrote:
> > > >
> > > > > Hi Zakelly,
> > > > >
> > > > > Thanks for driving this topic. From good software engineering's
> > > > > perspective, I have different thoughts:
> > > > >
> > > > > 1. The idea to get rid of all checked Exceptions and replace them with
> > > > > unchecked Exceptions is a known ant-pattern: "Generally speaking, do
> > > not
> > > > > throw a RuntimeException or create a subclass of RuntimeException
> > > simply
> > > > > because you don't want to be bothered with specifying the exceptions
> > > your
> > > > > methods can throw." [1] Checked Exceptions mean expected exceptions
> > > that
> > > > > can help developers find a way to catch them and decide what to do. It
> > > is
> > > > > part of the public API signature that can help developers build robust
> > > > > systems. We should not mix concepts and build expected exceptions with
> > > > > unchecked Java Exception classes.
> > > > > 2. The comment Stephan left [2] clearly pointed out that we should
> > > avoid
> > > > > using generic Java Exceptions, and "find some more 'specific'
> > > exceptions
> > > > > for the signature, like throws IOException or throws
> > > StateAccessException."
> > > > > So, the idea is to define/use specific checked Exception classes
> > > instead of
> > > > > using unchecked Exceptions.
> > > > >
> > > > > Looking forward to your thoughts.
> > > > >
> > > > > Best regards,
> > > > > Jing
> > > > >
> > > > >
> > > > > [1]
> > > > >
> > > https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
> > > > > [2] https://github.com/apache/flink/pull/3380#issuecomment-281631160
> > > > >
> > > > > On Wed, Sep 20, 2023 at 4:52 PM Zakelly Lan <za...@gmail.com>
> > > wrote:
> > > > >
> > > > >> Hi Yanfei,
> > > > >>
> > > > >> Thanks for your reply!
> > > > >>
> > > > >> Yes, this FLIP aims to change all state-related exceptions to
> > > > >> unchecked exceptions and remove all exceptions from the signature. So
> > > > >> I believe we have come to an agreement to keep the interfaces simple.
> > > > >>
> > > > >>
> > > > >> Best regards,
> > > > >> Zakelly
> > > > >>
> > > > >> On Wed, Sep 20, 2023 at 2:26 PM Zakelly Lan <za...@gmail.com>
> > > > >> wrote:
> > > > >> >
> > > > >> > Hi Hangxiang,
> > > > >> >
> > > > >> > Thank you for your response! Here are my thoughts:
> > > > >> >
> > > > >> > 1. Regarding the exceptions thrown by internal interfaces, I suggest
> > > > >> > keeping them as checked exceptions. Since these exceptions will be
> > > > >> > handled by the internal callers, it is meaningful to throw them as
> > > > >> > checked ones. If we need to make changes to these classes, we can
> > > > >> > create separate tickets alongside this FLIP. What are your thoughts
> > > on
> > > > >> > this?
> > > > >> > 2. StateIOException is primarily thrown by file-based state like
> > > > >> > RocksDB, while StateAccessException is more generic and can be
> > > thrown
> > > > >> > by heap states. Additionally, I believe there may be more subclasses
> > > > >> > of StateAccessException that we can add. We can consider this when
> > > > >> > implementing.
> > > > >> > 3. I would like to make this change in version 1.19. As mentioned in
> > > > >> > this FLIP, many users do not catch any exceptions since the element
> > > > >> > processing function exposes the exception to the upper layer.
> > > > >> > Therefore, the impact is manageable. And I completely agree that we
> > > > >> > should clearly document this change in the next release notes.
> > > > >> >
> > > > >> >
> > > > >> > Best regards,
> > > > >> > Zakelly
> > > > >> >
> > > > >> > On Wed, Sep 20, 2023 at 12:35 PM Yanfei Lei <fr...@gmail.com>
> > > > >> wrote:
> > > > >> > >
> > > > >> > > Hi Zakelly,
> > > > >> > >
> > > > >> > > Thanks for bringing this up. +1 for reorganizing.
> > > > >> > >
> > > > >> > > IIUC, this proposal aims to change all state-related exceptions to
> > > > >> > > unchecked exceptions. If users have caught checked exceptions
> > > (such as
> > > > >> > > IOException ) in their code, leaving the code as is would also
> > > work.
> > > > >> > >
> > > > >> > > Is it possible not to put any exceptions in the signature of
> > > > >> > > user-facing interfaces? As the proposal mentioned, users can do a
> > > few
> > > > >> > > things even if they catch the exceptions. Keeping the interface
> > > simple
> > > > >> > > may be easier to understand.
> > > > >> > >
> > > > >> > >
> > > > >> > > Best,
> > > > >> > > Yanfei
> > > > >> > >
> > > > >> > > Hangxiang Yu <ma...@gmail.com> 于2023年9月19日周二 18:16写道：
> > > > >> > > >
> > > > >> > > > Hi, Zakelly.
> > > > >> > > >
> > > > >> > > > Thanks for the proposal.
> > > > >> > > >
> > > > >> > > > +1 for reorganizing exceptions of state interfaces which indeed
> > > > >> confuses me
> > > > >> > > > currently.
> > > > >> > > >
> > > > >> > > > From my experience, users usually omit these exceptions because
> > > > >> they cannot
> > > > >> > > > do much even if they catch the exceptions.
> > > > >> > > >
> > > > >> > > > I have some problems and suggestions, PTAL:
> > > > >> > > >
> > > > >> > > > 1. Could we also reorganize or add more state exceptions
> > > (may be
> > > > >> related
> > > > >> > > > to other state interfaces/classes e.g. KeyedStateBackend)
> > > into
> > > > >> the
> > > > >> > > > exception class diagrams ? Although these state-related
> > > classes
> > > > >> may not
> > > > >> > > > be public, it could be better to consider them together to
> > > make
> > > > >> all
> > > > >> > > > state-related exceptions clear. For example, we could
> > > reorganize
> > > > >> some
> > > > >> > > > existing exceptions such as StateMigrationException, add some
> > > > >> exceptions
> > > > >> > > > such as StateNotFoundException.
> > > > >> > > > 2. Could you clarify or give an example about the extended
> > > > >> relation
> > > > >> > > > "StateAccessException -- StateIOException" ? When do we just
> > > > >> return
> > > > >> > > > StateAccessException instead of StateIOException or others ?
> > > > >> > > > 3. Which version do you want to implement it in ? Since it
> > > has
> > > > >> to break
> > > > >> > > > changes for users who have catched the IOException, If we
> > > want
> > > > >> to implement
> > > > >> > > > it in 1.19, we must mark it very clearly in the release
> > > note, or
> > > > >> we should
> > > > >> > > > make it in 2.0.
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > On Tue, Sep 19, 2023 at 5:08 PM Zakelly Lan <
> > > zakelly.lan@gmail.com>
> > > > >> wrote:
> > > > >> > > >
> > > > >> > > > > Hi everyone,
> > > > >> > > > >
> > > > >> > > > > I would like to initiate a discussion on FLIP-368, which
> > > focuses
> > > > >> on
> > > > >> > > > > reorganizing the exceptions thrown in state interfaces [1].
> > > > >> > > > >
> > > > >> > > > > Currently, we have identified several problems with the
> > > exceptions
> > > > >> > > > > thrown by state-related interfaces:
> > > > >> > > > > 1. The exception types thrown by each interface are
> > > > >> inconsistent.
> > > > >> > > > > While most of the interfaces claim to throw `Exception`, the
> > > > >> > > > > interfaces of `ValueState` throw `IOException`. Additionally,
> > > the
> > > > >> > > > > `State#clear()` never throws an exception. This can be
> > > confusing
> > > > >> for
> > > > >> > > > > users.
> > > > >> > > > > 2. The use of `Exception` or `IOException` as the thrown
> > > > >> exception
> > > > >> > > > > type is too generic and lacks specificity.
> > > > >> > > > > 3. Users may not be able to handle these exceptions. In
> > > cases
> > > > >> where
> > > > >> > > > > an exception occurs while accessing state, the job should
> > > fail.
> > > > >> This
> > > > >> > > > > aligns more with the characteristic of *unchecked exceptions*
> > > > >> instead
> > > > >> > > > > of checked exceptions.
> > > > >> > > > >
> > > > >> > > > > To address these issues, we borrow the idea of throwing
> > > unchecked
> > > > >> > > > > exceptions in Java Collection API and propose the following
> > > > >> changes in
> > > > >> > > > > state-related exceptions:
> > > > >> > > > > 1. Introduction of specific unchecked exception types for
> > > > >> different
> > > > >> > > > > reasons, providing users with more precise information about
> > > the
> > > > >> cause
> > > > >> > > > > of the exception.
> > > > >> > > > > 2. Removal of all checked exceptions from interface
> > > signatures
> > > > >> and
> > > > >> > > > > instead, throwing newly introduced unchecked exceptions in the
> > > > >> > > > > implementations.
> > > > >> > > > >
> > > > >> > > > > Please share your thoughts and suggestions regarding the
> > > proposed
> > > > >> > > > > changes. Thank you for your attention and support.
> > > > >> > > > >
> > > > >> > > > >
> > > > >> > > > > Best,
> > > > >> > > > > Zakelly
> > > > >> > > > >
> > > > >> > > > > [1] FLIP-368: Reorganize the exceptions thrown in state
> > > > >> interfaces,
> > > > >> > > > > https://cwiki.apache.org/confluence/x/eZ2zDw
> > > > >> > > > >
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > --
> > > > >> > > > Best,
> > > > >> > > > Hangxiang.
> > > > >>
> > > > >
> > >