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

Upgrade multi-lang implementation which may have broken in Storm 0.9.3 #82

Closed
amontalenti opened this issue Dec 2, 2014 · 7 comments · Fixed by #87
Closed

Upgrade multi-lang implementation which may have broken in Storm 0.9.3 #82

amontalenti opened this issue Dec 2, 2014 · 7 comments · Fixed by #87
Assignees
Milestone

Comments

@amontalenti
Copy link
Contributor

Storm 0.9.3 release notes indicate that "the multi-lang API has changed in a non-backward-compatible way". This has something to do with the way that ShellBolts implement heartbeat'ing / sync'ing. Should test, investigate, and upgrade as necessary the ipc module.

@dan-blanchard
Copy link
Member

And yet they still haven't updated the documentation for it. I find that the most frustrating part of working with Storm: the official documentation is very poorly formatted, and is frequently out of date.

@amontalenti
Copy link
Contributor Author

@dan-blanchard blame me, I recently voluntwered to update the Storm docs. Wanna help? :)

@dan-blanchard
Copy link
Member

I believe this diff shows the clearest indication of what actually changed.

The only changes to the protocol I see are:

  • New metrics command. It looks like you can use it to report custom performance metrics in multilang bolts. The JIRA issue discussing it in more detail is here, and I didn't have a chance to read through all of it yet.
  • Heartbeats (as mentioned in Add support for multilang heartbeats #70). To check if a received message is a heartbeat, we need to check that the task is -1 and the stream is __heartbeat. If we receive a heartbeat on the Python side, we need to send a sync back. Tuple heartbeats are not supposed to be processed (or acked) otherwise.

It also seems like it is recommended now that we use the error command instead of log to report exceptions, but we already did that in #75.

@dan-blanchard
Copy link
Member

Consider listing the actual changes to the API above my contribution to that effort. ;)

Oh, and I should mentioned in my previous comment that heartbeats appear to only be sent to multilang bolts and not spouts.

@HeartSaVioR
Copy link

I've requested for pull to update documentation.
https://github.com/HeartSaVioR/storm/blob/STORM-544/docs/documentation/Multilang-protocol.md
But @dan-blanchard commented it already. ;)

@amontalenti
Copy link
Contributor Author

patch from pyleus recently contributed:

https://github.com/Yelp/pyleus/pull/76/files

@dan-blanchard
Copy link
Member

I should note that #87 made it so we're compatible with 0.9.3, but we're still not taking advantage of the ability to have custom metrics reported to Nimbus like was added in apache/storm#38. That should probably be a separate issue though.

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

Successfully merging a pull request may close this issue.

4 participants