You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by jangie <gi...@git.apache.org> on 2015/03/24 03:41:18 UTC

[GitHub] storm pull request: [STORM-468] First attempt

GitHub user jangie opened a pull request:

    https://github.com/apache/storm/pull/477

    [STORM-468] First attempt

    Am pretty sure this is not the best way. Am open to suggestions.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jangie/storm STORM-468

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/storm/pull/477.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #477
    
----
commit f910e0db9447ca0bdc1a382c09ee1a242f4fc166
Author: Jang-Soo Lee <ja...@gmail.com>
Date:   2015-03-24T02:34:59Z

    Attempt at STORM-468
    
    Am pretty sure this is not the best way. Am open to suggestions.

----


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-468] java.io.NotSerializableException s...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/storm/pull/477


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-468] java.io.NotSerializableException s...

Posted by jangie <gi...@git.apache.org>.
Github user jangie commented on the pull request:

    https://github.com/apache/storm/pull/477#issuecomment-169017636
  
    @revans2 Sure, I'll try and take a look later tonight for a fixup for the conflicts and spacing preference, thanks!


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-468] java.io.NotSerializableException s...

Posted by dossett <gi...@git.apache.org>.
Github user dossett commented on the pull request:

    https://github.com/apache/storm/pull/477#issuecomment-169778180
  
    @jangie Yes, my comment was based a sloppy reading of your PR.  Nice work!


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-468] java.io.NotSerializableException s...

Posted by jangie <gi...@git.apache.org>.
Github user jangie commented on the pull request:

    https://github.com/apache/storm/pull/477#issuecomment-169525901
  
    @dossett At initial time of the PR, I believe that the NotSerializableException was wrapped inside of a RuntimeException (and may very well still be).
    
    For what it's worth, I believe I've addressed the other comments, I guess we'll see what Travis says.


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-468] java.io.NotSerializableException s...

Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:

    https://github.com/apache/storm/pull/477#issuecomment-168881795
  
    Ping (mostly to other committers to solicit review/discussion).
    
    This is not a bad idea.


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-468] java.io.NotSerializableException s...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/storm/pull/477#discussion_r26999928
  
    --- Diff: docs/documentation/FAQ.md ---
    @@ -62,6 +62,10 @@ You can join streams with join, merge or multiReduce.
     
     At time of writing, you can't emit to multiple output streams from Trident -- see [STORM-68](https://issues.apache.org/jira/browse/STORM-68)
     
    +### Why am I getting a NotSerializableException/IllegalStateException when my topology is being started up?
    +
    +Within the Storm lifecycle, the topology is instantiated and then serialized to byte format to be stored in ZooKeeper, prior to the topology being executed. Within this step, if a spout or bolt within the topology has an initialized unserializable property, serialization will fail. If there is a need for a field that is unserializable, initialize it within the bolt or spout's prepare method, which is run after the topology is delivered to the worker.
    --- End diff --
    
    I'm not sure but it may be better to document "serialized fields' values are reused when workers are relaunched whether live worker modifies it or not.". 
    I had this issue earlier, and sure it was by mistake, but it would be hard to find bug.
    
    Wish to hear everyone's thought.


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-468] java.io.NotSerializableException s...

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:

    https://github.com/apache/storm/pull/477#issuecomment-169018717
  
    @jangie great. I want to apologize too 10 months is way too long for a pull request to linger.  We really need to do better with some of these.


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-468] java.io.NotSerializableException s...

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:

    https://github.com/apache/storm/pull/477#issuecomment-169014532
  
    +1 for the concept.  The formatting of all of the code is off.  Looks like they are using tabs, not spaces.  I would also like to see @HeartSaVioR's comment addressed.
    
    There also appear to be merge conflicts now.  @jangie sorry this has lingered so very long.  This is a great concept.  Is there any hope that you can address these comments?


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-468] java.io.NotSerializableException s...

Posted by dossett <gi...@git.apache.org>.
Github user dossett commented on the pull request:

    https://github.com/apache/storm/pull/477#issuecomment-169046382
  
    Would it be cleaner to just catch NotSerializableException directly and rethrow an IllegalStateException?


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---