Github user knusbaum commented on a diff in the pull request:

    https://github.com/apache/storm/pull/1710#discussion_r80798282
  
    --- Diff: 
storm-core/src/jvm/org/apache/storm/cluster/PaceMakerStateStorage.java ---
    @@ -159,10 +162,6 @@ public void set_worker_hb(String path, byte[] data, 
List<ACL> acls) {
                             ret = details;
                         }
                     }
    -                if(ret == null) {
    -                    throw new HBExecutionException("Failed to get a 
response.");
    -                }
    -                LOG.debug("Successful get_worker_hb");
                     return ret;
    --- End diff --
    
    Yes, I wasn't quite sure how we wanted to handle this, NULL responses are 
valid, and right now the code treats mismatched responses as if they were NULL. 
We can distinguish between them if we want the behavior to differ whether the 
pacemakers returned NULL details or returned an invalid message. Throwing on 
null is not what we wanted, though, since it will cause Nimbus to crash when 
nimbus tries to read a topology's heartbeats before it has started sending them.
    
    Maybe we want to add some logic so that if ALL pacemakers return an invalid 
response, then we throw?


---
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 [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to