Skip to content

Conversation

bhupendrasingh
Copy link
Contributor

No description provided.

@jayunit100
Copy link
Contributor

This looks like its rolling along nicely.

Bhupendra can you paste the results of some manual tests if it's working?

* Fault tolerance and state management
* Native rolling and tumbling window support
* Hadoop-native YARN & HDFS implementation

Copy link
Member

Choose a reason for hiding this comment

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

Add ----- 'Complete Event Processing (CEP)' , Table API for SQL, Gelly - Graph Processing Framework --- to the list of key features

@bhupendrasingh
Copy link
Contributor Author

Updated the spec file for adding features and yes surely will look into 1.0 RC for merging.

@Harshita27
Copy link

Sure Jay, were working on Debian packaging, completed that. Will definitely try to deploy and run some manual tests.

@smarthi
Copy link
Member

smarthi commented Feb 29, 2016

This is great indeed. For the next pass (@jayunit100 ?), since Flink has streaming connectors with Kafka 0.8.x, 0.9x and Storm, it would be great to add end-2-end integration tests to validate the connectors.

@@ -0,0 +1,5 @@
flink (0.10.2-1) UNRELEASED; urgency=medium
Copy link
Member

Choose a reason for hiding this comment

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

Change that to 1.0.0, since Flink 1.0.0 is now official

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure.

@bhupendrasingh
Copy link
Contributor Author

@C0S updated the bigtop repo url for openstack provider..

@ghost
Copy link

ghost commented Mar 29, 2016

There's four different commits in here. I will just pick one for the repo url and generate the patch from it.

@smarthi
Copy link
Member

smarthi commented Mar 29, 2016

Bhupendra, do u have any manual test results yet that you would like to share about Flink-BigTop integration ?

@jayunit100
Copy link
Contributor

@smarthi We went over the work so far saturday. As of now the RPMs are able to launch flink masters and slaves. Now its just a matter of tooling the puppet stuff to launch similar to how spark does, rather than using a node-aware bash wrapper.

@smarthi
Copy link
Member

smarthi commented Mar 29, 2016

Thanks @jayunit100, looking forward to it

bigtop {
/** Base Configuration of the mirror and archives */
version = "1.2.0-SNAPSHOT"
version = "1.1.0-SNAPSHOT"
Copy link

Choose a reason for hiding this comment

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

Is this change made intentionally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure how that got changed, must have been by mistake as I don't see any reason to change it to 1.1.0, will fix that. thanks for pointing out.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this still needs to be addressed

@mxm
Copy link

mxm commented Apr 1, 2016

I've built the pull request to see if RPM package generation works. The binary package contents look fine. The Debian package is still missing. Are you working on it or do you want to do a follow-up pull request? I haven't played around with Bigtop much but it looks like it wouldn't be too hard to build a Debian package, now that we have the RPM.

@bhupendrasingh
Copy link
Contributor Author

The scripts for building the Debian package are also part of the prior commits. You can check those scripts in /bigtop-packages/src/deb/Flink.

@mxm
Copy link

mxm commented Apr 4, 2016

Ah thanks. I've also checked out the Debian package contents. Look good as well. I went ahead and installed it on my Ubuntu machine.

I was wondering, why does the package depend on hadoop-client? Flink packages Hadoop and doesn't require an existing installation of Hadoop. Further, it depends on bigtop-utils which is IMHO only necessary because of a java detection check. The latter is probably ok because most people have that package installed anyways if they work with Bigtop. However, I think we should consider removing the Hadoop dependency.


Package: flink
Architecture: all
Depends: hadoop-client, bigtop-utils (>= 0.7)
Copy link

Choose a reason for hiding this comment

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

Flink ships the Hadoop client, IMHO no need to depend on it here.

@mxm
Copy link

mxm commented Apr 4, 2016

I've gone ahead and installed the Debian package and commented on some issues. The log4j*.properties files are not included in the Debian package, probably because the /log directory is missing from flink.dirs.

/usr/lib/flink
/usr/lib/flink/lib
/usr/lib/flink/bin
/usr/bin
Copy link

Choose a reason for hiding this comment

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

Might need to add the log directory here to include the log files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for feedback, will soon clean the scripts and add the missing parameters. Probably by the coming week will update the things.

@jayunit100
Copy link
Contributor

Overall this is coming together nicely. I think given the community demand if the rpm builds I'm on the side of merging after I test this , with smoke tests to come in a follow up patch, so that the community can start banging it around.

We should remove the vagrantfile changes but that is mostly just a nit.

@jayunit100
Copy link
Contributor

Obviously i will work with Bhupendra and the others to rebase the commits and so on so that attribution is done properly
And so it's a clean patch. But overall seems like it should work now.zzz

@rmetzger
Copy link
Contributor

What is the status on this pull request?
We have a flink user who's using Bigtop and who would like to have a Flink integration from bigtop, in particular the packaging.
I'm pretty busy right now but maybe tomorrow I have some time to address the open comments and open another pull request based on this one.

@jayunit100
Copy link
Contributor

Looks like the tests are still failing as is but I think it is in a state where it might be useable.
Maybe we should commit it to a WIP branch?

@rmetzger
Copy link
Contributor

If you agree I can address @mxm comments regarding the rpm packaging. Once these comments are addressed I assume the code is good to be merged to master.

@bhupendrasingh
Copy link
Contributor Author

The issues pointed out by @mxm have been addressed and we have also tested the new rpm package on local environment as standalone package. We have also incorporated the master worker configuration and currently facing some issues during puppet deployment. We are still resolving the issue. Should I push the latest code so that others can also have a look.?

@bhupendrasingh
Copy link
Contributor Author

Ignore the changes in the vagrantfile and vagrantconfig.yaml, they were only for testing purpose.

class flink {

class deploy ($roles) {
<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks a lot like unresolved merge conflicts.

@jayunit100
Copy link
Contributor

Also rebase into 2 or 3 commits attributed to your team after you fix the merge.

@bhupendrasingh
Copy link
Contributor Author

Need a couple of days, have got submissions and finals this week. So everyone on the team is busy. We will take this up on Friday and hope to fix everything over the weekend. :)

fi

if [ "$FLINK_MASTER_IP" = "" ]; then
FLINK_MASTER_IP=`hostname`
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the purpose of this environment variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this can be removed safely. This was set for testing purpose, to provide the workers with the master-node name. I don't think we need it now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, I've removed it

@rmetzger
Copy link
Contributor

Just FYI: I opened a pull request just for the packaging, based on this one: #101

@rmetzger
Copy link
Contributor

Since #101 has been merged, this pull request is obsolete (I have to admit that I forgot to properly attribute the work from #101. This was a mistake while rebasing on my end when I was addressing some of the PR feedback)

rvs pushed a commit to rvs/bigtop that referenced this pull request Nov 1, 2016
ODPI-182. Adding Hive CLI tests.
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.

7 participants