Hey all, starting a vote in 24 hours. Unless there are more points to discuss.
Thanks, Alexey On Mon, Jun 17, 2024 at 8:28 AM Alexey Leonov-Vendrovskiy < vendrov...@gmail.com> wrote: > Done. There was some accidental removal during editing. > > Thanks, > Alexey > > On Mon, Jun 17, 2024 at 5:23 AM Jim Hughes <jhug...@confluent.io.invalid> > wrote: > >> Hi Alexey, >> >> Looks reasonable. As a note, I think this sentence is missing some words: >> >> "Due to batch queries nature, we do not expect the jobs to run forever >> (days, months, years) – unlike from . " >> >> Can you update it? >> >> Cheers, >> >> Jim >> >> On Mon, Jun 17, 2024 at 3:16 AM Alexey Leonov-Vendrovskiy < >> vendrov...@gmail.com> wrote: >> >> > Thanks Timo and Jim! Added a few sentences to the FLIP to cover your >> > points. >> > -Alexey >> > >> > On Mon, Jun 10, 2024 at 11:23 PM Timo Walther <twal...@apache.org> >> wrote: >> > >> > > Hi Alexey, >> > > >> > > thanks for proposing this FLIP. It is a nice continuation of the >> vision >> > > we had for CompiledPlan when writing and implementing FLIP-190. The >> > > whole stack is prepared for serializing BatchExecNodes as well so it >> > > shouldn't be too hard to make this a reality. >> > > >> > > > I think the FLIP should be clear on the backwards support strategy >> > > > here. The strategy for streaming is "forever". This may be the >> most >> > > > interesting part of the FLIP to discuss. >> > > >> > > I agree with Jim. We shouldn't put too much burden on us (the Flink >> > > community). BatchExecNodes can evolve quicker than StreamExecNodes as >> > > the state component isn't an issue. Backwards compatibility of 2-3 >> Flink >> > > versions and at least 1 year of time should be enough for batch >> > > infrastructure to update. Of course we should avoid breaking changes >> > > whenever possible. This should be written down in the FLIP. >> > > >> > > Regards, >> > > Timo >> > > >> > > >> > > >> > > >> > > On 07.06.24 23:10, Jim Hughes wrote: >> > > > Hi Alexey, >> > > > >> > > > Responses inline below: >> > > > >> > > > On Mon, May 13, 2024 at 7:18 PM Alexey Leonov-Vendrovskiy < >> > > > vendrov...@gmail.com> wrote: >> > > > >> > > >> Thanks Jim. >> > > >> >> > > >>> 1. For the testing, I'd call the tests "execution" tests rather >> than >> > > >>> "restore" tests. For streaming execution, restore tests have the >> > > >> compiled >> > > >>> plan and intermediate state; the tests verify that those can work >> > > >> together >> > > >>> and continue processing. >> > > >> >> > > >> Agree that we don't need to store and restore the intermediate >> state. >> > So >> > > >> the most critical part is that the CompiledPlan for batch can be >> > > executed. >> > > >> >> > > > >> > > > On the FLIP, can you be more specific about what we are checking >> during >> > > > execution? I'd suggest that `executeSql(_)` and >> > > > `executePlan(compilePlanSql(_))` should be compared. >> > > > >> > > > >> > > >> 2. The FLIP implicitly suggests "completeness tests" (to use >> > FLIP-190's >> > > >>> words). Do we need "change detection tests"? I'm a little >> unsure if >> > > >> that >> > > >>> is presently happening in an automatic way for streaming >> operators. >> > > >> >> > > >> >> > > >> We might need to elaborate more on this, but the idea is that we >> > > need to >> > > >> make sure that compiled plans created by an older version of SQL >> > Planner >> > > >> are executable on newer runtimes. >> > > >> >> > > >> 3. Can we remove old versions of batch operators eventually? Or >> do >> > we >> > > >>> need to keep them forever like we would for streaming operators? >> > > >>> >> > > >> >> > > >> We could have deprecation paths for old operator nodes in some >> cases. >> > > It is >> > > >> a matter of the time window: what could be practical the "time >> > distance" >> > > >> between query planner and flink runtime against which the query >> query >> > > can >> > > >> be resubmitted. >> > > >> Note, here we don't have continuous queries, so there is always an >> > > option >> > > >> to "re-plan" the original SQL query text into a newer version of >> the >> > > >> CompiledPlan. >> > > >> With this in mind, a time window of 1yr+ would allow deprecation of >> > > older >> > > >> batch exec nodes, though I don't see this as a frequent event. >> > > >> >> > > > >> > > > As I read the JavaDocs for `TableEnvironment.loadPlan`, it looks >> like >> > the >> > > > compiled plan ought to be sufficient to run a job at a later time. >> > > > >> > > > I think the FLIP should be clear on the backwards support strategy >> > here. >> > > > The strategy for streaming is "forever". This may be the most >> > > interesting >> > > > part of the FLIP to discuss. >> > > > >> > > > Can you let us know when you've updated the FLIP? >> > > > >> > > > Cheers, >> > > > >> > > > Jim >> > > > >> > > > >> > > >> -Alexey >> > > >> >> > > >> >> > > >> >> > > >> On Mon, May 13, 2024 at 1:52 PM Jim Hughes >> > <jhug...@confluent.io.invalid >> > > > >> > > >> wrote: >> > > >> >> > > >>> Hi Alexey, >> > > >>> >> > > >>> After some thought, I have a question about deprecations: >> > > >>> >> > > >>> 3. Can we remove old versions of batch operators eventually? Or >> do >> > we >> > > >>> need to keep them forever like we would for streaming operators? >> > > >>> >> > > >>> Cheers, >> > > >>> >> > > >>> Jim >> > > >>> >> > > >>> On Thu, May 9, 2024 at 11:29 AM Jim Hughes <jhug...@confluent.io> >> > > wrote: >> > > >>> >> > > >>>> Hi Alexey, >> > > >>>> >> > > >>>> Overall, the FLIP looks good and makes sense to me. >> > > >>>> >> > > >>>> 1. For the testing, I'd call the tests "execution" tests rather >> than >> > > >>>> "restore" tests. For streaming execution, restore tests have the >> > > >>> compiled >> > > >>>> plan and intermediate state; the tests verify that those can work >> > > >>> together >> > > >>>> and continue processing. >> > > >>>> >> > > >>>> For batch execution, I think we just want that all existing >> compiled >> > > >>> plans >> > > >>>> can be executed in future versions. >> > > >>>> >> > > >>>> 2. The FLIP implicitly suggests "completeness tests" (to use >> > > FLIP-190's >> > > >>>> words). Do we need "change detection tests"? I'm a little >> unsure >> > if >> > > >>> that >> > > >>>> is presently happening in an automatic way for streaming >> operators. >> > > >>>> >> > > >>>> In RestoreTestBase, generateTestSetupFiles is disabled and has >> to be >> > > >> run >> > > >>>> manually when tests are being written. >> > > >>>> >> > > >>>> Cheers, >> > > >>>> >> > > >>>> Jim >> > > >>>> >> > > >>>> On Tue, May 7, 2024 at 5:11 AM Paul Lam <paullin3...@gmail.com> >> > > wrote: >> > > >>>> >> > > >>>>> Hi Alexey, >> > > >>>>> >> > > >>>>> Thanks a lot for bringing up the discussion. I’m big +1 for the >> > FLIP. >> > > >>>>> >> > > >>>>> I suppose the goal doesn’t involve the interchangeability of >> json >> > > >> plans >> > > >>>>> between batch mode and streaming mode, right? >> > > >>>>> In other words, a json plan compiled in a batch program can’t be >> > run >> > > >> in >> > > >>>>> streaming mode without a migration (which is not yet supported). >> > > >>>>> >> > > >>>>> Best, >> > > >>>>> Paul Lam >> > > >>>>> >> > > >>>>>> 2024年5月7日 14:38，Alexey Leonov-Vendrovskiy < >> vendrov...@gmail.com> >> > > >> 写道： >> > > >>>>>> >> > > >>>>>> Hi everyone, >> > > >>>>>> >> > > >>>>>> PTAL at the proposed FLIP-456: CompiledPlan support for Batch >> > > >>> Execution >> > > >>>>>> Mode. It is pretty self-describing. >> > > >>>>>> >> > > >>>>>> Any thoughts are welcome! >> > > >>>>>> >> > > >>>>>> Thanks, >> > > >>>>>> Alexey >> > > >>>>>> >> > > >>>>>> [1] >> > > >>>>>> >> > > >>>>> >> > > >>> >> > > >> >> > > >> > >> https://cwiki.apache.org/confluence/display/FLINK/FLIP-456%3A+CompiledPlan+support+for+Batch+Execution+Mode >> > > >>>>>> . >> > > >>>>> >> > > >>>>> >> > > >>> >> > > >> >> > > > >> > > >> > > >> > >> >
