When deciding whether or not to fetch a repo based on a refs
announcement, we need to know whether our signed refs are stale or not,
compared to the ones in the announcement. This can be quite expensive as
it requires multiple reads from Git's ODB, which often results in
packfile loading.
To remedy this, we introduce a refs cache in our node database that is
consulted when an announcement is received.
We also remove the logic that generates `info` messages, since they are
rarely needed and introduced more complexity.
Additionally, we revert commit 3ad2b4431f,
which introduced logic to minimze wants and haves with additional logic,
since it potentially also increases the load on the ODB.
Instead of dropping the existing connection(s), we drop the same
connection on both sides, by using a simple rule that yields the same
result whether the node sees the connection as inbound or outbound.
The trick is to use something that both nodes agree on, for instance
whos public key is greater than the other, and use that information
to close the same connection on both sides.
Promote message logging to `debug` level, and change default level to
`info`.
This is because `trace` level often covers details at a different level
of detail than for eg. messages sent and received. This means we don't
have to use `trace` level just to see the gossip.
Instead of re-loading the full inventory on `rad init`, we simply add
the new repository to the in-memory cache.
We also keep track of whether a fetch was a full clone, to know when
to update our routing table.
For large seeds with many repos, it's expensive to traverse the
inventory everytime the node connects to a new peer, for example.
To improve the situation, we cache the inventory per `Storage` instance,
and add a `refresh` method to refresh the inventory.
This method is called when the node is told to synchronize its
inventory.
This change limits the amount of `wants` and `haves` data that is sent
to the serving side of a fetch.
When a `RefsAt` announcement is sent, the fetching peer can calculate
the difference between the `rad/sigrefs` they're aware of -- if it
exists -- and the newly advertised `rad/sigrefs`. This means they can
efficiently ask for the `wants` and `haves` of references that have
changed -- saving some data sent over to the serving side.
Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
X-Clacks-Overhead: GNU Terry Pratchett
Announcement are logged at the debug level and are very frequent. This
causes a lot of noisy logging in a node.
Downgrade the log levels for these messages from debug to trace.
Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
X-Clacks-Overhead: GNU Terry Pratchett
Two concurrent fetches can occur from the same peer, while also being
for a different set of references. This was seen in the
`test_outdated_sigrefs` test. Alice would have an ongoing fetch from
Eve, and concurrently, Eve will create a new issue -- thus creating a
new `RefsAt` message -- and Alice would attempt to fetch this.
The previous logic would only check that Alice is fetching from Eve
and ignore the new updates -- meaning the test could fail. However, if
the `RefsAt` are checked then the other fetch can be recognised as
new, and queue it.
Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
X-Clacks-Overhead: GNU Terry Pratchett
Dequeued fetches were always fetching all refs, instead of fetching the
specific refs that were initially requested. We fix that by storing the
refs to fetch in the fetch queue.
The previous change to the wire protocol introduced a bug: outbound
peers that failed to connect fully would not notify the service on
disconnect. This change fixes it.
Allow some margin of error when sending multiple inventory announcements
at a time. Previously, they might end up with the same timestamp, so
only the first announcement would be processed by nodes. Now, we ensure
that the timestamp is updated.
When a peer is performing a background and foreground fetch from two
different peers, the background fetch result was being used as the
result for the foreground fetch. This results in inconsistencies,
since the foreground peer has new updates from themselves.
The reason for this issue was the fetches that were queued were not
correctly being handled, and the other peer's results were being
returned instead.
This fixes the queueing and dequeueing by ensuring that the queued
fetch occurs, passing the correct result channel. This fetch will then
be subsequently dequeued correctly and the foreground fetch obtains
the correct result.
Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
X-Clacks-Overhead: GNU Terry Pratchett
This test is flakey and requires further investigation. Some extra
assertions are added to ensure that the expected behaviour is
happening upon fetching. It is unknown if the fetch queue is behaving
properly, and it's suspected that some existing fetch results might be
used for queued fetches.
Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
X-Clacks-Overhead: GNU Terry Pratchett
Previously, we might ban a node if we're having trouble connecting to
it, but this could be due to internet connectivity issues.
We introduce a penalty score for nodes, that increases if there are
connection issues or the node misbehaves, and decreases on successful
connections.
When connected to new nodes, we sort them so that low-penalty nodes are
attempted first.
Instead of having a routing database, and an addresses database,
consolidate that in a single database called `node.db`.
We also simplify database access, by providing a single type for opening
the database, and use traits to access the various functions.
We create a new `seed::Store` that has seed-related methods split out of
the `address::Store`.
Note that tracking is still in its own database currently, as it isn't
ephemeral state. This makes it easy to delete the new `node.db` without
risk of losing any important information.
To avoid renaming all database methods, we create a `Stores` wrapper
that allows us to access routing, addresses, seeds etc. without type
ambiguity.
This is a breaking change and requires `addresses.db` and `routing.db`
to be deleted.
cloudhead
Fintan Halpenny
Alexis Sellier