seed: Don't error if self isn't found

[cloudhead]

No description provided.

[xla]

@FintanH If I understand correctly, this should never happen with the recent fixes you landed. If the peer is in the tracked there should be a valid id for it, right?

[FintanH]

If I understand correctly, this should never happen with the recent fixes you landed. If the peer is in the tracked there should be a valid id for it, right?

I don't think so actually. We were able to get rad self before, possibly due to it being a symref -- but I'm honestly not sure.

I guess it depends on the semantics here anyway. If there is an error in trying to retrieve the user, is it show-stopping for the seed or can it function without it?

[kim]

Iiuc, the code just walks the entire storage to find some rad/self branch corresponding to a given PeerId. It may not find one because a. we have no tracking relationship to this peer, b. we did not yet receive all data, c. we are not finished updating refs (nb. we do not have atomic operations in lieu of a custom refdb), or d. some other error occurred.

A more efficient approach would be to perform a search only on the refs (ie. refs/namespaces/*/refs/remotes/{}/rad/self, peer_id), and access the odb only if that yields a result. It should be noted that there is no guarantee that any two refs/remotes/<pid>/rad/self point to the same identity, nor that they point to the same head if they do.

Maintaining an index PeerId -> [Identity] for arbitrary PeerIds is not a trivial problem in general, as it requires to reverse the arrow (ie. replicate all possible identities on the network). I do not agree that there are any benefits in doing so -- it would be enough to look up given PeerId in the set of known (that is, tracked) identities (it is possible to do so relatively efficiently, as the remotes are set up in the form <identity root>/<peer id>). I'd be happy to review an RFC claiming otherwise, though.

In general, I think that ignoring all errors is not a good idea -- there's a plethora of things that can go wrong, and swallowing IO errors f.ex. will not improve the robustness of the seed node executable.

[cloudhead]

A more efficient approach would be to perform a search only on the refs (ie. refs/namespaces/*/refs/remotes/{}/rad/self, peer_id), and access the odb only if that yields a result. It should be noted that there is no guarantee that any two refs/remotes//rad/self point to the same identity, nor that they point to the same head if they do.

Cool, maybe that's something we can implement in librad at some point, as this functionality is used in a couple of places now.

In general, I think that ignoring all errors is not a good idea -- there's a plethora of things that can go wrong, and swallowing IO errors f.ex. will not improve the robustness of the seed node executable.

Fixed!

[xla]

🍠 🚌 🌒 💕