[BEAM-12277] Add flink 1.13 build target. #14719

dmvk · 2021-05-04T09:57:33Z

https://issues.apache.org/jira/browse/BEAM-12277

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) and mention them in a comment (R: @username).
Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

`ValidatesRunner` compliance status (on master branch)

Lang	ULR	Samza	Twister2
Go	---	---	---
Java
Python	---	---	---
XLang		---	---

Examples testing status on various runners

Lang	ULR	Dataflow	Flink	Samza	Spark	Twister2
Go	---	---	---	---	---	---	---
Java	---		---	---	---	---	---
Python	---	---	---	---	---	---	---
XLang	---	---	---	---	---	---	---

Post-Commit SDK/Transform Integration Tests Status (on master branch)

Go	Java	Python

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website	Whitespace	Typescript
Non-portable
Portable	---			---	---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

dmvk · 2021-05-04T10:10:54Z

R: @iemejia Would you mind taking a look at this one? I'm not sure if there's anything else needed for portable runner to work 🤔
CC: @ibzib

codecov · 2021-05-04T10:12:54Z

Codecov Report

Merging #14719 (881da20) into master (31988c8) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #14719   +/-   ##
=======================================
  Coverage   83.78%   83.78%           
=======================================
  Files         435      435           
  Lines       58423    58423           
=======================================
  Hits        48951    48951           
  Misses       9472     9472

Impacted Files	Coverage Δ
...hon/apache_beam/portability/api/schema_pb2_grpc.py
...xamples/snippets/transforms/elementwise/flatmap.py
...am/examples/snippets/transforms/elementwise/map.py
...hon/apache_beam/runners/worker/worker_pool_main.py
...ild/srcs/sdks/python/apache_beam/utils/__init__.py
.../python/apache_beam/runners/worker/statesampler.py
...rcs/sdks/python/apache_beam/typehints/typehints.py
.../python/apache_beam/testing/test_stream_service.py
...s/snippets/transforms/aggregation/combinevalues.py
...srcs/sdks/python/apache_beam/examples/wordcount.py
... and 860 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31988c8...881da20. Read the comment docs.

dmvk · 2021-05-04T12:05:53Z

Run Java PreCommit

iemejia · 2021-05-04T12:51:03Z

Can you please change

beam/.test-infra/jenkins/CommonTestProperties.groovy

Lines 28 to 30 in cc29b5b

    
           static String getFlinkVersion() { 
        
             return "1.12" 
        
           }

to be 1.13 so we can run the VR tests and other CI tests by default on the new version.
Also can you add 1.13 to the website https://github.com/apache/beam/blob/cc29b5b308b6ed91d787449b8105cc0b292273ac/website/www/site/content/en/documentation/runners/flink.md

iemejia · 2021-05-04T12:55:14Z

Also add a note on CHANGES.md -> 2.31.0 -> New Features

iemejia · 2021-05-04T12:57:52Z

and

beam/sdks/python/apache_beam/options/pipeline_options.py

Line 1272 in 2e9ee8c

PUBLISHED_FLINK_VERSIONS = ['1.8', '1.9', '1.10', '1.11', '1.12']

too (extra points if you remove the 1.8 and 1.9 versions that we don't support anymore)

ibzib · 2021-05-04T19:25:13Z

Besides the things Ismaël pointed out, we also need INFRA to create a new docker repo: INFRA-21833

iemejia · 2021-05-05T15:28:44Z

Docker repo seems to be up now https://hub.docker.com/r/apache/beam_flink1.13_job_server

iemejia · 2021-05-07T20:56:27Z

retest this please

iemejia · 2021-05-07T20:57:08Z

Run Flink ValidatesRunner

iemejia · 2021-05-07T20:57:31Z

Run Java Flink PortableValidatesRunner Batch

iemejia · 2021-05-07T20:57:43Z

Run Java Flink PortableValidatesRunner Streaming

ibzib

LGTM

iemejia · 2021-05-08T05:18:43Z

Arrghh seems metrics are broken for the Portable runner. Can you maybe what is the deal @ibzib ?
https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Phrase/131/

ibzib · 2021-05-10T16:50:18Z

Arrghh seems metrics are broken for the Portable runner. Can you maybe what is the deal @ibzib ?
https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Phrase/131/

Looks like it's hitting an NPE while unregistering metrics because the taskExecutorManager is null. This seems like a Flink bug to me -- if registers these metrics, they should remain accessible even after the slot manager has been shut down. Getting these metrics should probably return 0 rather than NPE.

May 08, 2021 4:57:46 AM org.apache.flink.runtime.metrics.MetricRegistryImpl unregister
WARNING: Error while unregistering metric: taskSlotsAvailable.
java.lang.NullPointerException
	at org.apache.flink.runtime.resourcemanager.slotmanager.DeclarativeSlotManager.getNumberFreeSlots(DeclarativeSlotManager.java:715)
	at org.apache.flink.runtime.resourcemanager.slotmanager.DeclarativeSlotManager.lambda$registerSlotManagerMetrics$2(DeclarativeSlotManager.java:200)
	at org.apache.beam.runners.flink.metrics.Metrics.toString(Metrics.java:33)
	at org.apache.beam.runners.flink.metrics.FileReporter.notifyOfRemovedMetric(FileReporter.java:72)
	at org.apache.flink.runtime.metrics.MetricRegistryImpl.unregister(MetricRegistryImpl.java:435)
	at org.apache.flink.runtime.metrics.groups.AbstractMetricGroup.close(AbstractMetricGroup.java:333)
	at org.apache.flink.runtime.resourcemanager.slotmanager.DeclarativeSlotManager.close(DeclarativeSlotManager.java:241)
	at org.apache.flink.runtime.resourcemanager.ResourceManager.stopResourceManagerServices(ResourceManager.java:298)
	at org.apache.flink.runtime.resourcemanager.ResourceManager.onStop(ResourceManager.java:275)
	at org.apache.flink.runtime.rpc.RpcEndpoint.internalCallOnStop(RpcEndpoint.java:214)
	at org.apache.flink.runtime.rpc.akka.AkkaRpcActor$StartedState.terminate(AkkaRpcActor.java:563)
	at org.apache.flink.runtime.rpc.akka.AkkaRpcActor.handleControlMessage(AkkaRpcActor.java:186)
	at akka.japi.pf.UnitCaseStatement.apply(CaseStatements.scala:26)
	at akka.japi.pf.UnitCaseStatement.apply(CaseStatements.scala:21)
	at scala.PartialFunction$class.applyOrElse(PartialFunction.scala:123)
	at akka.japi.pf.UnitCaseStatement.applyOrElse(CaseStatements.scala:21)
	at scala.PartialFunction$OrElse.applyOrElse(PartialFunction.scala:170)
	at scala.PartialFunction$OrElse.applyOrElse(PartialFunction.scala:171)
	at akka.actor.Actor$class.aroundReceive(Actor.scala:517)
	at akka.actor.AbstractActor.aroundReceive(AbstractActor.scala:225)
	at akka.actor.ActorCell.receiveMessage(ActorCell.scala:592)
	at akka.actor.ActorCell.invoke(ActorCell.scala:561)
	at akka.dispatch.Mailbox.processMailbox(Mailbox.scala:258)
	at akka.dispatch.Mailbox.run(Mailbox.scala:225)
	at akka.dispatch.Mailbox.exec(Mailbox.scala:235)
	at akka.dispatch.forkjoin.ForkJoinTask.doExec(ForkJoinTask.java:260)
	at akka.dispatch.forkjoin.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1339)
	at akka.dispatch.forkjoin.ForkJoinPool.runWorker(ForkJoinPool.java:1979)
	at akka.dispatch.forkjoin.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:107)

May 08, 2021 4:57:46 AM org.apache.flink.runtime.metrics.MetricRegistryImpl unregister
WARNING: Error while unregistering metric: taskSlotsTotal.
java.lang.NullPointerException
	at org.apache.flink.runtime.resourcemanager.slotmanager.DeclarativeSlotManager.getNumberRegisteredSlots(DeclarativeSlotManager.java:705)
	at org.apache.flink.runtime.resourcemanager.slotmanager.DeclarativeSlotManager.lambda$registerSlotManagerMetrics$3(DeclarativeSlotManager.java:202)
	at org.apache.beam.runners.flink.metrics.Metrics.toString(Metrics.java:33)
	at org.apache.beam.runners.flink.metrics.FileReporter.notifyOfRemovedMetric(FileReporter.java:72)
	at org.apache.flink.runtime.metrics.MetricRegistryImpl.unregister(MetricRegistryImpl.java:435)
	at org.apache.flink.runtime.metrics.groups.AbstractMetricGroup.close(AbstractMetricGroup.java:333)
	at org.apache.flink.runtime.resourcemanager.slotmanager.DeclarativeSlotManager.close(DeclarativeSlotManager.java:241)
	at org.apache.flink.runtime.resourcemanager.ResourceManager.stopResourceManagerServices(ResourceManager.java:298)
	at org.apache.flink.runtime.resourcemanager.ResourceManager.onStop(ResourceManager.java:275)
	at org.apache.flink.runtime.rpc.RpcEndpoint.internalCallOnStop(RpcEndpoint.java:214)
	at org.apache.flink.runtime.rpc.akka.AkkaRpcActor$StartedState.terminate(AkkaRpcActor.java:563)
	at org.apache.flink.runtime.rpc.akka.AkkaRpcActor.handleControlMessage(AkkaRpcActor.java:186)
	at akka.japi.pf.UnitCaseStatement.apply(CaseStatements.scala:26)
	at akka.japi.pf.UnitCaseStatement.apply(CaseStatements.scala:21)
	at scala.PartialFunction$class.applyOrElse(PartialFunction.scala:123)
	at akka.japi.pf.UnitCaseStatement.applyOrElse(CaseStatements.scala:21)
	at scala.PartialFunction$OrElse.applyOrElse(PartialFunction.scala:170)
	at scala.PartialFunction$OrElse.applyOrElse(PartialFunction.scala:171)
	at akka.actor.Actor$class.aroundReceive(Actor.scala:517)
	at akka.actor.AbstractActor.aroundReceive(AbstractActor.scala:225)
	at akka.actor.ActorCell.receiveMessage(ActorCell.scala:592)
	at akka.actor.ActorCell.invoke(ActorCell.scala:561)
	at akka.dispatch.Mailbox.processMailbox(Mailbox.scala:258)
	at akka.dispatch.Mailbox.run(Mailbox.scala:225)
	at akka.dispatch.Mailbox.exec(Mailbox.scala:235)
	at akka.dispatch.forkjoin.ForkJoinTask.doExec(ForkJoinTask.java:260)
	at akka.dispatch.forkjoin.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1339)
	at akka.dispatch.forkjoin.ForkJoinPool.runWorker(ForkJoinPool.java:1979)
	at akka.dispatch.forkjoin.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:107)

ibzib · 2021-05-10T16:55:24Z

Or perhaps these metrics should be unregistered when the slot manager is shut down, before the taskExecutorManager is nulled out.

dmvk · 2021-05-11T10:08:30Z

I don't think this is a regression, I'm pretty sure we've seen this in prior versions too. Maybe we can pull this into separate issue?

dmvk · 2021-05-11T10:24:50Z

I think this boils down to whether we want to wait for the issue being fixed in Flink or we want a "quick-win" solution 🤔

Maybe we can temporarily work around this in FileReporter, before fixing the issue upstream?

iemejia · 2021-05-12T11:49:21Z

Run Flink ValidatesRunner

iemejia · 2021-05-12T11:49:35Z

Run Java Flink PortableValidatesRunner Batch

iemejia · 2021-05-12T11:49:47Z

Run Java Flink PortableValidatesRunner Streaming

dmvk · 2021-05-12T11:50:33Z

issue with PR on the flink's side - https://issues.apache.org/jira/browse/FLINK-22646

iemejia · 2021-05-12T12:10:02Z

Run Flink ValidatesRunner

iemejia · 2021-05-12T12:10:12Z

Run Java Flink PortableValidatesRunner Batch

iemejia · 2021-05-12T12:10:27Z

Run Java Flink PortableValidatesRunner Streaming

ibzib · 2021-05-12T18:32:25Z

Looks like the metric name formatting changed, I added a commit to update it.

iemejia · 2021-05-12T18:47:04Z

Run Flink ValidatesRunner

iemejia · 2021-05-12T18:47:12Z

Run Java Flink PortableValidatesRunner Batch

iemejia · 2021-05-12T18:47:21Z

Run Java Flink PortableValidatesRunner Streaming

iemejia · 2021-05-15T11:21:20Z

Run Java PreCommit

dmvk · 2021-05-18T15:06:54Z

@iemejia @ibzib do you think we can still make it into 2.30.x release?

iemejia · 2021-05-18T15:27:49Z

i think there is not an RC1 yet so maybe we can convince the release manager to get it in, but well in that case we got to solve the still open jacoco issue to get this into master first.

aaltay · 2021-05-27T22:47:04Z

What is the next step on this PR?

I would not recommend cherry picking this to the current release.

ibzib · 2021-05-27T22:50:46Z

What is the next step on this PR?

I would not recommend cherry picking this to the current release.

As far as I know there's one outstanding issue, the Java precommit failure with Jacoco. We won't cherry pick this. Maybe 2.31.0.

ibzib · 2021-05-27T23:00:02Z

Looks like it was just a typo in the source overrides. I added another commit to fix it.

@dmvk can you rebase this?

ibzib · 2021-05-28T00:02:55Z

Run Java PreCommit

iemejia · 2021-05-28T08:14:41Z

I rebased it (and push forced it) since the conflict was on my commit. This should be good now!

iemejia · 2021-05-28T09:22:16Z

Run Java PreCommit

iemejia · 2021-05-28T09:23:50Z

Run Flink ValidatesRunner

iemejia · 2021-05-28T09:23:59Z

Run Java Flink PortableValidatesRunner Batch

iemejia · 2021-05-28T09:24:07Z

Run Java Flink PortableValidatesRunner Streaming

iemejia · 2021-05-28T09:24:11Z

Run Java Spark PortableValidatesRunner Batch

iemejia · 2021-05-28T13:38:49Z

Failing test is a well know flake. Thanks @ibzib for unblocking this one and of course @dmvk for bringing it in!

iemejia self-requested a review May 4, 2021 12:51

ibzib approved these changes May 7, 2021

View reviewed changes

iemejia force-pushed the BEAM-12277 branch from f7e76e1 to 871c7e7 Compare May 12, 2021 11:48

iemejia force-pushed the BEAM-12277 branch from 871c7e7 to ee40604 Compare May 12, 2021 12:09

dmvk and others added 4 commits May 28, 2021 10:12

[BEAM-12277] Add flink 1.13 build target.

b342805

[BEAM-12277] Make Flink 1.13 the default Flink version to run the tests

8ad8965

[BEAM-12277] Update expected metric name formatting.

8be1cb1

[BEAM-12277] fix mistake in source overrides

881da20

iemejia force-pushed the BEAM-12277 branch from 57f3f33 to 881da20 Compare May 28, 2021 08:14

iemejia merged commit 00eb420 into apache:master May 28, 2021

[BEAM-12277] Add flink 1.13 build target. #14719

[BEAM-12277] Add flink 1.13 build target. #14719

Conversation

dmvk commented May 4, 2021

ValidatesRunner compliance status (on master branch)

Examples testing status on various runners

Post-Commit SDK/Transform Integration Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

GitHub Actions Tests Status (on master branch)

dmvk commented May 4, 2021 • edited by iemejia

codecov bot commented May 4, 2021 • edited

Codecov Report

dmvk commented May 4, 2021

iemejia commented May 4, 2021 • edited

iemejia commented May 4, 2021

iemejia commented May 4, 2021

ibzib commented May 4, 2021 • edited by iemejia

iemejia commented May 5, 2021

iemejia commented May 7, 2021

iemejia commented May 7, 2021

iemejia commented May 7, 2021

iemejia commented May 7, 2021

ibzib left a comment

Choose a reason for hiding this comment

iemejia commented May 8, 2021

ibzib commented May 10, 2021

ibzib commented May 10, 2021 • edited

dmvk commented May 11, 2021

dmvk commented May 11, 2021

iemejia commented May 12, 2021

iemejia commented May 12, 2021

iemejia commented May 12, 2021

dmvk commented May 12, 2021

iemejia commented May 12, 2021

iemejia commented May 12, 2021

iemejia commented May 12, 2021

ibzib commented May 12, 2021

iemejia commented May 12, 2021

iemejia commented May 12, 2021

iemejia commented May 12, 2021

iemejia commented May 15, 2021

dmvk commented May 18, 2021

iemejia commented May 18, 2021

aaltay commented May 27, 2021

ibzib commented May 27, 2021

ibzib commented May 27, 2021

ibzib commented May 28, 2021

iemejia commented May 28, 2021

iemejia commented May 28, 2021

iemejia commented May 28, 2021

iemejia commented May 28, 2021

iemejia commented May 28, 2021

iemejia commented May 28, 2021

iemejia commented May 28, 2021

`ValidatesRunner` compliance status (on master branch)

dmvk commented May 4, 2021 •

edited by iemejia

codecov bot commented May 4, 2021 •

edited

iemejia commented May 4, 2021 •

edited

ibzib commented May 4, 2021 •

edited by iemejia

ibzib commented May 10, 2021 •

edited