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