[BEAM-1139] Add no-arg constructor for UnboundedReadFromBoundedSource #1586

tgroh · 2016-12-12T23:57:41Z

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

Make sure the PR title is formatted like:
[BEAM-<Jira issue #>] Description of pull request
Make sure tests pass via mvn clean verify. (Even better, enable
Travis-CI on your fork and ensure the whole test matrix passes).
Replace <Jira issue #> in the title with the actual Jira issue
number, if there is one.
If this contribution is large, please file an Apache
Individual Contributor License Agreement.

This allows Kryo to work with the type, currently required by
the Apex runner.

tgroh · 2016-12-13T00:06:13Z

My current exception

java.lang.InstantiationError: org.apache.beam.sdk.io.UnboundedSource

	at org.apache.beam.sdk.io.UnboundedSourceConstructorAccess.newInstance(Unknown Source)
	at com.esotericsoftware.kryo.Kryo$1.newInstance(Kryo.java:1015)
	at com.esotericsoftware.kryo.Kryo.newInstance(Kryo.java:1065)
	at com.esotericsoftware.kryo.serializers.FieldSerializer.create(FieldSerializer.java:228)
	at com.esotericsoftware.kryo.serializers.FieldSerializer.read(FieldSerializer.java:217)
	at com.esotericsoftware.kryo.Kryo.readObject(Kryo.java:626)
	at org.apache.beam.runners.core.UnboundedReadFromBoundedSourceTest.testAdapterKryoSerialization(UnboundedReadFromBoundedSourceTest.java:143)

tgroh · 2016-12-13T00:14:48Z

I get that with a public or private no-arg constructor, with a final or non-final class, and with a final or non-final boundedSource field.

This allows Kryo to work with the type, currently required by the Apex runner.

tgroh · 2016-12-13T00:34:57Z

R: @tweise @lukecwik

The serialization test passes in the DirectRunner Module locally.

kennknowles · 2016-12-13T01:14:18Z

Confirmed the fix for just the error in question manually, since Jenkins is severely backlogged.

mvn -DskipTests clean install -P apex-runner -pl examples/java -am

mvn --errors failsafe:integration-test@apex-runner-integration-tests \
    -pl examples/java \
    -P apex-runner \
    -P jenkins-precommit \
    -D test=WordCountIT \
    -D beamTestPipelineOptions='[
      "--runner=org.apache.beam.runners.apex.TestApexRunner",
      "--project=...",
      "--tempRoot=..."
    ]'

(deliberately trying to sprinkle knowledge and record of these incantations here and there)

dhalperi · 2016-12-13T01:25:08Z

runners/direct-java/pom.xml

+    <dependency>
+      <groupId>com.esotericsoftware.kryo</groupId>
+      <artifactId>kryo</artifactId>
+      <version>2.21</version>


please do not put versions in submodules -- top module only.

why is this needed at all -- isn't it picked up from the fact that this depends on runner-core?

davorbonaci · 2016-12-13T01:28:43Z

Can we investigate alternatives that don't involve kryo proliferation? I'd ask to please hold the PR in the meanwhile.

kennknowles · 2016-12-13T01:40:49Z

@davorbonaci this is precommit breakage

kennknowles · 2016-12-13T01:46:49Z

runners/core-java/pom.xml

+    <dependency>
+      <groupId>com.esotericsoftware.kryo</groupId>
+      <artifactId>kryo</artifactId>
+      <version>2.21</version>


It is unfortunate to need a version here. Do put the version into dependencyManagement regardless.

The trouble, for anyone looking at this line, is that the Spark runner and Apex runner pu
ll in Kryo at different versions. We could choose a version for Beam just for common style, but it would require overriding in one or the other, or both.

There shoudn't be any Kryo dependency in runners core. If at all, then this test should be in the Apex runner.

lukecwik · 2016-12-13T01:57:50Z

@davorbonaci There is currently no hard specification as to how objects need to be serialized. Kryo proliferation comes from runners relying on Kryo as their defacto serialization mechanism.

dhalperi · 2016-12-13T01:58:37Z

But Spark, for example, serializes sources and coders using java serialization as is expected today by Beam.

tweise · 2016-12-13T02:07:43Z

I'm going to look at it tonight. I believe Java serialization can be used for this reference in the runner also.

kennknowles · 2016-12-13T02:12:20Z

Thanks @tweise! Agree that we should hold off on a quick fix, then, if you think you can just avoid the whole issue. It will save us debating the relationship of Kryo and Beam :-)

davorbonaci · 2016-12-13T02:13:39Z

@tgroh seemed to have reduced scope of the change. With that, LGTM. Good for self-merge.

tweise · 2016-12-13T02:17:42Z

We can potentially improve it later to. Generally, if the class is Java serializable, then use

@Bind(JavaSerializer.class)

in the Apex operator code.

asfbot · 2016-12-13T03:13:13Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/5797/

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

org.apache.beam.examples.WordCountIT.testE2EWordCount

--none--

asfbot · 2016-12-13T04:05:30Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/5803/

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

org.apache.beam.examples.WordCountIT.testE2EWordCount

--none--

asfbot · 2016-12-13T04:31:26Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/5807/

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

org.apache.beam.examples.WordCountIT.testE2EWordCount

--none--

asfbot · 2016-12-13T04:49:25Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/5808/

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

org.apache.beam.examples.WordCountIT.testE2EWordCount

--none--

tweise · 2016-12-13T05:52:42Z

I'm looking at it, not sure what the issue is, the field is marked for Java serialization in master:

Caused by: com.esotericsoftware.kryo.KryoException: Class cannot be created (missing no-arg constructor): org.apache.beam.sdk.io.CompressedSource
Serialization trace:
boundedSource (org.apache.beam.runners.core.UnboundedReadFromBoundedSource$BoundedToUnboundedSourceAdapter)
source (org.apache.beam.runners.apex.translation.operators.ApexReadUnboundedInputOperator)
	at com.esotericsoftware.kryo.Kryo.newInstantiator(Kryo.java:1050)

  @Bind(JavaSerializer.class)
  private final UnboundedSource<OutputT, CheckpointMarkT> source;

There should be no Kryo exception.

tweise · 2016-12-13T06:34:25Z

I notice that the @Bind annotation that controls the serializer is only present in Kryo 2.24 (used by Apex), not in 2.21. Is there a dependency leak in the examples module?

dhalperi · 2016-12-13T13:29:33Z

Sure looks like it. This may indicate a flaw in our testing strategy. We have K profiles for the K runners we have, and they work fine with proper transitive dependency resolution when used individually. Precommit, however, we simply turn on all the profiles and hope that enabling them all concurrently just works. (Which it mostly, sorta, has.)

…

On Mon, Dec 12, 2016 at 10:34 PM, Thomas Weise ***@***.***> wrote: I notice that the @Bind <https://github.com/Bind> annotation that controls the serializer is only present in Kryo 2.24 (used by Apex), not in 2.21. Is there a dependency leak in the examples module? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1586 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAgIT5ua-KrAAvY2lyPrV0TVH7SJXWrhks5rHjxzgaJpZM4LLLau> .

@Bind

This commits merged the following commits: 47cc2dc Add Tests for Kryo Serialization of URFBS 3c2e550 Add no-arg constructor for UnboundedReadFromBoundedSource Kryo support is, in fact, not needed for UnboundedReadFromBoundedSource, but we have a dependency issue where the Apex runner's dependency on Kryo was being overridden with an older version that does not support the @Bind annotations. All runners and poms are independently correct except the examples pom which merges the dependencies from all.

@Bind

The above commit was a merge of the following commits to master: 47cc2dc Add Tests for Kryo Serialization of URFBS 3c2e550 Add no-arg constructor for UnboundedReadFromBoundedSource Kryo support is, in fact, not needed for UnboundedReadFromBoundedSource, but we have a dependency issue where the Apex runner's dependency on Kryo was being overridden with an older version that does not support the @Bind annotations. All runners and poms are independently correct except the examples pom which merges the dependencies from all.

@Bind

The above commit was a merge of the following commits to master: 47cc2dc Add Tests for Kryo Serialization of URFBS 3c2e550 Add no-arg constructor for UnboundedReadFromBoundedSource Kryo support is, in fact, not needed for UnboundedReadFromBoundedSource, but we have a dependency issue where the Apex runner's dependency on Kryo was being overridden with an older version that does not support the @Bind annotations. All runners and poms are independently correct except the examples pom which merges the dependencies from all.

@test

Adjustments in gearpump-runner: [BEAM-79] Upgrade to beam-0.5.0-incubating-SNAPSHOT [BEAM-79] Update to latest Gearpump API From master: Disable automatic archiving of Maven builds [BEAM-59] initial interfaces and classes of Beam FileSystem. Change counter name in TestDataflowRunner More escaping in Jenkins timestamp spec Add RunnableOnService test for Metrics Fix seed job fetch spec Show timestamps on log lines in Jenkins [BEAM-1165] Fix unexpected file creation when checking dependencies [BEAM-1178] Make naming of logger objects consistent [BEAM-716] Fix javadoc on with* methods [BEAM-959] Improve check preconditions in JmsIO [BEAM-716] Use AutoValue in JmsIO Fix grammar error (repeated for) Empty TestPipeline need not be run [BEAM-85, BEAM-298] Make TestPipeline a JUnit Rule checking proper usage Change counter name in TestDataflowRunner BigQueryIO: fix streaming write, typo in API [BEAM-853] Force streaming execution on batch pipelines for testing. Expose the adapted source. Use empty SideInputReader, fixes NPE in SimpleDoFnRunnerTest Test that SimpleDoFnRunner wraps exceptions in startBundle and finishBundle Add timer support to DoFnRunner(s) Make TimerSpec and StateSpec fields accessible View.asMap: minor javadoc fixes Revert "Move InMemoryTimerInternals to runners-core" Revert "Moves DoFnAdapters to runners-core" Revert "Removes ArgumentProvider.windowingInternals" Revert "Removes code for wrapping DoFn as an OldDoFn" checkstyle: missed newline in DistributionCell Make {Metric,Counter,Distribution}Cell public Add PTransformOverrideFactory to the Core SDK Move ActiveWindowSet and implementations to runners-core Update Dataflow worker to beam-master-20161216 [BEAM-1108] Remove outdated language about experimental autoscaling [BEAM-450] Shade modules to separate paths [BEAM-362] Port runners to runners-core AggregatoryFactory Move InMemoryTimerInternals to runners-core Delete deprecated TimerCallback Remove deprecated methods of InMemoryTimerInternals Don't incorrectly log error in MetricsEnvironment Renames ParDo.getNewFn to getFn Moves DoFnAdapters to runners-core Removes unused code from NoOpOldDoFn Removes ArgumentProvider.windowingInternals Removes code for wrapping DoFn as an OldDoFn Removes OldDoFn from ParDo Pushes uses of OldDoFn deeper inside Flink runner Remove ParDo.of(OldDoFn) from Apex runner Converts all easy OldDoFns to DoFn [BEAM-1022] Add testing coverage for BigQuery streaming writes Fix mvn command args in Apex postcommit Jenkins job [BEAM-932] Enable findbugs validation (and fix existing issues) Fail to split in FileBasedSource if filePattern expands to empty. [BEAM-1154] Get side input from proper window in ReduceFn [BEAM-1153] GcsUtil: use non-batch API for single file size requests. Fix NPE in StatefulParDoEvaluatorFactoryTest mocking [BEAM-1033] Retry Bigquery Verifier when Query Fails Implement GetDefaultOutputCoder in DirectGroupByKey SimpleDoFnRunner observes window if SideInputReader is nonempty Better comments and cleanup Allow empty string value for ValueProvider types. starter: fix typo in pom.xml Revert "Allow stateful DoFn in DataflowRunner" Re-exclude UsesStatefulParDo tests for Dataflow Some minor changes and fixes for sorter module [BEAM-1149] Explode windows when fn uses side inputs Add Jenkins postcommit for RunnableOnService in Apex runner Update version from 0.5.0-SNAPSHOT to 0.5.0-incubating-SNAPSHOT Update Maven Archetype versions after cutting the release branch Move PerKeyCombineFnRunner to runners-core Update Dataflow worker to beam-master-20161212 [maven-release-plugin] prepare for next development iteration [maven-release-plugin] prepare branch release-0.4.0-incubating Fix version of Kryo in examples/java jenkins-precommit profile Revert 91cc606 "This closes #1586": Kryo + UBRFBS [BEAM-909] improve starter archetype Fix JDom malformed comment in Apex runner. [BEAM-927] Fix findbugs and re-enable Maven plugin in JmsIO [BEAM-807] Replace OldDoFn with DoFn. [BEAM-757] Use DoFnRunner in the implementation of DoFn via FlatMapFunction. FileBasedSinkTest: fix tests in Windows OS by using IOChannelUtils.resolve(). FileBasedSink: ignore exceptions when removing temp output files for issues in Windows OS. [BEAM-1142] Upgrade maven-invoker to address maven bug ARCHETYPE-488. Add Tests for Kryo Serialization of URFBS Add no-arg constructor for UnboundedReadFromBoundedSource Revise WindowedWordCount for runner and execution mode portability Factor out ShardedFile from FileChecksumMatcher Add IntervalWindow coder to the standard registry Stop expanding PValues in DirectRunner visitors Migrate AppliedPTransform to use AutoValue Enable and fix DirectRunnerTest case missing @test [BEAM-1130] SparkRunner ResumeFromCheckpointStreamingTest Failing. [BEAM-1133] Add maxNumRecords per micro-batch for Spark runner options. BigQueryIO.Write: support runtime schema and table Fix handling of null ValueProviders in DisplayData [BEAM-551] Fix handling of default for VP [BEAM-1120] Move some DataflowRunner configurations from code to properties [BEAM-551] Fix toString for FileBasedSource [BEAM-921] spark-runner: register sources and coders to serialize with java serializer [BEAM-551] Fix handling of TextIO.Sink ...

Add no-arg constructor for UnboundedReadFromBoundedSource

db7b7cf

This allows Kryo to work with the type, currently required by the Apex runner.

tgroh force-pushed the take_over_1585 branch from e88e9ed to c8b57a0 Compare December 13, 2016 00:34

dhalperi reviewed Dec 13, 2016

View reviewed changes

kennknowles reviewed Dec 13, 2016

View reviewed changes

tgroh force-pushed the take_over_1585 branch from c8b57a0 to c5f7bda Compare December 13, 2016 02:11

Add Tests for Kryo Serialization of URFBS

9ec9ac4

tgroh force-pushed the take_over_1585 branch from c5f7bda to 9ec9ac4 Compare December 13, 2016 02:12

asfgit closed this in 91cc606 Dec 13, 2016

kennknowles mentioned this pull request Dec 13, 2016

[BEAM-1139] Revert Apex+Kryo PR and fix via deps #1597

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BEAM-1139] Add no-arg constructor for UnboundedReadFromBoundedSource #1586

[BEAM-1139] Add no-arg constructor for UnboundedReadFromBoundedSource #1586

tgroh commented Dec 12, 2016 •

edited

tgroh commented Dec 13, 2016

tgroh commented Dec 13, 2016

tgroh commented Dec 13, 2016

kennknowles commented Dec 13, 2016 •

edited

dhalperi Dec 13, 2016

dhalperi Dec 13, 2016

davorbonaci commented Dec 13, 2016

kennknowles commented Dec 13, 2016

kennknowles Dec 13, 2016

tweise Dec 13, 2016

lukecwik commented Dec 13, 2016

dhalperi commented Dec 13, 2016

tweise commented Dec 13, 2016

kennknowles commented Dec 13, 2016

davorbonaci commented Dec 13, 2016 •

edited

tweise commented Dec 13, 2016

asfbot commented Dec 13, 2016

asfbot commented Dec 13, 2016

asfbot commented Dec 13, 2016

asfbot commented Dec 13, 2016

tweise commented Dec 13, 2016

tweise commented Dec 13, 2016

dhalperi commented Dec 13, 2016 via email

[BEAM-1139] Add no-arg constructor for UnboundedReadFromBoundedSource #1586

[BEAM-1139] Add no-arg constructor for UnboundedReadFromBoundedSource #1586

Conversation

tgroh commented Dec 12, 2016 • edited

tgroh commented Dec 13, 2016

tgroh commented Dec 13, 2016

tgroh commented Dec 13, 2016

kennknowles commented Dec 13, 2016 • edited

dhalperi Dec 13, 2016

Choose a reason for hiding this comment

dhalperi Dec 13, 2016

Choose a reason for hiding this comment

davorbonaci commented Dec 13, 2016

kennknowles commented Dec 13, 2016

kennknowles Dec 13, 2016

Choose a reason for hiding this comment

tweise Dec 13, 2016

Choose a reason for hiding this comment

lukecwik commented Dec 13, 2016

dhalperi commented Dec 13, 2016

tweise commented Dec 13, 2016

kennknowles commented Dec 13, 2016

davorbonaci commented Dec 13, 2016 • edited

tweise commented Dec 13, 2016

asfbot commented Dec 13, 2016

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

asfbot commented Dec 13, 2016

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

asfbot commented Dec 13, 2016

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

asfbot commented Dec 13, 2016

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

tweise commented Dec 13, 2016

tweise commented Dec 13, 2016

dhalperi commented Dec 13, 2016 via email

tgroh commented Dec 12, 2016 •

edited

kennknowles commented Dec 13, 2016 •

edited

davorbonaci commented Dec 13, 2016 •

edited