scalecube / scalecube-cluster

ScaleCube Cluster is a lightweight Java VM implementation of SWIM: Scalable Weakly-consistent Infection-style Process Group Membership Protocol. features cluster membership, failure detection, and gossip protocol library.
http://scalecube.github.io/
Apache License 2.0
263 stars 88 forks source link

Adds to infected when gossip received from 2nd and following senders #379

Closed b9r5 closed 1 year ago

b9r5 commented 1 year ago

What is happening currently in GossipProtocolImpl.onGossipRequest(Message) is that if the (gossiper, sequence ID) is newly seen, then the sender of the gossip request is added to the gossip state's infected collection.

In a situation where member A originates some gossip, then sends it to members B and C, then B sends the gossip to C, C will not add B to the gossip state's infected collection. As least, this is how it appears to me.

This change adds the sender of the gossip request to the gossip state's infected collection even when the (gossiper, sequence ID) is not newly seen.

b9r5 commented 1 year ago

I thought I should leave some explanation of what I was thinking.

GossipProtocolImpl.selectGossipsToSend(period, member) is filtering out gossips that the member is already infected with, because there is no benefit in sending those gossips to the member.

So in the above example, where C has received a gossip from B, C knows that B is infected. It would be good for C to add B to the infected collection for the gossip so that the optimization in selectGossipsToSend(period, member) is effective.

It looks like what is currently happening is that C would not add B if it had previously received the gossip from A.

artem-v commented 1 year ago

I thought I should leave some explanation of what I was thinking.

GossipProtocolImpl.selectGossipsToSend(period, member) is filtering out gossips that the member is already infected with, because there is no benefit in sending those gossips to the member.

So in the above example, where C has received a gossip from B, C knows that B is infected. It would be good for C to add B to the infected collection for the gossip so that the optimization in selectGossipsToSend(period, member) is effective.

It looks like what is currently happening is that C would not add B if it had previously received the gossip from A.

Hi Thanks for PR. We're processing it.

artem-v commented 1 year ago

@b9r5

Looks like it's happening like that:

    // On C:
    // Comes from A:
    ensureSequence("A" /*gossiperId*/).add(1 /*sequenceId*/) => true
    // Comes from B:
    ensureSequence("A" /*gossiperId*/).add(1 /*sequenceId*/) => false

GossipProtocolImpl.sequenceIdCollectors map mapped by key Gossip.gossiperId and ignores GossipRequest.from => we're not catching infected members.

Good catch. Thanks for PR.

b9r5 commented 1 year ago

@artem-v

Anytime, glad I could help.