Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/568#issuecomment-108339300
And like Bobby already said: many thanks for your continued work and
improvements, @eshioji!
---
If your project is set up for it, you can reply to this email and have
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/568#issuecomment-108338846
> @eshioji wrote:
> Also I have a question, maybe @miguno could help; I've removed the
graceful shutdown which tries to flush all pending message before th
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/566#issuecomment-106795826
> My reasoning is as follows:
>
> * One has three option to deal with Netty's buffer filling up:
> 1. Discard incoming new messages
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/522#issuecomment-94378665
We already [received a
+1](https://github.com/apache/storm/pull/275#issuecomment-94068063) from
@nathanmarz.
---
If your project is set up for it, you can reply to this
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/405#issuecomment-94007128
@vesense As [I said in the JIRA
ticket](https://issues.apache.org/jira/browse/STORM-643?focusedCommentId=14494138&
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/275#issuecomment-93699373
My understanding is yes, you do need to ack tick tuples. See @nathanmarz
[comment](https://groups.google.com/forum/#!topic/storm-user/ZEJabXT5nQA) from
some time back
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/275#issuecomment-93694857
I created [STORM-786](https://issues.apache.org/jira/browse/STORM-786) to
track the tick tuple acking. Pull request is already sent.
---
If your project is set up for it
GitHub user miguno opened a pull request:
https://github.com/apache/storm/pull/522
STORM-786: KafkaBolt should ack tick tuples
[STORM-512](https://issues.apache.org/jira/browse/STORM-512) (KafkaBolt
doesn't handle ticks properly) adds special-casing of tick tuples. What is
mi
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/275#issuecomment-93690832
Since KafkaBolt is extending BaseRichBolt, I think we should perform a
`collector.ack(input)` before `return`. Tick tuples must be acked like
"normal&qu
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/268#issuecomment-82217610
@clockfly We may also want to check why we don't have the permissions to
close this PR ourselves.
---
If your project is set up for it, you can reply to this emai
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/463#issuecomment-81739459
I created [STORM-707](https://issues.apache.org/jira/browse/STORM-707) to
track this.
---
If your project is set up for it, you can reply to this email and have your
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/268#issuecomment-81735980
Yes, this pull request can be closed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/268#issuecomment-81736347
@d2r And btw I don't have permissions on GitHub to close this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitH
GitHub user miguno opened a pull request:
https://github.com/apache/storm/pull/463
Client (Netty): improving logging to help troubleshooting connection woes
These logging statements are not on a hot path, and `INFO` is the default
log level of Storm. These logging are filling a
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/456#issuecomment-78243873
Sorry that I haven't had the time yet to follow-up. Please stay tuned!
---
If your project is set up for it, you can reply to this email and have your
reply appe
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/456#issuecomment-77159580
Many thanks for the PR, Kai!
> However if the topology is submitted with `storm shell`, this cannot
track exception because spawnvp seems to return no stdout
Github user miguno commented on a diff in the pull request:
https://github.com/apache/storm/pull/456#discussion_r25774104
--- Diff: bin/storm ---
@@ -183,7 +209,16 @@ def exec_storm_class(klass, jvmtype="-server",
jvmopts=[], extrajars=[], args=[]
os.spawnvp
Github user miguno commented on a diff in the pull request:
https://github.com/apache/storm/pull/456#discussion_r25774112
--- Diff: bin/storm ---
@@ -183,7 +209,16 @@ def exec_storm_class(klass, jvmtype="-server",
jvmopts=[], extrajars=[], args=[]
os.spawnvp
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/429#issuecomment-75713928
Oh, Taylor. Could you also update STORM-404 and STORM-510 as appropriate?
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/429#issuecomment-75634123
Phew. :-)
Thanks for merging, Taylor!
> On 23.02.2015, at 22:06, P. Taylor Goetz wrote:
>
> Disregard last message. It wa
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/429#issuecomment-75022795
FYI: I took the opportunity to update
[DEVELOPER.md](https://github.com/apache/storm/blob/master/DEVELOPER.md#building)
with documentation how to run tests selectively in
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/429#issuecomment-74947350
What were the exact REPL and mvn commands?
> On 18.02.2015, at 21:48, P. Taylor Goetz wrote:
>
> @miguno Regarding the merge to 0.9.x, I di
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/429#issuecomment-74927415
@nathanmarz Thanks for the detailed feedback on the max-retries issue. As
Bobby suggested, would you mind if we decouple the work on max-retries (tracked
at STORM-677
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/429#issuecomment-74898690
Thanks, Taylor! Let me know if I can help with sorting out the test
failures.
Also regarding JIRA: I forgot to mention that it looks like we need to
update STORM
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/429#issuecomment-74895041
PS: We may also want to update the original
[STORM-329](https://issues.apache.org/jira/browse/STORM-329) ticket description
to reflect the changes in this PR.
---
If
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/429#issuecomment-74894542
FYI: I created [STORM-677: Maximum retries strategy may cause data
loss](https://issues.apache.org/jira/browse/STORM-677) to address the issue
that Bobby brought up in
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/429#issuecomment-74894825
> I'd also like to apply it to the 0.9.x branch as I feel it's an important
fix.
That's a good idea, Taylor. Would you also like volunteer to
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/429#issuecomment-74711484
Glad to hear it was only a temporary issue. :-)
To summarize the current status of this pull request:
- We have two +1 already.
- The max-reconnection
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/429#issuecomment-74702215
And FWIW, with the code in this PR the total test suite takes about 5mins
to complete.
```
$ mvn clean install
...
[INFO
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/429#issuecomment-74701138
> I am seeing a lot of tests timing out with this change. Has anyone else
seen this?
Hmm. All the tests are passing for me (and they have been since a wh
Github user miguno commented on a diff in the pull request:
https://github.com/apache/storm/pull/429#discussion_r24742354
--- Diff: storm-core/src/jvm/backtype/storm/messaging/netty/Client.java ---
@@ -42,344 +42,577 @@
import org.slf4j.LoggerFactory;
import
Github user miguno commented on a diff in the pull request:
https://github.com/apache/storm/pull/429#discussion_r24738502
--- Diff: storm-core/src/jvm/backtype/storm/messaging/netty/Client.java ---
@@ -42,344 +42,577 @@
import org.slf4j.LoggerFactory;
import
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/429#issuecomment-74147926
If you need at-least-once processing you must use an acking topology, which
will allow Storm to replay lost messages. If instead you go with an unacking
topology (= no
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/429#issuecomment-74050954
Thanks for your feedback, Nathan.
As far as I understand this patch does not enable backpressure. But:
because there is no backpressure (yet) that we can rely on
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/429#issuecomment-74032874
This patch allows a worker to properly detect that the connection to a peer
becomes unavailable -- for whatever reason (the remote worker is dead or
restarting, there was
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/428#issuecomment-73946399
I did exactly this in #429.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user miguno closed the pull request at:
https://github.com/apache/storm/pull/428
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/428#issuecomment-73941559
Closed in favor of updated PR at https://github.com/apache/storm/pull/429.
The updated PR uses a single commit to ensure we have a cleaner commit
history in our
GitHub user miguno opened a pull request:
https://github.com/apache/storm/pull/429
STORM-329: fix cascading Storm failure by improving reconnection strategy
and buffering messages
**This PR contains the same code as
https://github.com/apache/storm/pull/428 but as a single commit
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/428#issuecomment-73928303
I tried rebasing (also to fix the incorrect commit message that starts with
STORM-32*7*) but gave up after several failed attempts. Feel free to give it a
try though
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/268#issuecomment-73924933
The new pull request is available at
https://github.com/apache/storm/pull/428.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/428#issuecomment-73924851
Note: All of the code review comments by of @clockfly above that were
added before this pull was created have already been addressed.
---
If your project is set up for
GitHub user miguno opened a pull request:
https://github.com/apache/storm/pull/428
STORM-329: fix cascading Storm failure by improving reconnection strategy
and buffering messages
This is an improved version of the original pull request discussed at
https://github.com/apache/storm
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/268#issuecomment-73920741
> So, I don't think your TODO comment is a issue, it is actually designed
like this, how do you think?
That makes sense to me. I'll remove the TODO.
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/268#issuecomment-73905601
Regarding the TODO (how to handle messages if the channel is not writable):
I'd prefer to keep the current code as is in this regard, i.e. keep the
TODO in
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/268#issuecomment-73889052
The bolt-of-death topology is now available:
https://github.com/verisign/storm-bolt-of-death
---
If your project is set up for it, you can reply to this email and have
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/268#issuecomment-73847544
I think if the channel is not `WRITABLE` when we are trying to flush a full
message batch (or more than one, given that it's part of the `while` loop),
then we&#x
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/268#issuecomment-73671537
PS: I just noticed that I put the wrong Storm ticket into the commit
https://github.com/miguno/storm/commit/8ebaaf8dbc63df3c2691e0cc3ac5102af7721ec3.
The `STORM-327
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/268#issuecomment-73667341
Many thanks for your review, Sean. I addressed your comments, see the new
commits in
https://github.com/miguno/storm/commits/0.10.0-SNAPSHOT-STORM-392-miguno-merge
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/268#issuecomment-73527901
I have improved the patch in this pull request, which particularly meant
modifications to
The updated patch is availabe at:
*
https://github.com/miguno/storm
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/268#issuecomment-73276378
Thanks for the follow-up, @tedxia.
It looks as if I was able to report progress. My conversation with
@clockfly earlier today was very helpful in this regard
Github user miguno commented on a diff in the pull request:
https://github.com/apache/storm/pull/268#discussion_r24039811
--- Diff: storm-core/src/jvm/backtype/storm/messaging/netty/Client.java ---
@@ -142,6 +147,15 @@ public void run
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/268#issuecomment-72653588
@tedxia (or @clockfly): Have you experienced similar Storm behavior as [I
described
above](https://github.com/apache/storm/pull/268#issuecomment-72652704) in your
patched
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/268#issuecomment-72652704
Some additional feedback regarding Storm's behavior in the face of
failures, with and without this patch. This summary is slightly simplified to
make it a shorter
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/278#issuecomment-58267519
Thanks for merging, @ptgoetz !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not
Github user miguno commented on the pull request:
https://github.com/apache/storm/pull/278#issuecomment-58000787
Thanks, @harshach. I'll wait one or two days more so that other people can
voice their opinion if needed.
---
If your project is set up for it, you can reply to
GitHub user miguno opened a pull request:
https://github.com/apache/storm/pull/278
STORM-488: Exit with 254 error code if storm CLI is run with unknown command
This change follows the existing semantics when the `storm` CLI tool is
invoked without _any_ parameters.
With
GitHub user miguno opened a pull request:
https://github.com/apache/storm/pull/277
STORM-514: Refer to post-graduation Storm website and git repos
Also, we highlight even more that users must `cd` into the storm-starter
sub-directory.
This pull request only touches the
58 matches
Mail list logo
