[jira] [Commented] (GOSSIP-39) User controlled Gossip

["Dorian Ellerbe (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/GOSSIP-39?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15838072#comment-15838072 ] 

Dorian Ellerbe commented on GOSSIP-39:
--------------------------------------

https://github.com/apache/incubator-gossip/compare/master...edwardcapriolo:GOSSIP-39?expand=1

Pull request feedback:

Space between Parameterized types
https://github.com/apache/incubator-gossip/compare/master...edwardcapriolo:GOSSIP-39?expand=1#diff-35e57ba52a1021a9998b91e63d9ce02eR59

new HashMap<String, String>(), this is in a lot of places. Probably good to do a clean-up when possible.
https://github.com/apache/incubator-gossip/compare/master...edwardcapriolo:GOSSIP-39?expand=1#diff-3fdab6dbe84793d47aa0da8f75794c8cR55

Map<String, String> properties
https://github.com/apache/incubator-gossip/compare/master...edwardcapriolo:GOSSIP-39?expand=1#diff-c03a3ff6884dfea893f8ccc4cab2d1b0R44

new HashMap<String, String>()
https://github.com/apache/incubator-gossip/compare/master...edwardcapriolo:GOSSIP-39?expand=1#diff-a882881b342f8208577c330128eb16fdR39

Time to address this now?
https://github.com/apache/incubator-gossip/compare/master...edwardcapriolo:GOSSIP-39?expand=1#diff-d235710e38d43916e0915eb6c838ebe0R177

Gossipper -> Gossiper. I recommend doing a global search/replace for this one to pick up other classes and fields, too.
https://github.com/apache/incubator-gossip/compare/master...edwardcapriolo:GOSSIP-39?expand=1#diff-74123e0e74657b59d77ec906fcc0468fR51

You can do some method extract in this class. For instance, lines 73-81 can be extracted to a new method that returns a GossipCore. Or further down one that returns a UdpActiveGossipMessage.
https://github.com/apache/incubator-gossip/compare/master...edwardcapriolo:GOSSIP-39?expand=1#diff-74123e0e74657b59d77ec906fcc0468fR73

Maybe rename these args to "self" and "otherMember"?
https://github.com/apache/incubator-gossip/compare/master...edwardcapriolo:GOSSIP-39?expand=1#diff-74123e0e74657b59d77ec906fcc0468fR67

This should become a property.
https://github.com/apache/incubator-gossip/compare/master...edwardcapriolo:GOSSIP-39?expand=1#diff-9d369418fc6dc1e95f8b3f8a35d554a7R41

Lines 67-110 have some indentation inconsistencies.
https://github.com/apache/incubator-gossip/compare/master...edwardcapriolo:GOSSIP-39?expand=1#diff-9d369418fc6dc1e95f8b3f8a35d554a7R67

Shurdown -> Shutdown
https://github.com/apache/incubator-gossip/compare/master...edwardcapriolo:GOSSIP-39?expand=1#diff-b93576f92c0cc381f8b00acf684e5039R82

Maybe late to the party, but is this the way we are going to write tests? I'm a big fan of given-when-then so that the test cases are more readable, easier to modify and more succinct. Right now, I have to read the entire method to understand what is under test.
https://github.com/apache/incubator-gossip/compare/master...edwardcapriolo:GOSSIP-39?expand=1#diff-cf515c8f73af947a0a6d307043ff8dc6R25

> User controlled Gossip
> ----------------------
>
>                 Key: GOSSIP-39
>                 URL: https://issues.apache.org/jira/browse/GOSSIP-39
>             Project: Gossip
>          Issue Type: New Feature
>            Reporter: Edward Capriolo
>             Fix For: 0.1.2
>
>
> There are two interesting usecases:
> * Cassandra does this 'trick' where nodes gossip more often to seeds so that the cluster converges faster
> * It would be ideal if we could gossip more to some nodes than others. For example if we have a cluster in two data centers if they were more likely to gossip inside the wan
> The challenge here is flexibility vs complexity. Cassandra has a concept of Datacenter and Rack, do we make these 1st class parameters or do we let the logic consider ANY user data?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)