Hi Jinzhong, Thanks for driving this! 1. I'm wondering if `ConfigurableRocksDBOptionsFactory` will be used by users, currently it looks like only developers use it in rocksdb state backend module. And Its only non-testing subclass "DefaultConfigurableOptionsFactory" is marked @Deprecated. 2. Regarding @Internal, according to the comments, it is used for "Annotation to mark methods within stable, public APIs as an internal developer API." So marking "SingleStateIterator" and "RocksDBRestoreOperation" as @Internal is acceptable for me.
Best, Yanfei Jinzhong Li <lijinzhong2...@gmail.com> 于2024年1月25日周四 12:16写道： > > Hi Zakelly, > > Thanks for your comments! > > 1）I agree that almost no user would use "RocksDBStateUploader" and > "RocksDBStateDownloader" to do something. It's fine for me to keep them > unmarked. > 2）Regarding "SingleStateIterator", I think it's acceptable to either leave > it unmarked or mark it as @Internal. I just consider that > SingleStateIterator is one interface with the "public" modifier and it is > harmless to annotate it as @Internal. > > > > > Hi Hangxiang, > > Thanks for the reminder! > > It makes sense to mark RocksDBStateBackendFactory as Deprecated. > > Best, > Jinzhong Li > > > On Thu, Jan 25, 2024 at 10:22 AM Hangxiang Yu <master...@gmail.com> wrote: > > > Hi Jinzhong. > > Thanks for driving this! > > Some suggestions: > > 1. As RocksDBStateBackend marked as Deprecated, We should also > > mark RocksDBStateBackendFactory as Deprecated > > 2. Since 1.19 will be freezed in 1.26. Let's adjust the target version to > > 1.20 > > > > > > On Wed, Jan 24, 2024 at 11:50 PM Zakelly Lan <zakelly....@gmail.com> > > wrote: > > > > > Hi Jinzhong, > > > > > > Thanks for driving this! +1 for fixing the lack of annotation. > > > > > > I'm wondering if we really need to annotate *RocksDBStateUploader* and > > > *RocksDBStateDownloader > > > *with @Internal, as they seem to be ordinary classes without interacting > > > with other modules. > > > Also, I have reservations about annotating *SingleStateIterator*, but I'd > > > like to hear others' opinions and won't insist on this. > > > > > > Best, > > > Zakelly > > > > > > On Wed, Jan 24, 2024 at 10:26 PM Jinzhong Li <lijinzhong2...@gmail.com> > > > wrote: > > > > > > > Hi devs, > > > > > > > > I’m opening this thread to discuss about FLIP-420: Add API annotations > > > for > > > > RocksDB StateBackend user-facing classes[1]. > > > > > > > > As described in FLINK-18255[2] , several user-facing classes in > > > > flink-statebackend-rocksdb module don't have any API annotations, not > > > even > > > > @PublicEvolving. This FLIP will add annotations for them to clarify > > their > > > > usage. > > > > > > > > Looking forward to hearing from you, thanks! > > > > > > > > > > > > Best regards, > > > > Jinzhong Li > > > > > > > > > > > > [1] > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-420%3A+Add+API+annotations+for+RocksDB+StateBackend+user-facing+classes > > > > [2] https://issues.apache.org/jira/browse/FLINK-18255 > > > > > > > > > > > > > -- > > Best, > > Hangxiang. > >
