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

[FLINK-4450]update storm verion to 1.0.0 in flink-storm and flink-storm-examples #2439

Closed
wants to merge 299 commits into from

Conversation

liuyuzhong
Copy link

@liuyuzhong liuyuzhong commented Aug 30, 2016

This pull is for update storm version to 1.0.0 https://issues.apache.org/jira/browse/FLINK-4450

Because Apache Storm version after 1.0.0, the pacakge name was changed from "backtype." to "org.apache.", and this change will work for all the storm which version higher than 1.0.0

build successfully and run successfully in my test environment.

@liuyuzhong liuyuzhong changed the title update storm verion to 1.0.0 in flink-storm and flink-storm-examples [FLINK-4450]update storm verion to 1.0.0 in flink-storm and flink-storm-examples Aug 30, 2016
@liuyuzhong
Copy link
Author

mvn clean verify : ok

test flink-storm-examples: ok
bin/flink run WordCount-StormTopology.jar
bin/flink run WordCount-BoltTokenizer.jar
bin/flink run WordCount-SpoutSource.jar

@liuyuzhong
Copy link
Author

Jenkins run error:

Running org.apache.flink.test.checkpointing.PartitionedStateCheckpointingITCase
SUREFIRE-859: Java HotSpot(TM) 64-Bit Server VM warning: INFO: os::commit_memory(0x00000000d8b00000, 100663296, 0) failed; error='Cannot allocate memory' (errno=12)
#
# There is insufficient memory for the Java Runtime Environment to continue.
# Native memory allocation (mmap) failed to map 100663296 bytes for committing reserved memory.
# An error report file with more information is saved as:
# /home/jenkins/jenkins-slave/workspace/flink-github-ci/flink-tests/target/hs_err_pid9818.log
Running org.apache.flink.test.checkpointing.WindowCheckpointingITCase
ERROR: Maven JVM terminated unexpectedly with exit code 137
Putting comment on the pull request
Finished: FAILURE

But I run it successfully in my environment, does it a Jenkins memory problem? Rerun it in Jenkins?

@liuyuzhong liuyuzhong closed this Aug 31, 2016
@liuyuzhong liuyuzhong reopened this Aug 31, 2016
@@ -186,15 +177,15 @@ public void testCreateTopologyContext() {
.shuffleGrouping("bolt2", TestDummyBolt.groupingStreamId)
.shuffleGrouping("bolt2", TestDummyBolt.shuffleStreamId);

LocalCluster cluster = new LocalCluster();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why `did you modify this code?

Copy link
Author

Choose a reason for hiding this comment

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

It can't work, so delete it first.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you elaborate a bit why this cannot work any more with Storm 1.0?

@StephanEwen
Copy link
Contributor

There is an interesting question about how to proceed with that.

If we merge this, the compatibility layer will only support Storm 1.0 programs, which are incompatible with prior versions. I would assume a lot of people have Storm jobs with prior versions.

In some sense we would need two compatibility projects, one for Storm pre-1.0 and one for Storm post-1.0.

@liuyuzhong
Copy link
Author

liuyuzhong commented Sep 1, 2016

@StephanEwen
If we merge this, flink-storm will compatible with all the storm which version higher than 1.0.0

And we can import "storm-rename-hack" to compatible with previous storm version. It can change class package name previous storm version jar.

see the detail: https://github.com/apache/storm/tree/master/storm-rename-hack

comments update:

Sorry, It can't work with previous storm version. I will add two new module "flink-storm-post-1.0" "flink-storm-examples-post-1.0" in flink-contrib. What do you think?

@liuyuzhong
Copy link
Author

liuyuzhong commented Sep 1, 2016

close it, and will open new pull request which add new module "flink-apache-storm" "flink-apache-storm-examples" later.

flink-apache-storm: campatible for storm version higher than 1.0.0

@liuyuzhong
Copy link
Author

Jenkins run successful, but CI fail.

image

@liuyuzhong
Copy link
Author

Jenkins run error
1.build success

[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:18 h
[INFO] Finished at: 2016-10-09T12:42:31+00:00
[INFO] Final Memory: 210M/1154M

2.finish fail:

[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/flink-github-ci/flink-contrib/flink-streaming-contrib/target/flink-streaming-contrib_2.10-1.2-SNAPSHOT-javadoc.jar to org.apache.flink/flink-streaming-contrib_2.10/1.2-SNAPSHOT/flink-streaming-contrib_2.10-1.2-SNAPSHOT-javadoc.jar
channel stopped
Putting comment on the pull request
Finished: UNSTABLE

Copy link
Contributor

@StephanEwen StephanEwen left a comment

Choose a reason for hiding this comment

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

I think this looks very good. The last question I have before merging is about the dependencies. The flink-storm/pom.xml originally excluded all the Clojure Web Frameworks. That is no longer the case. Did storm 1.0 completely drop these Frameworks (migrate to Java)?

@liuyuzhong
Copy link
Author

@StephanEwen Actually, I'm not very clear about Clojure Web FrameWork, but it's web framework are using "ring.ring-core".
import: https://github.com/apache/storm/blob/1.x-branch/pom.xml#L703
code detail: https://github.com/apache/storm/blob/1.x-branch/storm-core/src/clj/org/apache/storm/ui/helpers.clj#L36

@StephanEwen
Copy link
Contributor

Okay, then we should re-add all the dependency exclusions that were removed from the flink-storm/pom.xml.

@StephanEwen
Copy link
Contributor

I would like to merge this. But I think we need to change back the dependency exclusions in flink-storm/pom.xml to as they are in the current master.

Can you do that? Then I will merge this...

@liuyuzhong
Copy link
Author

Sorry.forget to check it. I will do it tomorrow.

@liuyuzhong
Copy link
Author

liuyuzhong commented Oct 28, 2016

@StephanEwen Added dependency exclusions in flink-storm/pom.xml. Help me to review it please.

@StephanEwen
Copy link
Contributor

I think this is getting into good shape, we can almost merge it.
Two things remaining:

  1. Can you rebase/squash all commits into one commit?
  2. There is a complilation error, probably a missing dependency, see below.

Hope that when those are addresses, we can merge this.

[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /home/travis/build/apache/flink/flink-contrib/flink-storm/src/main/java/org/apache/flink/storm/api/FlinkSubmitter.java:[30,23] package org.json.simple does not exist
[INFO] 1 error
[INFO] -------------------------------------------------------------

@liuyuzhong
Copy link
Author

Fixed complie error.

@liuyuzhong
Copy link
Author

Complie error is fixed. But how to squash all commits into one commit?

@StephanEwen
Copy link
Contributor

Thanks!

You can squash commits by rebasing. Have a look here: http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html

zentol and others added 20 commits December 5, 2016 21:04
…nly fails terminally if no more restarts are possible

If in state RESTARTING a failure occurs, then a new restart attempt is started. Only if the
restart strategy no longer allows further restarts or if the thrown exception is of type
SuppressRestartsException a job can go from RESTARTING into FAILED.

This closes apache#2710
After emitting a record via the RecordWriter, we eagerly requested
a new buffer for the next emit on that channel (although it's not clear
that we will immediately need it). With this change, we request that
buffer lazily when an emit call requires it.

This closes apache#2716.
…scheduleOrUpdateConsumers call

Instead of failing the complete ExecutionGraph, a failing scheduleOrUpdateConsumers call
will be reported back to the caller. The caller can then decide what to do. Per default,
it will fail the calling task.

This closes apache#2700
@liuyuzhong
Copy link
Author

Sorry, something was wrong when I am rebase.

@liuyuzhong
Copy link
Author

@StephanEwen Sorry, I can't continue with this pull request. I will use another githup account to pull.

@liuyuzhong
Copy link
Author

@StephanEwen I use new githup account make a new pull request #3037, help me to review it please.Thanks.

@greghogan
Copy link
Contributor

Please close this since there is a newer PR.

@liuyuzhong
Copy link
Author

#3037 has merged. done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.