Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BEAM-1180] Implement GearpumpPipelineResult #1661

Merged
merged 11 commits into from Jan 24, 2017

Conversation

manuzhang
Copy link
Contributor

@manuzhang manuzhang commented Dec 20, 2016

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.

@asfbot
Copy link

asfbot commented Dec 20, 2016

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

Build result: FAILURE

[...truncated 11540 lines...] at org.eclipse.aether.internal.impl.DefaultRepositorySystem.resolveDependencies(DefaultRepositorySystem.java:384) at org.apache.maven.project.DefaultProjectDependenciesResolver.resolve(DefaultProjectDependenciesResolver.java:205) ... 35 moreCaused by: org.eclipse.aether.resolution.ArtifactResolutionException: The following artifacts could not be resolved: org.apache.apex:apex-common:jar:3.5.0-SNAPSHOT, org.apache.apex:apex-engine:jar:3.5.0-SNAPSHOT: Could not find artifact org.apache.apex:apex-common:jar:3.5.0-SNAPSHOT in Nexus (http://repository.apache.org/snapshots) at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolve(DefaultArtifactResolver.java:444) at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolveArtifacts(DefaultArtifactResolver.java:246) at org.eclipse.aether.internal.impl.DefaultRepositorySystem.resolveDependencies(DefaultRepositorySystem.java:367) ... 36 moreCaused by: org.eclipse.aether.transfer.ArtifactNotFoundException: Could not find artifact org.apache.apex:apex-common:jar:3.5.0-SNAPSHOT in Nexus (http://repository.apache.org/snapshots) at org.eclipse.aether.connector.basic.ArtifactTransportListener.transferFailed(ArtifactTransportListener.java:39) at org.eclipse.aether.connector.basic.BasicRepositoryConnector$TaskRunner.run(BasicRepositoryConnector.java:355) at org.eclipse.aether.util.concurrency.RunnableErrorForwarder$1.run(RunnableErrorForwarder.java:67) ... 3 more2016-12-20T02:56:10.229 [ERROR] 2016-12-20T02:56:10.229 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2016-12-20T02:56:10.229 [ERROR] 2016-12-20T02:56:10.229 [ERROR] For more information about the errors and possible solutions, please read the following articles:2016-12-20T02:56:10.229 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/DependencyResolutionException2016-12-20T02:56:10.230 [ERROR] 2016-12-20T02:56:10.230 [ERROR] After correcting the problems, you can resume the build with the command2016-12-20T02:56:10.230 [ERROR] mvn -rf :beam-runners-apexchannel stoppedSetting status of efd1021 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/6092/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

@manuzhang
Copy link
Contributor Author

R: @kennknowles please review

@kennknowles
Copy link
Member

retest this please

@manuzhang
Copy link
Contributor Author

build failed due to apex dependency problem. Do I have to rebase on latest trunk?

@asfbot
Copy link

asfbot commented Dec 20, 2016

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

Build result: FAILURE

[...truncated 7702 lines...] at org.apache.maven.wagon.providers.http.httpclient.impl.DefaultBHttpClientConnection.receiveResponseHeader(DefaultBHttpClientConnection.java:161) at org.apache.maven.wagon.providers.http.httpclient.impl.conn.CPoolProxy.receiveResponseHeader(CPoolProxy.java:153) at org.apache.maven.wagon.providers.http.httpclient.protocol.HttpRequestExecutor.doReceiveResponse(HttpRequestExecutor.java:271) at org.apache.maven.wagon.providers.http.httpclient.protocol.HttpRequestExecutor.execute(HttpRequestExecutor.java:123) at org.apache.maven.wagon.providers.http.httpclient.impl.execchain.MainClientExec.execute(MainClientExec.java:254) at org.apache.maven.wagon.providers.http.httpclient.impl.execchain.ProtocolExec.execute(ProtocolExec.java:195) at org.apache.maven.wagon.providers.http.httpclient.impl.execchain.RetryExec.execute(RetryExec.java:86) at org.apache.maven.wagon.providers.http.httpclient.impl.execchain.RedirectExec.execute(RedirectExec.java:108) at org.apache.maven.wagon.providers.http.httpclient.impl.client.InternalHttpClient.doExecute(InternalHttpClient.java:184) at org.apache.maven.wagon.providers.http.httpclient.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:82) at org.apache.maven.wagon.providers.http.AbstractHttpClientWagon.execute(AbstractHttpClientWagon.java:832) at org.apache.maven.wagon.providers.http.AbstractHttpClientWagon.fillInputData(AbstractHttpClientWagon.java:983) ... 61 more2016-12-20T04:10:58.661 [ERROR] 2016-12-20T04:10:58.661 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2016-12-20T04:10:58.661 [ERROR] 2016-12-20T04:10:58.661 [ERROR] For more information about the errors and possible solutions, please read the following articles:2016-12-20T04:10:58.661 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/DependencyResolutionException2016-12-20T04:10:58.662 [ERROR] 2016-12-20T04:10:58.662 [ERROR] After correcting the problems, you can resume the build with the command2016-12-20T04:10:58.662 [ERROR] mvn -rf :beam-sdks-java-io-kinesischannel stoppedSetting status of efd1021 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/6095/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

@kennknowles
Copy link
Member

I see a different failure. Are we looking at different builds? Neither failure seems related to the Gearpump runner. Probably merging in master is a good idea.

@manuzhang
Copy link
Contributor Author

okay, let me sync first

@manuzhang
Copy link
Contributor Author

created #1663. Sorry it brings in a lot of commits.

@kennknowles
Copy link
Member

Merged #1663.

@asfbot
Copy link

asfbot commented Dec 22, 2016

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

@manuzhang
Copy link
Contributor Author

@kennknowles rebased. Please review again.

@manuzhang
Copy link
Contributor Author

not sure why travis is not pulling in latest gearpump snapshot

@asfbot
Copy link

asfbot commented Jan 4, 2017

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

@asfbot
Copy link

asfbot commented Jan 4, 2017

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

@kennknowles
Copy link
Member

Back from the holidays, taking a look.

Copy link
Member

@kennknowles kennknowles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just thinking about this, I think that for your branch, we may want to run the gearpump ROS tests by default to get very high signal. You can also now manually execute them for a pull request by going to https://builds.apache.org/view/Beam/job/beam_PostCommit_Java_RunnableOnService_Gearpump/build?delay=0sec

@@ -69,6 +69,7 @@
<dependenciesToScan>
<dependency>org.apache.beam:beam-sdks-java-core</dependency>
</dependenciesToScan>
<argLine>-noverify</argLine>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After recent changes on Gearpump build, launching JVM will throw verify errors. I haven't found the root cause and this is a work around.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a JIRA filed for this? We should sort it out at some point, and maybe someone with some expertise will get interested.

@@ -61,7 +61,7 @@ install:
- rm -rf "$HOME/.m2/repository/org/apache/beam"

script:
- travis_retry mvn --batch-mode --update-snapshots --no-snapshot-updates $MAVEN_OVERRIDE install && travis_retry bash -ex .travis/test_wordcount.sh
- travis_retry mvn --batch-mode --update-snapshots $MAVEN_OVERRIDE install && travis_retry bash -ex .travis/test_wordcount.sh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this may cause problems. I'll try to look into the references for the behavior here, but I think it will cause us to go to Maven Central for snapshots that we don't want to. This is to get the latest gearpump snapshot, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, my plan was to temporarily remove this in a separate commit which you can exclude with cherry-pick but it didn't work either.

cluster.stop();
assert(state == PipelineResult.State.DONE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here that you want to try to raise some underlying exception / give the user a message, or there are RunnnableOnService tests that won't pass. Or if I recall correctly, waitUntilFinish could throw. But I think that design decision has gone back and forth a couple of times.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I add that later and skip those tests for now since Gearpump doesn't have a good way to return status to users yet ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

@manuzhang
Copy link
Contributor Author

@kennknowles tried removing --no-snapshot-updates but the latest gearpump snapshot was not pulled in.

@kennknowles
Copy link
Member

Hmm, yes it seems that even with your change it is failing to compile due to the this, right?

@asfbot
Copy link

asfbot commented Jan 5, 2017

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

@asfbot
Copy link

asfbot commented Jan 5, 2017

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

@manuzhang
Copy link
Contributor Author

@kennknowles what else do I need to enable gearpump ROS besides the current configurations ?

@@ -64,11 +64,18 @@
</goals>
<configuration>
<groups>org.apache.beam.sdk.testing.RunnableOnService</groups>
<excludedGroups>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if you change <activeByDefault> to true then you should get ROS coverage in presubmit.

@asfbot
Copy link

asfbot commented Jan 7, 2017

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

Build result: FAILURE

[...truncated 13141 lines...] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:745)Caused by: org.apache.maven.plugin.MojoFailureException: There are test failures.Please refer to /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/runners/gearpump/target/surefire-reports for the individual test results. at org.apache.maven.plugin.surefire.SurefireHelper.reportExecution(SurefireHelper.java:91) at org.apache.maven.plugin.surefire.SurefirePlugin.handleSummary(SurefirePlugin.java:320) at org.apache.maven.plugin.surefire.AbstractSurefireMojo.executeAfterPreconditionsChecked(AbstractSurefireMojo.java:892) at org.apache.maven.plugin.surefire.AbstractSurefireMojo.execute(AbstractSurefireMojo.java:755) at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208) ... 31 more2017-01-07T03:38:48.440 [ERROR] 2017-01-07T03:38:48.440 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2017-01-07T03:38:48.440 [ERROR] 2017-01-07T03:38:48.440 [ERROR] For more information about the errors and possible solutions, please read the following articles:2017-01-07T03:38:48.440 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException2017-01-07T03:38:48.440 [ERROR] 2017-01-07T03:38:48.440 [ERROR] After correcting the problems, you can resume the build with the command2017-01-07T03:38:48.441 [ERROR] mvn -rf :beam-runners-gearpumpchannel stoppedSetting status of ea633d2 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/6461/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

@asfbot
Copy link

asfbot commented Jan 14, 2017

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

@asfbot
Copy link

asfbot commented Jan 16, 2017

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

@asfbot
Copy link

asfbot commented Jan 16, 2017

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

@manuzhang
Copy link
Contributor Author

@kennknowles not sure why this particular build MAVEN_OVERRIDE="-Peclipse-jdt -DskipTests $MAVEN_OVERRIDE $MAVEN_CONTAINER_OVERRIDE" CUSTOM_JDK="oraclejdk8" failed while other builds and ROS tests succeeded. Any ideas ?

@kennknowles
Copy link
Member

It sure looks like that profile is not getting the latest version of gearpump.

@asfbot
Copy link

asfbot commented Jan 20, 2017

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

Build result: FAILURE

[...truncated 7044 lines...] ... 61 moreCaused by: java.io.FileNotFoundException: /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall@2/.repository/org/apache/ant/ant/1.9.4/ant-1.9.4.jar (No such file or directory) at java.util.zip.ZipFile.open(Native Method) at java.util.zip.ZipFile.(ZipFile.java:219) at java.util.zip.ZipFile.(ZipFile.java:149) at java.util.jar.JarFile.(JarFile.java:166) at java.util.jar.JarFile.(JarFile.java:103) at sun.net.www.protocol.jar.URLJarFile.(URLJarFile.java:93) at sun.net.www.protocol.jar.URLJarFile.getJarFile(URLJarFile.java:69) at sun.net.www.protocol.jar.JarFileFactory.get(JarFileFactory.java:99) at sun.net.www.protocol.jar.JarURLConnection.connect(JarURLConnection.java:122) at org.apache.tools.ant.taskdefs.Antlib.createAntlib(Antlib.java:69) ... 62 more2017-01-20T00:49:46.238 [ERROR] 2017-01-20T00:49:46.238 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2017-01-20T00:49:46.238 [ERROR] 2017-01-20T00:49:46.238 [ERROR] For more information about the errors and possible solutions, please read the following articles:2017-01-20T00:49:46.238 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginExecutionException2017-01-20T00:49:46.238 [ERROR] 2017-01-20T00:49:46.238 [ERROR] After correcting the problems, you can resume the build with the command2017-01-20T00:49:46.238 [ERROR] mvn -rf :beam-sdks-java-io-google-cloud-platformchannel stoppedSetting status of 83abfc7 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/6666/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 85dcfbd on manuzhang:gearpump-runner-result into ** on apache:gearpump-runner**.

@asfbot
Copy link

asfbot commented Jan 20, 2017

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

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 85dcfbd on manuzhang:gearpump-runner-result into ** on apache:gearpump-runner**.

@asfbot
Copy link

asfbot commented Jan 20, 2017

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

@manuzhang
Copy link
Contributor Author

@kennknowles Removing cache for Gearpump fixes it.

@kennknowles
Copy link
Member

Great! That must mean it has something to do with the --no-snapshot-updates. I don't remember how that interacts with --update-snapshots and why we have both. The .travis.yml really should be commented when weird things like that are added. Sorry for the trouble!

@@ -59,6 +59,7 @@ before_install:
install:
# Removing this here protects from inadvertent caching
- rm -rf "$HOME/.m2/repository/org/apache/beam"
- rm -rf "$HOME/.m2/repository/org/apache/gearpump"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting that you could also do this by removing the cache: section. But I think you should keep it, because it saves a lot of traffic to Maven Central - it is mostly about the number of requests and the fact that they often fail.

@@ -69,6 +69,7 @@
<dependenciesToScan>
<dependency>org.apache.beam:beam-sdks-java-core</dependency>
</dependenciesToScan>
<argLine>-noverify</argLine>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a JIRA filed for this? We should sort it out at some point, and maybe someone with some expertise will get interested.

do {
try {
Thread.sleep(defaultWaitInterval.getMillis());
} catch (InterruptedException e) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think you want to do this a little differently: if it is an InterruptedException you want to note that the thread is interrupted.

https://github.com/apache/beam/blob/master/runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowPipelineJob.java#L172

Bigger picture, we should probably allow waitUntilFinish to throw InterruptedException. It is the canonical example of when that exception is appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I filed https://issues.apache.org/jira/browse/GEARPUMP-236 for -noverify. Should I file one in Beam as well ?

  2. Seems no conclusion has been reached for the redesign of PipelineResult and waitUntilFinish is esp. "controversial".

@kennknowles
Copy link
Member

OK LGTM. Do you having any squashing and rearranging of commits you would like to do? I don't think they should all be squashed, since many are not very related. So perhaps they are mostly fine as-is except for little fixups.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling d814857 on manuzhang:gearpump-runner-result into ** on apache:gearpump-runner**.

@asfbot
Copy link

asfbot commented Jan 20, 2017

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

@manuzhang
Copy link
Contributor Author

Nope. I consciously make small changes in each commit.

@manuzhang
Copy link
Contributor Author

@kennknowles any more comments ?

@kennknowles
Copy link
Member

Nope, sorry! I forgot to merge it after your last comment. Merging now.

@asfgit asfgit merged commit d814857 into apache:gearpump-runner Jan 24, 2017
asfgit pushed a commit that referenced this pull request Jan 24, 2017
  note thread is interrupted on InterruptedException
  Remove cache for Gearpump on travis
  reduce timeout to wait for result
  fix ParDo.BoundMulti translation
  return encoded key for GroupByKey translation
  support OutputTimeFn
  update to latest gearpump dsl function interface
  fix group by window
  activate ROS on Gearpump by default
  update ROS configurations
  [BEAM-1180] Implement GearpumpPipelineResult
@manuzhang manuzhang deleted the gearpump-runner-result branch February 14, 2017 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants