You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by hmcl <gi...@git.apache.org> on 2016/12/28 16:01:57 UTC

[GitHub] storm pull request #1844: STORM-2265: Incorrectly Serialized JSON in Transac...

GitHub user hmcl opened a pull request:

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

    STORM-2265: Incorrectly Serialized JSON in TransactionalState causes Worker to Die

    

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

    $ git pull https://github.com/hmcl/storm-apache Apache_master_Apache_STORM-2265_IncorrectlySerialJSON

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

    https://github.com/apache/storm/pull/1844.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 #1844
    
----
commit 69ff19cc53434dbebfc8313ab4dc177da9b3194d
Author: Hugo Louro <hm...@gmail.com>
Date:   2016-12-28T15:57:47Z

    STORM-2265: Incorrectly Serialized JSON in TransactionalState causes Worker to Die

----


---
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 issue #1844: STORM-2265: Incorrectly Serialized JSON in TransactionalS...

Posted by hmcl <gi...@git.apache.org>.
Github user hmcl commented on the issue:

    https://github.com/apache/storm/pull/1844
  
    @HeartSaVioR `JSONValue.parse` will simply return null if there is a serialization exception. How can that be better than this approach? Isn't anything other than null better than null?


---
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 issue #1844: STORM-2265: Incorrectly Serialized JSON in TransactionalS...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:

    https://github.com/apache/storm/pull/1844
  
    @hmcl 
    I filed STORM-2264, and thought about this approach before.
    
    This approach makes GlobalPartitionInformation serialized well, but the type is lost when deserializing. Deserialized object is not GlobalPartitionInformation. 
    I haven't deeply look into storm-kafka Trident implementation so hard to guess how this affects.
    
    If we're not sure, just replacing JSONValue.parseWithException to JSONValue.parse only from TransactionalState rolls back the problem. It's just hiding an long ago issue again so need to find out clearer way but given that it is critical, we may need quick fix first.


---
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 issue #1844: STORM-2265: Incorrectly Serialized JSON in TransactionalS...

Posted by hmcl <gi...@git.apache.org>.
Github user hmcl commented on the issue:

    https://github.com/apache/storm/pull/1844
  
    @HeartSaVioR I agree with you. Let's merge [STORM-2264](https://github.com/apache/storm/pull/1853) and keep this open to explore a more compelling fix. If we find out that the work around the Stream API will make this fix unnecessary, we can then close this PR and the associated JIRA.


---
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 issue #1844: STORM-2265: Incorrectly Serialized JSON in TransactionalS...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:

    https://github.com/apache/storm/pull/1844
  
    @hmcl 
    What I'm saying is, given that we've had no issue regarding this and we don't have clear and safe approach, it would be better to roll back to previous.
    
    Two things should be clear:
    1. What's the type of deserialized value of GlobalPartitionInformation? (Please note that JSONValue doesn't guarantee restoring origin type.)
    2. Is it guaranteed for all the paths to use the value from TransactionalState as that type?
    
    And there's compile failure on your change. Please check the Travis CI result.


---
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.
---