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. > > > >> > > > > > >