You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by Jinzhong Li <li...@gmail.com> on 2024/01/24 14:24:40 UTC
[DISCUSS]FLIP-420: Add API annotations for RocksDB StateBackend user-facing classes
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
Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB StateBackend user-facing classes
Posted by Jinzhong Li <li...@gmail.com>.
Hi Jing,
Thanks for your feedback!
> I am fine with @Internal and once we
> use @Internal, we should add it to all classes within a module to keep the
> consistency.
I think the scope to keep consistency within a module is all interfaces,
not all classes.
For example, in changelog-statebackend module or flink-runtime/state/heap
package (hashmap-statebackend), all interfaces are annotated, and classes'
annotations are optional.
The changes in this FLIP(Flip-420) also covers all interfaces of the
rocksdb statebackend module.
Best,
Jinzhong
On Wed, Mar 13, 2024 at 7:30 AM Jing Ge <ji...@ververica.com.invalid> wrote:
> Hi Jinzhong,
>
> Thanks for your clarification!
>
>
> > As mentioned in Flink's “API compatibility guarantees”[1], "any API
> without
> > such an annotation is considered internal to Flink, with no guarantees
> > being provided."
> > I think these interfaces are treated as internal interfaces regardless of
> > whether they are marked @Internal.
> >
> > So, it's acceptable for me to either leave it unmarked or mark it as
> > @Internal.
> >
>
> It is a question of consistency. I am fine with @Internal and once we
> use @Internal, we should add it to all classes within a module to keep the
> consistency. Otherwise, when developers read some interfaces
> have @Internal, some other classes have no annotations. It turns out there
> are different meanings of with or without @Internal. But it is not a big
> deal. If no other one has the same concern. I will not block you :-)
>
>
>
> > As for FLIP-197, I found that the proposal in this flip has not actually
> > been implemented. Because FLINK-25352 [2] has not been resolved，it looks
> > like we can't add more info into the PublicEvolving annotation.
> >
> >
> Fair enough! Thanks for the hint!
>
> Best regards,
> Jing
>
>
>
> On Mon, Mar 11, 2024 at 9:03 AM Jinzhong Li <li...@gmail.com>
> wrote:
>
> > Hi Jing.
> >
> > Thanks for your suggestion.
> >
> > >> I was wondering if SingleStateIterator and RocksDBRestoreOperation
> are
> > exposed to users even if they are interfaces.
> >
> > I agree that very very few users implement these two interfaces, except
> for
> > some advanced users who implement their own StateBackend based on
> > rocksdbStateBackend. (The "StateBackend" and
> "EmbeddedRocksDBStateBackend"
> > are PublicEvolving interface)
> >
> > >> Only adding @Internal annotation to these two interfaces could be
> > considered as an implicit hint that users can replace the default
> behaviour
> > with their own implementation.
>
>
>
> > As mentioned in Flink's “API compatibility guarantees”[1], "any API
> without
> > such an annotation is considered internal to Flink, with no guarantees
> > being provided."
> > I think these interfaces are treated as internal interfaces regardless of
> > whether they are marked @Internal.
> >
> > So, it's acceptable for me to either leave it unmarked or mark it as
> > @Internal.
> >
>
>
>
> > >> Another suggestion is that we should start following FLIP-197[1](which
> > is already accepted by the community) to add more info into the
> > PublicEvolving annotation in order to kick off the graduation process.
> >
> >
>
> > As for FLIP-197, I found that the proposal in this flip has not actually
> > been implemented. Because FLINK-25352 [2] has not been resolved，it looks
> > like we can't add more info into the PublicEvolving annotation.
> >
> >
>
> > [1]
> >
> >
> https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees
> > [2] https://issues.apache.org/jira/browse/FLINK-25352
> >
> > Best,
> > Jinzhong
> >
> >
> > On Mon, Mar 11, 2024 at 12:29 AM Jing Ge <ji...@ververica.com.invalid>
> > wrote:
> >
> > > Hi Jinzhong,
> > >
> > > Sorry for the late reply. The key guideline of adding visibility
> > annotation
> > > is whether the interface/class is a customer-facing API. I was
> wondering
> > if
> > > SingleStateIterator and RocksDBRestoreOperation are exposed to users
> even
> > > if they are interfaces, i.e. users can use their own implementation
> > > classes. The flink-statebackend-rocksdb module has many more classes
> than
> > > the FLIP described. Only adding @Internal annotation to these two
> > > interfaces could be considered as an implicit hint that users can
> replace
> > > the default behaviour with their own implementation.
> > >
> > > Another suggestion is that we should start following FLIP-197[1](which
> is
> > > already accepted by the community) to add more info into the
> > PublicEvolving
> > > annotation in order to kick off the graduation process. WDYT?
> > >
> > > Best regards,
> > > Jing
> > >
> > > [1]
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-197%3A+API+stability+graduation+process
> > >
> > > On Mon, Feb 26, 2024 at 2:22 PM Jinzhong Li <li...@gmail.com>
> > > wrote:
> > >
> > > > Hi all,
> > > >
> > > > Thanks for all the feedback. It seems there are no more questions
> > > > unaddressed. I would like to open the voting thread after three
> days.
> > > >
> > > > Please let me know if you have any concerns, thanks!
> > > >
> > > > Best,
> > > > Jinzhong Li
> > > >
> > > > On Mon, Feb 26, 2024 at 11:29 AM Yanfei Lei <fr...@gmail.com>
> > wrote:
> > > >
> > > > > @Yun Tang
> > > > > Thanks for the information, +1 for marking
> > > > > `ConfigurableRocksDBOptionsFactory` as `PublicEvolving `.
> > > > >
> > > > > Best,
> > > > > Yanfei
> > > > >
> > > > > Yun Tang <my...@live.com> 于2024年2月23日周五 19:54写道：
> > > > > >
> > > > > > Hi Jinzhong,
> > > > > >
> > > > > > Thanks for driving this topic, and +1 for fixing the lack of
> > > > annotation.
> > > > > >
> > > > > > @Yanfei the `ConfigurableRocksDBOptionsFactory` interface is
> > > introduced
> > > > > for user extension, you can refer to the doc[1], which shows an
> > example
> > > > of
> > > > > how to use this interface.
> > > > > >
> > > > > >
> > > > > > [1]
> > > > >
> > > >
> > >
> >
> https://nightlies.apache.org/flink/flink-docs-master/docs/ops/state/large_state_tuning/#tuning-rocksdb-memory
> > > > > >
> > > > > >
> > > > > > Best
> > > > > > Yun Tang
> > > > > > ________________________________
> > > > > > From: Yanfei Lei <fr...@gmail.com>
> > > > > > Sent: Thursday, February 22, 2024 15:39
> > > > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > > > Subject: Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB
> > > > > StateBackend user-facing classes
> > > > > >
> > > > > > 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 <li...@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 <
> > masteryhx@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.lan@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 <
> > > > > lijinzhong2051@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.
> > > > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB StateBackend user-facing classes
Posted by Jing Ge <ji...@ververica.com.INVALID>.
Hi Jinzhong,
Thanks for your clarification!
> As mentioned in Flink's “API compatibility guarantees”[1], "any API without
> such an annotation is considered internal to Flink, with no guarantees
> being provided."
> I think these interfaces are treated as internal interfaces regardless of
> whether they are marked @Internal.
>
> So, it's acceptable for me to either leave it unmarked or mark it as
> @Internal.
>
It is a question of consistency. I am fine with @Internal and once we
use @Internal, we should add it to all classes within a module to keep the
consistency. Otherwise, when developers read some interfaces
have @Internal, some other classes have no annotations. It turns out there
are different meanings of with or without @Internal. But it is not a big
deal. If no other one has the same concern. I will not block you :-)
> As for FLIP-197, I found that the proposal in this flip has not actually
> been implemented. Because FLINK-25352 [2] has not been resolved，it looks
> like we can't add more info into the PublicEvolving annotation.
>
>
Fair enough! Thanks for the hint!
Best regards,
Jing
On Mon, Mar 11, 2024 at 9:03 AM Jinzhong Li <li...@gmail.com>
wrote:
> Hi Jing.
>
> Thanks for your suggestion.
>
> >> I was wondering if SingleStateIterator and RocksDBRestoreOperation are
> exposed to users even if they are interfaces.
>
> I agree that very very few users implement these two interfaces, except for
> some advanced users who implement their own StateBackend based on
> rocksdbStateBackend. (The "StateBackend" and "EmbeddedRocksDBStateBackend"
> are PublicEvolving interface)
>
> >> Only adding @Internal annotation to these two interfaces could be
> considered as an implicit hint that users can replace the default behaviour
> with their own implementation.
> As mentioned in Flink's “API compatibility guarantees”[1], "any API without
> such an annotation is considered internal to Flink, with no guarantees
> being provided."
> I think these interfaces are treated as internal interfaces regardless of
> whether they are marked @Internal.
>
> So, it's acceptable for me to either leave it unmarked or mark it as
> @Internal.
>
> >> Another suggestion is that we should start following FLIP-197[1](which
> is already accepted by the community) to add more info into the
> PublicEvolving annotation in order to kick off the graduation process.
>
>
> As for FLIP-197, I found that the proposal in this flip has not actually
> been implemented. Because FLINK-25352 [2] has not been resolved，it looks
> like we can't add more info into the PublicEvolving annotation.
>
>
> [1]
>
> https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees
> [2] https://issues.apache.org/jira/browse/FLINK-25352
>
> Best,
> Jinzhong
>
>
> On Mon, Mar 11, 2024 at 12:29 AM Jing Ge <ji...@ververica.com.invalid>
> wrote:
>
> > Hi Jinzhong,
> >
> > Sorry for the late reply. The key guideline of adding visibility
> annotation
> > is whether the interface/class is a customer-facing API. I was wondering
> if
> > SingleStateIterator and RocksDBRestoreOperation are exposed to users even
> > if they are interfaces, i.e. users can use their own implementation
> > classes. The flink-statebackend-rocksdb module has many more classes than
> > the FLIP described. Only adding @Internal annotation to these two
> > interfaces could be considered as an implicit hint that users can replace
> > the default behaviour with their own implementation.
> >
> > Another suggestion is that we should start following FLIP-197[1](which is
> > already accepted by the community) to add more info into the
> PublicEvolving
> > annotation in order to kick off the graduation process. WDYT?
> >
> > Best regards,
> > Jing
> >
> > [1]
> >
> >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-197%3A+API+stability+graduation+process
> >
> > On Mon, Feb 26, 2024 at 2:22 PM Jinzhong Li <li...@gmail.com>
> > wrote:
> >
> > > Hi all,
> > >
> > > Thanks for all the feedback. It seems there are no more questions
> > > unaddressed. I would like to open the voting thread after three days.
> > >
> > > Please let me know if you have any concerns, thanks!
> > >
> > > Best,
> > > Jinzhong Li
> > >
> > > On Mon, Feb 26, 2024 at 11:29 AM Yanfei Lei <fr...@gmail.com>
> wrote:
> > >
> > > > @Yun Tang
> > > > Thanks for the information, +1 for marking
> > > > `ConfigurableRocksDBOptionsFactory` as `PublicEvolving `.
> > > >
> > > > Best,
> > > > Yanfei
> > > >
> > > > Yun Tang <my...@live.com> 于2024年2月23日周五 19:54写道：
> > > > >
> > > > > Hi Jinzhong,
> > > > >
> > > > > Thanks for driving this topic, and +1 for fixing the lack of
> > > annotation.
> > > > >
> > > > > @Yanfei the `ConfigurableRocksDBOptionsFactory` interface is
> > introduced
> > > > for user extension, you can refer to the doc[1], which shows an
> example
> > > of
> > > > how to use this interface.
> > > > >
> > > > >
> > > > > [1]
> > > >
> > >
> >
> https://nightlies.apache.org/flink/flink-docs-master/docs/ops/state/large_state_tuning/#tuning-rocksdb-memory
> > > > >
> > > > >
> > > > > Best
> > > > > Yun Tang
> > > > > ________________________________
> > > > > From: Yanfei Lei <fr...@gmail.com>
> > > > > Sent: Thursday, February 22, 2024 15:39
> > > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > > Subject: Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB
> > > > StateBackend user-facing classes
> > > > >
> > > > > 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 <li...@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 <
> masteryhx@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.lan@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 <
> > > > lijinzhong2051@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.
> > > > > > >
> > > >
> > >
> >
>
Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB StateBackend user-facing classes
Posted by Jinzhong Li <li...@gmail.com>.
Hi Jing.
Thanks for your suggestion.
>> I was wondering if SingleStateIterator and RocksDBRestoreOperation are
exposed to users even if they are interfaces.
I agree that very very few users implement these two interfaces, except for
some advanced users who implement their own StateBackend based on
rocksdbStateBackend. (The "StateBackend" and "EmbeddedRocksDBStateBackend"
are PublicEvolving interface)
>> Only adding @Internal annotation to these two interfaces could be
considered as an implicit hint that users can replace the default behaviour
with their own implementation.
As mentioned in Flink's “API compatibility guarantees”[1], "any API without
such an annotation is considered internal to Flink, with no guarantees
being provided."
I think these interfaces are treated as internal interfaces regardless of
whether they are marked @Internal.
So, it's acceptable for me to either leave it unmarked or mark it as
@Internal.
>> Another suggestion is that we should start following FLIP-197[1](which
is already accepted by the community) to add more info into the
PublicEvolving annotation in order to kick off the graduation process.
As for FLIP-197, I found that the proposal in this flip has not actually
been implemented. Because FLINK-25352 [2] has not been resolved，it looks
like we can't add more info into the PublicEvolving annotation.
[1]
https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees
[2] https://issues.apache.org/jira/browse/FLINK-25352
Best,
Jinzhong
On Mon, Mar 11, 2024 at 12:29 AM Jing Ge <ji...@ververica.com.invalid> wrote:
> Hi Jinzhong,
>
> Sorry for the late reply. The key guideline of adding visibility annotation
> is whether the interface/class is a customer-facing API. I was wondering if
> SingleStateIterator and RocksDBRestoreOperation are exposed to users even
> if they are interfaces, i.e. users can use their own implementation
> classes. The flink-statebackend-rocksdb module has many more classes than
> the FLIP described. Only adding @Internal annotation to these two
> interfaces could be considered as an implicit hint that users can replace
> the default behaviour with their own implementation.
>
> Another suggestion is that we should start following FLIP-197[1](which is
> already accepted by the community) to add more info into the PublicEvolving
> annotation in order to kick off the graduation process. WDYT?
>
> Best regards,
> Jing
>
> [1]
>
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-197%3A+API+stability+graduation+process
>
> On Mon, Feb 26, 2024 at 2:22 PM Jinzhong Li <li...@gmail.com>
> wrote:
>
> > Hi all,
> >
> > Thanks for all the feedback. It seems there are no more questions
> > unaddressed. I would like to open the voting thread after three days.
> >
> > Please let me know if you have any concerns, thanks!
> >
> > Best,
> > Jinzhong Li
> >
> > On Mon, Feb 26, 2024 at 11:29 AM Yanfei Lei <fr...@gmail.com> wrote:
> >
> > > @Yun Tang
> > > Thanks for the information, +1 for marking
> > > `ConfigurableRocksDBOptionsFactory` as `PublicEvolving `.
> > >
> > > Best,
> > > Yanfei
> > >
> > > Yun Tang <my...@live.com> 于2024年2月23日周五 19:54写道：
> > > >
> > > > Hi Jinzhong,
> > > >
> > > > Thanks for driving this topic, and +1 for fixing the lack of
> > annotation.
> > > >
> > > > @Yanfei the `ConfigurableRocksDBOptionsFactory` interface is
> introduced
> > > for user extension, you can refer to the doc[1], which shows an example
> > of
> > > how to use this interface.
> > > >
> > > >
> > > > [1]
> > >
> >
> https://nightlies.apache.org/flink/flink-docs-master/docs/ops/state/large_state_tuning/#tuning-rocksdb-memory
> > > >
> > > >
> > > > Best
> > > > Yun Tang
> > > > ________________________________
> > > > From: Yanfei Lei <fr...@gmail.com>
> > > > Sent: Thursday, February 22, 2024 15:39
> > > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > > Subject: Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB
> > > StateBackend user-facing classes
> > > >
> > > > 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 <li...@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 <masteryhx@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.lan@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 <
> > > lijinzhong2051@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.
> > > > > >
> > >
> >
>
Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB StateBackend user-facing classes
Posted by Jing Ge <ji...@ververica.com.INVALID>.
Hi Jinzhong,
Sorry for the late reply. The key guideline of adding visibility annotation
is whether the interface/class is a customer-facing API. I was wondering if
SingleStateIterator and RocksDBRestoreOperation are exposed to users even
if they are interfaces, i.e. users can use their own implementation
classes. The flink-statebackend-rocksdb module has many more classes than
the FLIP described. Only adding @Internal annotation to these two
interfaces could be considered as an implicit hint that users can replace
the default behaviour with their own implementation.
Another suggestion is that we should start following FLIP-197[1](which is
already accepted by the community) to add more info into the PublicEvolving
annotation in order to kick off the graduation process. WDYT?
Best regards,
Jing
[1]
https://cwiki.apache.org/confluence/display/FLINK/FLIP-197%3A+API+stability+graduation+process
On Mon, Feb 26, 2024 at 2:22 PM Jinzhong Li <li...@gmail.com>
wrote:
> Hi all,
>
> Thanks for all the feedback. It seems there are no more questions
> unaddressed. I would like to open the voting thread after three days.
>
> Please let me know if you have any concerns, thanks!
>
> Best,
> Jinzhong Li
>
> On Mon, Feb 26, 2024 at 11:29 AM Yanfei Lei <fr...@gmail.com> wrote:
>
> > @Yun Tang
> > Thanks for the information, +1 for marking
> > `ConfigurableRocksDBOptionsFactory` as `PublicEvolving `.
> >
> > Best,
> > Yanfei
> >
> > Yun Tang <my...@live.com> 于2024年2月23日周五 19:54写道：
> > >
> > > Hi Jinzhong,
> > >
> > > Thanks for driving this topic, and +1 for fixing the lack of
> annotation.
> > >
> > > @Yanfei the `ConfigurableRocksDBOptionsFactory` interface is introduced
> > for user extension, you can refer to the doc[1], which shows an example
> of
> > how to use this interface.
> > >
> > >
> > > [1]
> >
> https://nightlies.apache.org/flink/flink-docs-master/docs/ops/state/large_state_tuning/#tuning-rocksdb-memory
> > >
> > >
> > > Best
> > > Yun Tang
> > > ________________________________
> > > From: Yanfei Lei <fr...@gmail.com>
> > > Sent: Thursday, February 22, 2024 15:39
> > > To: dev@flink.apache.org <de...@flink.apache.org>
> > > Subject: Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB
> > StateBackend user-facing classes
> > >
> > > 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 <li...@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 <ma...@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.lan@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 <
> > lijinzhong2051@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.
> > > > >
> >
>
Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB StateBackend user-facing classes
Posted by Jinzhong Li <li...@gmail.com>.
Hi all,
Thanks for all the feedback. It seems there are no more questions
unaddressed. I would like to open the voting thread after three days.
Please let me know if you have any concerns, thanks!
Best,
Jinzhong Li
On Mon, Feb 26, 2024 at 11:29 AM Yanfei Lei <fr...@gmail.com> wrote:
> @Yun Tang
> Thanks for the information, +1 for marking
> `ConfigurableRocksDBOptionsFactory` as `PublicEvolving `.
>
> Best,
> Yanfei
>
> Yun Tang <my...@live.com> 于2024年2月23日周五 19:54写道：
> >
> > Hi Jinzhong,
> >
> > Thanks for driving this topic, and +1 for fixing the lack of annotation.
> >
> > @Yanfei the `ConfigurableRocksDBOptionsFactory` interface is introduced
> for user extension, you can refer to the doc[1], which shows an example of
> how to use this interface.
> >
> >
> > [1]
> https://nightlies.apache.org/flink/flink-docs-master/docs/ops/state/large_state_tuning/#tuning-rocksdb-memory
> >
> >
> > Best
> > Yun Tang
> > ________________________________
> > From: Yanfei Lei <fr...@gmail.com>
> > Sent: Thursday, February 22, 2024 15:39
> > To: dev@flink.apache.org <de...@flink.apache.org>
> > Subject: Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB
> StateBackend user-facing classes
> >
> > 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 <li...@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 <ma...@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 <za...@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 <
> lijinzhong2051@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.
> > > >
>
Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB StateBackend user-facing classes
Posted by Yanfei Lei <fr...@gmail.com>.
@Yun Tang
Thanks for the information, +1 for marking
`ConfigurableRocksDBOptionsFactory` as `PublicEvolving `.
Best,
Yanfei
Yun Tang <my...@live.com> 于2024年2月23日周五 19:54写道：
>
> Hi Jinzhong,
>
> Thanks for driving this topic, and +1 for fixing the lack of annotation.
>
> @Yanfei the `ConfigurableRocksDBOptionsFactory` interface is introduced for user extension, you can refer to the doc[1], which shows an example of how to use this interface.
>
>
> [1] https://nightlies.apache.org/flink/flink-docs-master/docs/ops/state/large_state_tuning/#tuning-rocksdb-memory
>
>
> Best
> Yun Tang
> ________________________________
> From: Yanfei Lei <fr...@gmail.com>
> Sent: Thursday, February 22, 2024 15:39
> To: dev@flink.apache.org <de...@flink.apache.org>
> Subject: Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB StateBackend user-facing classes
>
> 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 <li...@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 <ma...@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 <za...@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 <li...@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.
> > >
Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB StateBackend user-facing classes
Posted by Yun Tang <my...@live.com>.
Hi Jinzhong,
Thanks for driving this topic, and +1 for fixing the lack of annotation.
@Yanfei the `ConfigurableRocksDBOptionsFactory` interface is introduced for user extension, you can refer to the doc[1], which shows an example of how to use this interface.
[1] https://nightlies.apache.org/flink/flink-docs-master/docs/ops/state/large_state_tuning/#tuning-rocksdb-memory
Best
Yun Tang
________________________________
From: Yanfei Lei <fr...@gmail.com>
Sent: Thursday, February 22, 2024 15:39
To: dev@flink.apache.org <de...@flink.apache.org>
Subject: Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB StateBackend user-facing classes
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 <li...@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 <ma...@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 <za...@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 <li...@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.
> >
Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB StateBackend user-facing classes
Posted by Yanfei Lei <fr...@gmail.com>.
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 <li...@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 <ma...@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 <za...@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 <li...@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.
> >
Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB StateBackend user-facing classes
Posted by Jinzhong Li <li...@gmail.com>.
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 <ma...@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 <za...@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 <li...@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.
>
Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB StateBackend user-facing classes
Posted by Hangxiang Yu <ma...@gmail.com>.
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 <za...@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 <li...@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.
Re: [DISCUSS]FLIP-420: Add API annotations for RocksDB StateBackend user-facing classes
Posted by Zakelly Lan <za...@gmail.com>.
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 <li...@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
>