interledgerjs / ilp-connector

Reference implementation of an Interledger connector.
Other
136 stars 53 forks source link

Add code comment that explains epoch filtering #387

Open michielbdejong opened 7 years ago

michielbdejong commented 7 years ago

Routes expire due to https://github.com/interledgerjs/ilp-routing/blame/master/src/lib/routing-tables.js#L18 IIRC, the idea is that routes expire after 45 seconds, but are sent every 30 seconds. Just to be sure that this is not affecting our demo servers, I set noExpire true here on both red.ilpdemo.org and blue.ilpdemo.org This change will be overwritten when we next update them, so let's try to fix this issue permanently before that. ;)

Regardless of this, on red.ilpdemo.org and blue.ilpdemo.org I see these route broadcasts happening in each direction, every 30 seconds, so that part is working. However, there's this epoch filtering where some routes are not sent if they have already been sent in the same epoch. The assumption is, IIUC, the other peer already knows about that route, so no use repeating it over and over. sending the empty route broadcast is a 'heartbeat', telling the peer "all routes I sent you before are still valid, don't expire anything you currently have".

The problem seems to be though, if I restart red then how will it get blue's route, if blue doesn't know that red was restarted?

Also, how long is one epoch? And wouldn't it be easier to just send a whole table dump every 30 seconds? So far, I have not seen an ilp-kit with more than 20 or 30 destinations in its routing table, so I would say it would definitely be an acceptable simplification? cc @momerath42

For now, I disabled the [epoch filtering]() on both red and blue, so they will always send 1 route instead of 0 routes (even if the epoch is not finished yet).

momerath42 commented 7 years ago

epochs aren't defined by wall-time; a new epoch begins whenever genuinely new routes are added. We do seem to be lacking a robust mechanism for determining whether a peer has gone down and come back up before the next attempt to broadcast to them (which, having failed, would reset the sender's understanding of what routes their peer has seen). I'd rather we corrected that, rather than killing scalability just because we don't need it right now; it's not just the message, it's the repeated evaluation by receiving connectors.

michielbdejong commented 7 years ago

ok, so how do we go about that? a "give me your routes" request, maybe?

momerath42 commented 7 years ago

Sure; just something that indicates to the connector that it should reset the relevant entry in broadcaster.peerEpochs

michielbdejong commented 7 years ago

attempted fix here: https://github.com/interledgerjs/ilp-connector/pull/388 - will test tomorrow.

michielbdejong commented 7 years ago

Still have to see if the problem disappears in the live network after the next version of ilp-kit is published.