Skip to content

Conversation

@roshannaik
Copy link
Contributor

Implemented the ACK-ing logic in Java.
See Jira for profiler info on performance boost.

@Parth-Brahmbhatt
Copy link
Contributor

+1.

@HeartSaVioR
Copy link
Contributor

Nice, +1
According to https://gist.github.com/tolitius/1721519, maybe we can try recursive approach to improve performance if we want to stick in clojure. But now we don't need to, and this PR is clearer.

@roshannaik
Copy link
Contributor Author

I was able to run the unit tests successfully for this.. but wanted to ensure a committer is able to review the Java ACKing logic implementation and confirm it looks good.

@ptgoetz
Copy link
Member

ptgoetz commented Feb 12, 2016

+1

I haven't looked at the conflicts yet, But we may need separate pull requests for the mast and 1.x branches. But that should be trivial.

Can you add this to STORM-1491?

@satishd
Copy link
Member

satishd commented Feb 12, 2016

+1

@roshannaik
Copy link
Contributor Author

Added it to STORM-1491.
The PR for 1.x branch is #1105

@roshannaik
Copy link
Contributor Author

will update PR by removing unused clojure function and squashing commits.

@revans2
Copy link
Contributor

revans2 commented Feb 12, 2016

+1 please have a version for 1.x, and please upmerge for 2.x.

@roshannaik
Copy link
Contributor Author

@revans2 as mentioned, the PR for 1.x version is #1105

@roshannaik
Copy link
Contributor Author

@revans2 .. PR updated after rebase to master branch

@HeartSaVioR
Copy link
Contributor

+1 again

@revans2
Copy link
Contributor

revans2 commented Feb 13, 2016

+1

@asfgit asfgit merged commit d110b1f into apache:master Feb 17, 2016
revans2 pushed a commit to revans2/incubator-storm that referenced this pull request May 7, 2018
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