Hi Zakelly, Thank you for the proposal! Please find my comments below.
1. The name `emptyFuture` seems a little unintuitive, and it is hard to understand in what use case the `emptyFuture` should be used. If I understand correctly, it is similar to the FutureUtils#completedVoidFuture. How about naming it completedVoidStateFuture? 2. IIUC, the `FutureUtils` is intended to be used by the user. If that's the case, `FutureUtils` should be annotated as a public interface, such as `PublicEvolving`. 3. The state classes, such as `ValueState`, `ListState`, etc., are essential for users, and we should add JavaDocs to those classes and their methods. Best regards, Xuanna On Mon, Mar 11, 2024 at 4:21 PM Yunfeng Zhou <flink.zhouyunf...@gmail.com> wrote: > > Hi Zakelly, > > Thanks for the proposal! The structure of the Async API generally > looks good to me. Some comments on the details of the design are as > follows. > > +1 for JingGe's suggestion to introduce an AsyncState API, instead of > having both get() and asyncGet() in the same State class. As a > supplement to its benefits, this design could help avoid having users > to use sync and async API in a mixed way (unless they create both a > State and an AsyncState from the same state descriptor), which is > supposed to bring suboptimal performance according to the FLIP's > description. > > I noticed that the FLIP proposes to place the newly introduced API in > the package "org.apache.flink.api.common.state.v2", which seems a > little strange to me as there has not been such a naming pattern > ".v2." for packages in Flink. I would suggest discussing this topic > with the main authors of Datastream V2, like Weijie Guo, so that the > newly introduced APIs from both sides comply with a unified naming > style. If we reach an agreement on the first comment, my personal idea > is that we can place the AsyncState interfaces to > "org.apache.flink.api.common.state.async", and the existing state APIs > to "org.apache.flink.api.common.state" or > "org.apache.flink.api.common.state.sync". > > Best regards, > Yunfeng Zhou > > On Thu, Mar 7, 2024 at 4:48 PM Zakelly Lan <zakelly....@gmail.com> wrote: > > > > Hi devs, > > > > I'd like to start a discussion on a sub-FLIP of FLIP-423: Disaggregated > > State Storage and Management[1], which is a joint work of Yuan Mei, Zakelly > > Lan, Jinzhong Li, Hangxiang Yu, Yanfei Lei and Feng Wang: > > > > - FLIP-424: Asynchronous State APIs [2] > > > > This FLIP introduces new APIs for asynchronous state access. > > > > Please make sure you have read the FLIP-423[1] to know the whole story, and > > we'll discuss the details of FLIP-424[2] under this mail. For the > > discussion of overall architecture or topics related with multiple > > sub-FLIPs, please post in the previous mail[3]. > > > > Looking forward to hearing from you! > > > > [1] https://cwiki.apache.org/confluence/x/R4p3EQ > > [2] https://cwiki.apache.org/confluence/x/SYp3EQ > > [3] https://lists.apache.org/thread/ct8smn6g9y0b8730z7rp9zfpnwmj8vf0 > > > > > > Best, > > Zakelly
