Hi Zakelly,
Fair point to have short names.
How about having the following configs then?
connector=savepoint
state.type=keyed|non-keyed|metadata
This would introduce the proper keyed/non-keyed option + the config pair
will
exactly tell that savepoint metadata is going to be read.
BR,
G
On Thu, Dec 19, 2024 at 4:11 AM Zakelly Lan <zakelly....@gmail.com> wrote:
> Hi Gabor,
>
> Thanks for your answers!
>
> I'm good with naming it 'savepoint' with an extra option like 'mode'
> specifying the reading type, or suffixes in names like 'savepoint-metadata'
> for each type of state. For the second option it means the current
> connector will be named 'savepoint-keyed'? Well I slightly prefer a short
> name.
>
> For connector='metadata', it seems unclear to me (what is metadata of Flink
> :D), so I don't recommend this option.
>
>
> Best,
> Zakelly
>
>
> On Wed, Dec 18, 2024 at 10:35 PM Gabor Somogyi <gabor.g.somo...@gmail.com>
> wrote:
>
> > Hi Zakelly,
> >
> > Thanks for your comments, please find my answers inline.
> >
> > BR,
> > G
> >
> >
> > On Wed, Dec 18, 2024 at 2:42 PM Zakelly Lan <zakelly....@gmail.com>
> wrote:
> >
> > > Hi Gabor,
> > >
> > > Thanks for the FLIP! It's very convenient for users to use SQL to read
> > > checkpoint data, so overall I'd +1 for this FLIP. Here are my
> > > thoughts/questions:
> > >
> > > 1. I see this connector is designed to read the checkpoint/savepoint
> > > data, so I'd suggest naming this connector as 'checkpoint' or
> > > 'savepoint',
> > > which is more intuitive. The "connector='state'" makes me think this
> > is
> > > reading the runtime state. Also the 'state.path' can be changed to
> > > 'checkpoint.path'.
> > >
> > Rename is a possibility, I'm fine with either way in terms of naming.
> > The only thing what I would consider is that later on it would be good
> > to add metadata reading possibility. Such case we can either provide
> > "savepoint" for data reading and "savepoint_metadata" or simply
> "metadata"
> > for meta reading (See 3rd bullet for another approach). Please share your
> > thoughts on this.
> >
> >
> > > 2. IIUC this connector is a bounded source and batch execution is
> > > preferred, right? And it cannot read checkpoints while the original
> > job
> > > is
> > > running and doing periodic checkpoints. Thus I'd prefer the name
> > > 'savepoint', as the lifecycle of savepoints is managed by the user
> > > instead
> > > of the Flink job.
> > >
> > We can call it savepoint, just agree on the metadata naming as well to
> have
> > an overall picture.
> >
> >
> > > 3. As you pictured, there might be more connectors for states other
> > than
> > > keyed state, so I'm thinking about providing them in one connector
> > with
> > > an
> > > option 'state.type' enumerating 'keyed', 'non-keyed' ('operator'),
> > and
> > > so
> > > on. Well for the first version this is not a required option. Seems
> > more
> > > organized. WDYT?
> > >
> > Yeah, later on we must provide such config parameter somehow.
> > Couple of things to consider:
> > - Maybe we can add "metadata" as possible value here instead of
> > "connector=X"
> > - I'm thinking about whether we can come up with something
> > where we can mix metadata, keyed and non-keyed data in a single SQL
> > statement.
> > Without super deep consideration I would say no, but I'm going to think
> it
> > through.
> >
> >
> > > 4. I'm not sure if the general option 'key.format' is needed, would
> > > using 'fields.k.format' while the 'k' is the name of the SQL primary
> > > column
> > > be feasible?
> > >
> > I think that's a nice catch of a simplification.
> >
> >
> > > 5. Reading window state[1] is unsupported, right?
> > >
> > Yeah, not yet covered here and can come in a later FLIP.
> >
> >
> > >
> > >
> > > [1]
> > >
> > >
> >
> https://nightlies.apache.org/flink/flink-docs-master/docs/libs/state_processor_api/#window-state
> > >
> > > Best,
> > > Zakelly
> > >
> > > On Wed, Dec 18, 2024 at 5:46 PM Gabor Somogyi <
> gabor.g.somo...@gmail.com
> > >
> > > wrote:
> > >
> > > > Hi All,
> > > >
> > > > I'd like to start a discussion of FLIP-496: SQL connector for keyed
> > state
> > > > data [1].
> > > > Feel free to add your thoughts to make this feature better.
> > > >
> > > > [1]
> > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-496%3A+SQL+connector+for+keyed+state+data
> > > >
> > > > BR,
> > > > G
> > > >
> > >
> >
>
