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