Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Dec 1, 2022

Conversation

b9r5
Copy link
Contributor

@b9r5 b9r5 commented Nov 25, 2022

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.

@artem-v artem-v self-requested a review November 26, 2022 11:10
@b9r5
Copy link
Contributor Author

b9r5 commented Nov 27, 2022

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
Copy link
Contributor

artem-v commented Nov 30, 2022

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
Copy link
Contributor

artem-v commented Dec 1, 2022

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

@artem-v artem-v merged commit b7d8714 into scalecube:master Dec 1, 2022
@b9r5
Copy link
Contributor Author

b9r5 commented Dec 1, 2022

@artem-v

Anytime, glad I could help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants