There is no point in keeping an expired route, which is not selected, as
part of the fallback routes with infinite metric. Either there was no
selected route, so it can be removed, or the selected route got
retracted in which case that already caused a retraction to be sent.
Closes#330
Signed-off-by: Lee Smet <lee.smet@hotmail.com>
If an update is unfeasible, apply it as per the spec but don't update
the route timer. The main reason unfeasible routes are kept, is because
"it migh become feasible in the future". In our case, since we use
reliable transports (for protocol messages), and we send a seqno request
when a route is lost, we know that if a feasible route exists in one of
our peers, we will get it shortly after a route is lost. Even if we keep
the unfeasible route as fallback route, it won't be selected, and a
seqno request is needed to refresh it. In short, there is functionally
no difference for the route if we keep the unfeasible entry around or
not: a seqno request _must_ be send to a peer in the hopes of getting a
valid route again. All things considered, this changes allows unfeasible
routes to gracefully go out of scope, after which the unfeasible update
will be blocked.
Closes#320Closes#159
Signed-off-by: Lee Smet <lee.smet@hotmail.com>
If an unfeasible update is applied to an existing, unselected route, we
now skip route selection altogether. Since the route is not selected,
it can obviously not be unselected, and selecting it won't be possible
since its now not feasible.
By immediately returning we also can't notify downstream peers which
have sent a seqno request for this combination. This is fine, since if
there is an outstanding request for this seqno, considering it is
unfeasible, it won't be of interest to the downstream peer. Note that if
the seqno request triggered a bump of the route seqno, the _first_ update
is always feasible.
Also expose a metric value to track how often we just skip route
selection.
Closes#329
Signed-off-by: Lee Smet <lee.smet@hotmail.com>
This avoids duplication of the field between all RouteEntries in the
same RouteList, which should reduce memory consumption (especially on
public nodes) of the route table.
Closes#321
Signed-off-by: Lee Smet <lee.smet@hotmail.com>
This starts using structured logging a bit, causing log messages to
mostly more static strings, and the actual error messages, specific
endpoint addresses etc. be log fields.
This makes it easier to grep for specific log messages, as well as
filter things down for a specific peer.
Code-wise, this is mostly a matter of moving from format-strings
interpolating various variables to actually putting these in front of
the message.
Frequently, we can also avoid writing it out explicitly alltogether:
In `connect_peer` we already have `endpoint`, so we can ask it for
`proto()` and `address()`, and just record it in a span, and it'll be
present in all child log messages - as it's somewhere part of the span
hierarchy and printed by our log formatters.
In addition to the previous format "compact", which is the default, this
can be set to "logfmt".
This isn't colorful yet, so `ansi_logs` is not enabled, as we'd only
want that for interactive usage, not when run via a service manager.
There's some amount of duplication between the two CLI entrypoints, but
I was a bit unsure where to put the shared code for tracing setup.
Happy to move this elsewhere too.
Where we already use it, use the % sigil to inform tracing we want to
use the display impl of the fields to log them.
Signed-off-by: Lee Smet <lee.smet@hotmail.com>
Previously we got the index of a possible entry and then loaded the
entry (using the now removed IndexMut impl). Now we just get the entry
and unwrap it based on it's internal value.
Signed-off-by: Lee Smet <lee.smet@hotmail.com>
Since it returned a &mut RouteEntry instead of a RouteEntryGuard, items
returned by it are not updated, so outright remove it.
Signed-off-by: Lee Smet <lee.smet@hotmail.com>
Instead of iterating, which is wastefull since the RouteList internally
keeps the selected route as the first element (if one exists).
Signed-off-by: Lee Smet <lee.smet@hotmail.com>
The flag is not needed, since these types are always the result of an
iterator, therefore the returned value always exists.
Signed-off-by: Lee Smet <lee.smet@hotmail.com>
The cache seems to stay small when observed on nodes connected to the
network, but still it's good practice to clean up after ourselves. For
now we aggressively shrink as much as possible since the cache is
generally small. If needed (i.e. it is found the cache adds a bunch of
noticeable overhead), we can choose to make this less aggressive in the
future.
Closes#291
Signed-off-by: Lee Smet <lee.smet@hotmail.com>
Since the full setup requires waiting for a client to send data, move
this to a separate future collection which gets polled next to the main
accept future. Also limit the amount of time the future is allowed to
take by wrapping it in a timeout.
Fixes#309
Signed-off-by: Lee Smet <lee.smet@hotmail.com>
Maxime Van Hees