Re: [DISCUSS] Sink V2 API missing features

[Péter Váry]

Hi Martijn,

Thanks for the prompt answer, good to hear that there is interest in
the community to improve things around the APIs!

*First and foremost, we still plan to use the PostCommitTopology* to do the
compaction for the Iceberg Sink. That is on our roadmap, but the first step
is to do the SinkV2 migration, with which we have found several
difficulties as I have mentioned in my previous email.

Here is how I envision the Flink Sink flow in the long term:

   1. We need a rebalancing step before the writers, to redistribute the
   incoming data - *WithPreWriteTopology* will be used for this step
   (parallelism N)
   2. We will write out the data files, and collect the statistics and
   other metadata about the files - *SinkWriter* will be used for this step
   (parallelism N)
   3. We will aggregate the data file names and the metadata about the data
   files generated in this checkpoint - *WithPreCommitTopology* will be
   used (parallelism 1)
   4. We commit the changes aggregated during the checkpoint - *Committer*
   will be used (parallelism 1)
   5. After a few commits we do quick incremental compaction for these
   commits - *WithPostCommitTopology* will be used (parallelism 1?)

I have a PR available for 1-4 steps, I am still in the process of finding
the best solution for step 5. Currently I am leaning toward using only
minimal data (like the number of commits, or files) coming from the
committer in this step, and relying on the Iceberg table data/metadata on
compaction. If this does not change then I think the current
WithPostCommitTopology would be enough for our case.

I am in CET TZ, but we can have a quick chat on slack or webex or whatever
your poison is :)

Thanks,
Peter

 - [1] Iceberg design doc:
https://docs.google.com/document/d/1K1M4wb9r_Tr-SDsUvqLyBaI5F14eRcqe3-ud6io0Da0/edit

Martijn Visser  ezt írta (időpont: 2023. szept.
29., P, 2:44):

> Hi Peter,
>
> I actually had a session this week at Current, talking about the state
> of the Flink connector ecosystem. Afterwards, I had a lot of
> conversations with people who were interested in the compaction with
> Iceberg and I actually was planning to look at what the current status
> was on the Iceberg topic. Your timing is great :)
>
> I'm looping in Gordon, since we talked about the stabilization of
> these experimental interfaces offline. Also looping in Guowei Ma and
> Yun Gao since they helped a lot in the discussion for the Sink V2 API.
>
> When we discussed FLIP-191 [1], my understanding was that Iceberg
> would "writes the files immediately and the post topology will take
> care of compacting the already written files and updating the file log
> after the compaction" [2]. Looking at the Iceberg proposal, the entire
> PostCommit wouldn't be used by Iceberg. I'm wondering if that means we
> should remove the entire PostCommit topology support API, since as far
> as I can tell there's no other connector that needs it. Or is there a
> plan to use that at a later stage by Iceberg?
>
> I need to think a bit more on what are potential options to improve
> these interfaces, but wanted to already loop in some people in the
> thread.
>
> Best regards,
>
> Martijn
>
> [1] https://lists.apache.org/thread/zjc4p47k4sxjcbnntt8od2grnmx51xg0
> [2]
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-191%3A+Extend+unified+Sink+interface+to+support+small+file+compaction#FLIP191:ExtendunifiedSinkinterfacetosupportsmallfilecompaction-Alternative1GlobalSinkCoordinator
> :
>
>
>
> On Thu, Sep 28, 2023 at 7:56 AM Péter Váry 
> wrote:
> >
> > Hi Flink Team,
> >
> > I am working on implementing a new Iceberg Sink which would use the new
> > SinkV2 API [1], [2]. The main goal is to open up the possibility for
> doing
> > the incremental small files compaction inside a Flink job using the new
> > Iceberg Sink.
> >
> > During the implementation I have found several places where the SinkV2
> API
> > has some missing features, some of which have already been discussed on
> > this mailing list [3].
> >
> > Here is the abbreviated list (the full description of the missing feature
> > and the proposed workarounds are available in the design doc [1])
> >
> >- Committer
> >   - Metrics is not available - No workaround - Would be a simple
> >   additional change
> >   - No init method - Simple workaround - Would be a simple additional
> >   change
> >   - Parallelism - Simple, ugly workaround - Would be a simple(?)
> >   additional change
> >- WithPreCommitTopology
> >   - Transformation input/output parameters are the same - Very ugly
> >   workaround - I see multiple ways to solve this, and I would like
> to hear
> >   the opinion of the Community. See the discussion below.
> >
> > Let's just focus on the precommit topology. This is currently defined as
> > this:
> >
> >
> > *@Experimental*
> >
> > *public interface WithPreCommitTopology*
> > *extends TwoPhaseCommittingSink {*
> >
> >
> >
> >
> >
> > *  

Support AWS SDK V2 for Flink's S3 FileSystem

[Min, Maomao]

Hi Flink Dev,

I’m Maomao, a developer from AWS EMR.

Recently, our team is working on adding AWS SDK V2 support for Flink’s S3 
Filesystem. During development, we found out that our work was blocked by 
Presto. This is because that Presto still uses AWS SDK V1 and won’t add support 
for AWS SDK V2 in short term. To unblock, our team proposed several options and 
I’ve created a JIRA issue as 
here.

Since our team plans to contribute this work back to the community later, we’d 
like to collect feedback from the community about the options we proposed in 
the long term so that the community won’t need to duplicate this work in the 
future.

Best,
Maomao

Re: [DISCUSS] FLIP 333 - Redesign Apache Flink website

[Robert Metzger]

There's now a PR (https://github.com/apache/flink-web/pull/676) and a
preview available for this FLIP (
https://website-refresh.d193kg429zpv7e.amplifyapp.com/).

On Fri, Jul 28, 2023 at 8:45 AM Mohan, Deepthi 
wrote:

> Matthias, Markos, Martijn, thank you for your feedback.
>
> Markos, I've addressed you feedback re: separating use cases from the
> Flink capabilities. I think it is a positive change that will help new
> users distinguish between the two.
> Also attached screenshots for 'light mode'. I am personally partial to
> dark mode, and several developers say they prefer dark mode vs light
> generally. However, this is not a specific question I've posed to customers
> regarding the Flink website. In addition, after talking to engineering,
> I've been told it's not 2x the effort to introduce both modes on the
> website and a toggle to switch between the two. For implementation, we
> could start with one mode, without the toggle.
>
> Mattias, your comments about accessibility were very useful and helped us
> to further improve the design. A UX designer (Kaushal, also new to the
> community) helped evaluate the color and text size for accessibility. He
> can respond to any specific questions that you may have about
> accessibility. I did not quite get the related comment about "the menu
> structure stays the same and there are no plans to replace text with
> images". However, since it's not there in the current website I propose we
> table this conversation for now.
>
> Martijn, thanks for your comment on accessibility. I hope some of your
> concerns are addressed above in my response to Mattias and the screenshots
> now attached to the FLIP. I have purposely kept documentation out of scope
> due to the comments received in the previous discussion thread on this
> topic [1]. We will also include links to the blog and GitHub repo in the
> drop down under the getting started menu as well as include the links at
> the bottom of the page (as seen in a few other Apache websites).
>
> [1] https://lists.apache.org/thread/c3pt00cf77lrtgt242p26lgp9l2z5yc8
>
> Thanks,
> Deepthi
>
>
>
>
> On 7/23/23, 7:22 PM, "liu ron"  ron9@gmail.com>> wrote:
>
>
> CAUTION: This email originated from outside of the organization. Do not
> click links or open attachments unless you can confirm the sender and know
> the content is safe.
>
>
>
>
>
>
> +1,
>
>
> The dark mode looks very cool.
>
>
> Best,
> Ron
>
>
> Matthias Pohl  matthias.p...@aiven.io.inva>lid> 于2023年7月20日周四 15:45写道：
>
>
> > I think Martijn and Markos brought up a few good points:
> >
> > - We shouldn't degrade the accessibility but ideally improve it as part
> of
> > the redesign. The current proposal doesn't look like we're doing changes
> in
> > a way that it lowers the accessibility (considering that the menu
> structure
> > stays the same and there are no plans to replace text with images). But
> > nonetheless, it would be good to have this topic explicitly covered in
> the
> > FLIP.
> >
> > - I see Markos' concern about the white vs black background (Flink
> > documentation vs Flink website): Does it make a big difference to change
> to
> > a white background design? The switch to dark mode could happen after a
> > similar (or the same) theme is supported by the documentation. Or are
> there
> > other reasons why the dark background is favorable?
> >
> > Best,
> > Matthias
> >
> > On Wed, Jul 19, 2023 at 12:28 PM Markos Sfikas
> > mailto:markos.sfi...@aiven.io.inva>lid>
> wrote:
> >
> > > +1 Thanks for proposing this FLIP, Deepthi.
> > >
> > > The designs on FLIP-333 [1] look fresh and modern and I feel they
> achieve
> > > the goal in general.
> > >
> > > A couple of suggestions from my side could be the following:
> > >
> > > [a] Assuming that no changes are implemented to the Flink
> documentation,
> > I
> > > would like to see a visual with a 'white background' instead of the
> 'dark
> > > mode'. This is primarily for two reasons: Firstly, it provides a more
> > > consistent experience for the website visitor going from the home page
> to
> > > the documentation (instead of switching from dark to white mode on the
> > > website) and secondly, from an accessibility and inclusivity
> perspective
> > > that was mentioned earlier, we should give the option to either switch
> > > between dark and white mode or have something that is universally easy
> to
> > > read and consume (not everyone is comfortable reading white text on
> dark
> > > background).
> > >
> > > [b] Regarding structuring the home page, right now the Flink website
> has
> > > use cases blending with what seems to be Flink's 'technical
> > > characteristics' (i.e. the sections that talk about 'Guaranteed
> > > correctness', 'Layered APIs', 'Operational Focus', etc.). As someone
> new
> > to
> > > Flink and considering using the technology, I would like to understand
> > > firstly the use cases and secondly dive into the characteristics that
> > make
> > > Flink stand out. I would suggest 

Re: [ANNOUNCE] Release 1.18.0, release candidate #1

[Etienne Chauchot]

Hi all,

Thanks to the team for this RC.

I did a quick check of this RC against user pipelines (1) coded with 
DataSet (even if deprecated and soon removed), DataStream and SQL APIs


based on the small scope of this test, LGTM

+1 (non-binding)

[1] https://github.com/echauchot/tpcds-benchmark-flink

Best
Etienne

Le 28/09/2023 à 19:35, Jing Ge a écrit :

Hi everyone,

The RC1 for Apache Flink 1.18.0 has been created. The related voting
process will be triggered once the announcement is ready. The RC1 has all
the artifacts that we would typically have for a release, except for the
release note and the website pull request for the release announcement.

The following contents are available for your review:

- Confirmation of no benchmarks regression at the thread[1].
- The preview source release and binary convenience releases [2], which
are signed with the key with fingerprint 96AE0E32CBE6E0753CE6 [3].
- all artifacts that would normally be deployed to the Maven
Central Repository [4].
- source code tag "release-1.18.0-rc1" [5]

Your help testing the release will be greatly appreciated! And we'll
create the rc1 release and the voting thread as soon as all the efforts are
finished.

[1]https://lists.apache.org/thread/yxyphglwwvq57wcqlfrnk3qo9t3sr2ro
[2]https://dist.apache.org/repos/dist/dev/flink/flink-1.18.0-rc1/
[3]https://dist.apache.org/repos/dist/release/flink/KEYS
[4]https://repository.apache.org/content/repositories/orgapacheflink-1657
[5]https://github.com/apache/flink/releases/tag/release-1.18.0-rc1

Best regards,
Qingsheng, Sergei, Konstantin and Jing

[jira] [Created] (FLINK-33169) Window TVFs don't consider column expansion

[Timo Walther (Jira)]

Timo Walther created FLINK-33169:


 Summary: Window TVFs don't consider column expansion
 Key: FLINK-33169
 URL: https://issues.apache.org/jira/browse/FLINK-33169
 Project: Flink
  Issue Type: Sub-task
  Components: Table SQL / Planner
Reporter: Timo Walther
Assignee: Timo Walther


Window TVFs don't consider the column expansion. The reason for this is that 
`TABLE t` is expanded by a custom logic in the parser. The expansion logic 
should consider the descriptor in this case.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Re: [ANNOUNCE] Release 1.18.0, release candidate #1

[Matthias Pohl]

Thanks for creating RC1. I did the following checks:

* Downloaded artifacts
* Built Flink from sources
* Verified SHA512 checksums GPG signatures
* Compared checkout with provided sources
* Verified pom file versions
* Went over NOTICE file/pom files changes without finding anything
suspicious
* Deployed standalone session cluster and ran WordCount example in batch
and streaming: Nothing suspicious in log files found

+1 (binding)

On Fri, Sep 29, 2023 at 10:34 AM Etienne Chauchot 
wrote:

> Hi all,
>
> Thanks to the team for this RC.
>
> I did a quick check of this RC against user pipelines (1) coded with
> DataSet (even if deprecated and soon removed), DataStream and SQL APIs
>
> based on the small scope of this test, LGTM
>
> +1 (non-binding)
>
> [1] https://github.com/echauchot/tpcds-benchmark-flink
>
> Best
> Etienne
>
> Le 28/09/2023 à 19:35, Jing Ge a écrit :
> > Hi everyone,
> >
> > The RC1 for Apache Flink 1.18.0 has been created. The related voting
> > process will be triggered once the announcement is ready. The RC1 has all
> > the artifacts that we would typically have for a release, except for the
> > release note and the website pull request for the release announcement.
> >
> > The following contents are available for your review:
> >
> > - Confirmation of no benchmarks regression at the thread[1].
> > - The preview source release and binary convenience releases [2], which
> > are signed with the key with fingerprint 96AE0E32CBE6E0753CE6 [3].
> > - all artifacts that would normally be deployed to the Maven
> > Central Repository [4].
> > - source code tag "release-1.18.0-rc1" [5]
> >
> > Your help testing the release will be greatly appreciated! And we'll
> > create the rc1 release and the voting thread as soon as all the efforts
> are
> > finished.
> >
> > [1]https://lists.apache.org/thread/yxyphglwwvq57wcqlfrnk3qo9t3sr2ro
> > [2]https://dist.apache.org/repos/dist/dev/flink/flink-1.18.0-rc1/
> > [3]https://dist.apache.org/repos/dist/release/flink/KEYS
> > [4]
> https://repository.apache.org/content/repositories/orgapacheflink-1657
> > [5]https://github.com/apache/flink/releases/tag/release-1.18.0-rc1
> >
> > Best regards,
> > Qingsheng, Sergei, Konstantin and Jing
> >

Re: [DISCUSS] [FLINK-32873] Add a config to allow disabling Query hints

[Bonnie Arogyam Varghese]

Hi Jark,
 A minor suggestion. Would it be ok if we changed the config name to `
table.optimizer.query-options.enabled`?
This is inline with other existing configurations such as:

table.dynamic-table-options.enabled
table.optimizer.distinct-agg.split.enabled
table.optimizer.dynamic-filtering.enabled


On Wed, Sep 27, 2023 at 9:57 AM Bonnie Arogyam Varghese <
bvargh...@confluent.io> wrote:

> Hi Martjin,
> Yes, the suggestion for the configuration name made by Jark sounds good.
>
> Also, thanks to everyone who participated in this discussion.
>
> On Tue, Sep 19, 2023 at 2:40 PM Martijn Visser 
> wrote:
>
>> Hey Jark,
>>
>> Sounds fine to me.
>> @Bonnie does that also work for you?
>>
>> Best regards,
>>
>> Martijn
>>
>> On Fri, Sep 15, 2023 at 1:28 PM Jark Wu  wrote:
>> >
>> > Hi Martijn,
>> >
>> > Thanks for the investigation. I found the blog[1] shows a use case
>> > that they use "OPTIMIZER_IGNORE_HINTS" to check if embedded
>> > hints can be removed in legacy code. I think this is a useful tool to
>> > improve queries without complex hints strewn throughout the code.
>> > Therefore, I'm fine to support this now.
>> >
>> > Maybe we can follow Oracle to name the config
>> > "table.optimizer.ignore-query-hints=false"?
>> >
>> > Best,
>> > Jark
>> >
>> > [1]: https://dbaora.com/optimizer_ignore_hints-oracle-database-18c/
>> >
>> > On Fri, 15 Sept 2023 at 17:57, Martijn Visser > >
>> > wrote:
>> >
>> > > Hi Jark,
>> > >
>> > > Oracle has/had a generic "OPTIMIZER_IGNORE_HINTS" [1]. It looks like
>> MSSQL
>> > > has configuration options to disable specific hints [2] instead of a
>> > > generic solution.
>> > >
>> > > [1]
>> > >
>> > >
>> https://docs.oracle.com/en/database/oracle/oracle-database/23/refrn/OPTIMIZER_IGNORE_HINTS.html#GUID-D62CA6D8-D0D8-4A20-93EA-EEB4B3144347
>> > > [2]
>> > >
>> > >
>> https://www.mssqltips.com/sqlservertip/4175/disabling-sql-server-optimizer-rules-with-queryruleoff/
>> > >
>> > > Best regards,
>> > >
>> > > Martijn
>> > >
>> > > On Fri, Sep 15, 2023 at 10:53 AM Jark Wu  wrote:
>> > >
>> > > > Hi Martijn,
>> > > >
>> > > > I can understand that.
>> > > > Is there any database/system that supports disabling/enabling query
>> hints
>> > > >  with a configuration? This might help us to understand the use
>> case,
>> > > > and follow the approach.
>> > > >
>> > > > Best,
>> > > > Jark
>> > > >
>> > > > On Fri, 15 Sept 2023 at 15:34, Martijn Visser <
>> martijnvis...@apache.org>
>> > > > wrote:
>> > > >
>> > > > > Hi all,
>> > > > >
>> > > > > I think Jark has a valid point with:
>> > > > >
>> > > > > > Does this mean that in the future we might add an option to
>> disable
>> > > > each
>> > > > > feature?
>> > > > >
>> > > > > I don't think that's a reasonable outcome indeed, but we are
>> currently
>> > > > in a
>> > > > > situation where we don't have clear guidelines on when to add a
>> > > > > configuration option, and when not to add one. From a platform
>> > > > perspective,
>> > > > > there might not be an imminent or obvious security implication,
>> but you
>> > > > > want to minimize a potential attack surface.
>> > > > >
>> > > > > > We should try to remove the unnecessary configuration from the
>> list
>> > > in
>> > > > > Flink 2.0.
>> > > > >
>> > > > > I agree with that too.
>> > > > >
>> > > > > With these things in mind, my proposal would be the following:
>> > > > >
>> > > > > * Add a configuration option for this situation, given that we
>> don't
>> > > have
>> > > > > clear guidelines on when to add/not add a new config option.
>> > > > > * Since we want to overhaul the configuration layer anyway in
>> Flink
>> > > 2.0,
>> > > > we
>> > > > > clean-up the configuration list by not having an option for each
>> item,
>> > > > but
>> > > > > either a generic option that allows you to disable one or more
>> features
>> > > > (by
>> > > > > providing a list as the configuration option), or we already
>> bundle
>> > > > > multiple configuration options into a specific category, e.g. so
>> that
>> > > you
>> > > > > can have a default Flink without any hardening, a read-only
>> Flink, a
>> > > > > fully-hardened Flink etc)
>> > > > >
>> > > > > Best regards,
>> > > > >
>> > > > > Martijn
>> > > > >
>> > > > >
>> > > > > On Mon, Sep 11, 2023 at 4:51 PM Jim Hughes
>> > > > > > > >
>> > > > > wrote:
>> > > > >
>> > > > > > Hi Jing and Jark!
>> > > > > >
>> > > > > > I can definitely appreciate the desire to have fewer
>> configurations.
>> > > > > >
>> > > > > > Do you have a suggested alternative for platform providers to
>> limit
>> > > or
>> > > > > > restrict the hints that Bonnie is talking about?
>> > > > > >
>> > > > > > As one possibility, maybe one configuration could be set to
>> control
>> > > all
>> > > > > > hints.
>> > > > > >
>> > > > > > Cheers,
>> > > > > >
>> > > > > > Jim
>> > > > > >
>> > > > > > On Sat, Sep 9, 2023 at 6:16 AM Jark Wu 
>> wrote:
>> > > > > >
>> > > > > > > I agree with Jing,
>> > > > > > >
>> > > > > > > My biggest concern is this makes

Re: [ANNOUNCE] Release 1.18.0, release candidate #1

[Gabor Somogyi]

Thanks for the efforts!

+1 (non-binding)

* Verified versions in the poms
* Built from source
* Verified checksums and signatures
* Started basic workloads with kubernetes operator
* Verified NOTICE and LICENSE files

G

On Fri, Sep 29, 2023, 18:16 Matthias Pohl 
wrote:

> Thanks for creating RC1. I did the following checks:
>
> * Downloaded artifacts
> * Built Flink from sources
> * Verified SHA512 checksums GPG signatures
> * Compared checkout with provided sources
> * Verified pom file versions
> * Went over NOTICE file/pom files changes without finding anything
> suspicious
> * Deployed standalone session cluster and ran WordCount example in batch
> and streaming: Nothing suspicious in log files found
>
> +1 (binding)
>
> On Fri, Sep 29, 2023 at 10:34 AM Etienne Chauchot 
> wrote:
>
> > Hi all,
> >
> > Thanks to the team for this RC.
> >
> > I did a quick check of this RC against user pipelines (1) coded with
> > DataSet (even if deprecated and soon removed), DataStream and SQL APIs
> >
> > based on the small scope of this test, LGTM
> >
> > +1 (non-binding)
> >
> > [1] https://github.com/echauchot/tpcds-benchmark-flink
> >
> > Best
> > Etienne
> >
> > Le 28/09/2023 à 19:35, Jing Ge a écrit :
> > > Hi everyone,
> > >
> > > The RC1 for Apache Flink 1.18.0 has been created. The related voting
> > > process will be triggered once the announcement is ready. The RC1 has
> all
> > > the artifacts that we would typically have for a release, except for
> the
> > > release note and the website pull request for the release announcement.
> > >
> > > The following contents are available for your review:
> > >
> > > - Confirmation of no benchmarks regression at the thread[1].
> > > - The preview source release and binary convenience releases [2], which
> > > are signed with the key with fingerprint 96AE0E32CBE6E0753CE6 [3].
> > > - all artifacts that would normally be deployed to the Maven
> > > Central Repository [4].
> > > - source code tag "release-1.18.0-rc1" [5]
> > >
> > > Your help testing the release will be greatly appreciated! And we'll
> > > create the rc1 release and the voting thread as soon as all the efforts
> > are
> > > finished.
> > >
> > > [1]https://lists.apache.org/thread/yxyphglwwvq57wcqlfrnk3qo9t3sr2ro
> > > [2]https://dist.apache.org/repos/dist/dev/flink/flink-1.18.0-rc1/
> > > [3]https://dist.apache.org/repos/dist/release/flink/KEYS
> > > [4]
> > https://repository.apache.org/content/repositories/orgapacheflink-1657
> > > [5]https://github.com/apache/flink/releases/tag/release-1.18.0-rc1
> > >
> > > Best regards,
> > > Qingsheng, Sergei, Konstantin and Jing
> > >
>

Re: [DISCUSS] FLIP-360: Merging ExecutionGraphInfoStore and JobResultStore into a single component

[Matthias Pohl]

Thanks for sharing your thoughts, Shammon FY. I kind of see your point.

Initially, I didn't put much thought into splitting up the interface into
two because the dispatcher would have been the only component dealing with
the two interfaces. Having two interfaces wouldn't have added much value
(in terms of testability and readability, I thought).

But I iterated over the idea once more and came up with a new proposal that
involves the two components CompletedJobStore and JobDetailsStore. It's not
100% what you suggested (because the retrieval of the ExecutionGraphInfo
still lives in the CompletedJobStore) but the separation makes sense in my
opinion:
- The CompletedJobStore deals with the big data that might require
accessing the disk.
- The JobDetailsStore handles the small-footprint data that lives in memory
all the time. Additionally, it takes care of actually deleting the metadata
of the completed job in both stores if a TTL is configured.

See FLIP-360 [1] with the newly added class and sequence diagrams and
additional content. I only updated the Interfaces & Methods section (see
diff [2]).

I'm looking forward to feedback.

Best,
Matthias

[1]
https://cwiki.apache.org/confluence/display/FLINK/FLIP-360%3A+merging+the+executiongraphinfostore+and+the+jobresultstore+into+a+single+component+completedjobstore
[2]
https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=263428420&selectedPageVersions=14&selectedPageVersions=13

On Mon, Sep 18, 2023 at 1:20 AM Shammon FY  wrote:

> Hi Matthias,
>
> Thanks for initiating this discussion, and +1 for overall from my side.
> It's really strange to have two different places to store completed jobs,
> this also brings about the complexity of Flink. I agree with using a
> unified instance to store the completed job information.
>
> In terms of ability, `ExecutionGraphInfoStore` and `JobResultStore` are
> different: one is mainly used for information display, and the other is for
> failover. So after unifying storage, can we use different interfaces to
> meet the different requirements for jobs? Adding all these methods for
> different components into one interface such as `CompletedJobStore` may be
> a little strange. What do you think?
>
> Best,
> Shammon FY
>
>
>
> On Fri, Sep 8, 2023 at 8:08 PM Gyula Fóra  wrote:
>
> > Hi Matthias!
> >
> > Thank you for the detailed proposal, overall I am in favor of making this
> > unification to simplify the logic and make the integration for external
> > components more straightforward.
> > I will try to read through the proposal more carefully next week and
> > provide some detailed feedback.
> >
> > +1
> >
> > Thanks
> > Gyula
> >
> > On Fri, Sep 8, 2023 at 8:36 AM Matthias Pohl  > .invalid>
> > wrote:
> >
> > > Just a bit more elaboration on the question that we need to answer
> here:
> > Do
> > > we want to expose the internal ArchivedExecutionGraph data structure
> > > through JSON?
> > >
> > > - The JSON approach allows the user to have (almost) full access to the
> > > information (that would be otherwise derived from the REST API).
> > Therefore,
> > > there's no need to spin up a cluster to access this information.
> > > Any information that shall be exposed through the REST API needs to be
> > > well-defined in this JSON structure, though. Large parts of the
> > > ArchivedExecutionGraph data structure (essentially anything that shall
> be
> > > used to populate the REST API) become public domain, though, which puts
> > > more constraints on this data structure and makes it harder to change
> it
> > in
> > > the future.
> > >
> > > - The binary data approach allows us to keep the data structure itself
> > > internal. We have more control over what we want to expose by providing
> > > access points in the ClusterClient (e.g. just add a command to extract
> > the
> > > external storage path from the file).
> > >
> > > - The compromise (i.e. keeping ExecutionGraphInfoStore and
> JobResultStore
> > > separate and just expose the checkpoint information next to the
> JobResult
> > > in the JobResultStore file) would keep us the closest to the current
> > state,
> > > requires the least code changes and the least exposure of internal data
> > > structures. It would allow any system (like the Kubernetes Operator) to
> > > extract the checkpoint's external storage path. But we would still be
> > stuck
> > > with kind-of redundant components.
> > >
> > > From a user's perspective, I feel like the JSON approach is the best
> one
> > > because it gives him/her the most freedom to be independent of Flink
> > > binaries when handling completed jobs. But I see benefits from a Flink
> > > developer's perspective to not expose the entire data structure but use
> > the
> > > ClusterClient as an access point.
> > >
> > > The last option is my least favorite one: Moving the ExecutionGraphInfo
> > out
> > > of the JobManager seems to be the right thing to do when thinking about
> > > Flink's vision to become cloud-nativ

[jira] [Created] (FLINK-33170) HybridSourceSplitEnumerator causes dropped records in Hybrid Sources with 3+ sources

[Robert Hoyt (Jira)]

Robert Hoyt created FLINK-33170:
---

 Summary: HybridSourceSplitEnumerator causes dropped records in 
Hybrid Sources with 3+ sources
 Key: FLINK-33170
 URL: https://issues.apache.org/jira/browse/FLINK-33170
 Project: Flink
  Issue Type: Bug
  Components: Connectors / Common
Affects Versions: 1.17.1, 1.16.2, 1.15.4, 1.19.0, 1.18.1
Reporter: Robert Hoyt


Possibly related to FLINK-27916.

 

In all versions since 1.15.x there's a subtle bug 
`HybridSourceSplitEnumerator`'s when determining if it's time to move on to the 
next source:

```

finishedReaders.add(subtaskId);

if (finishedReaders.size() == context.currentParallelism()) {

  // move on to the next source if it exists

```

This snippet is correct, but when changing to the next source, 
`finishedReaders` is never cleared. So when processing the second source, the 
`finishedReaders.size()` check will return true when the _first_ subtask 
finishes. The hybrid source moves on to the next source if one exists, so any 
unsent records in other subtasks will get dropped.

 

Concrete example with three sources, two subtasks each:
 # subtask 0 finishes with the first source. `finishedReaders` has size 1
 # subtask 1 finishes with the first source. `finishedReaders` has size 2 now, 
and moves on to the second source
 # subtask 1 finishes with the first source. `finishedReaders.add(1)` doesn't 
change the set; `finishedReaders` still has size 2. So the hybrid source moves 
on to the third source.
 # subtask 0 wasn't finished with the second source, but receives the 
notification to move on. Any unsent records are lost. *Data loss.*
 # this continues to the last source. The source doesn't change over if at the 
last source so the race condition in step 3 never happens

 

So step 3 results in the race condition that will drop records indeterminately 
for all but the first source and last source.

In production this issue caused the loss of GBs to TBs of data depending on the 
sources. We fixed it in a private fork by clearing the `finishedReaders` set 
when changing to the next source.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)