Remove ping pong from reply stage #1745
Conversation
| // we didn't have the requested transaction (random or explicit) from the neighbor but we will immediately reply | ||
| // with the latest known milestone and a needed transaction hash, to keep up the ping-pong | ||
| try { | ||
| final TransactionViewModel msTVM = TransactionViewModel.fromHash(tangle, | ||
| latestMilestoneTracker.getLatestMilestoneHash()); | ||
| neighborRouter.gossipTransactionTo(neighbor, msTVM, false); | ||
| } catch (Exception e) { | ||
| e.printStackTrace(); | ||
| } | ||
|
|
There was a problem hiding this comment.
I am trying to think...
Intuitively I would think that there is an else is missing above the try...
This is because this is what the comment says. But let's think critically about this.
If we delete this code it means that if we request a non-existing tx the ping-pong would stop.
If I am a non-synced node and I am getting requests for transactions that I don't have then I won't request transactions that are missing from those nodes, and the ping pong would stop.
However if a neighbor is requesting from me transactions that I don't have then the neighbor is:
- Also out of sync
- Malicious
In case of (1) it means that I am sending the same transactions over and over again, thus slowing down an not helping with the sync. This will stop the ping pong between them but newly broadcasted txs should liven it up
In case of (2) we obviously shouldn't send a message
As long as there is 1 synced node in the network it should be able to sync the rest of the nodes eventually
So I support deleting and not adding else
This reverts commit eb4e1a4.
This reverts commit eb4e1a4
This reverts commit a735448
This reverts commit a735448
Description
There has been a syncing issue that arose from the internal tests that resulted in a flood of back and forth gossiping of the latest milestone. It fills the queues and stops the node from processing relevant transactions in a timely manner. This issue self corrects at times, but in other cases it stalls out for hours. It is possible that this is caused by the continued pushing of these transactions to the neighbors through the reply stage.
Type of change
How Has This Been Tested?
Checklist: