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 <david_rad...@uk.ibm.com>
Date: Tuesday, 10 October 2023 at 09:49
To: dev@flink.apache.org <dev@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 <zakelly....@gmail.com>
Date: Monday, 9 October 2023 at 18:12
To: dev@flink.apache.org <dev@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....@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 <j...@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....@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 <j...@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 <j...@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....@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....@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 <fredia...@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 <master...@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....@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
