[
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)