Skip to content

Revise the usage of Time and TimeGetter in p2p #1443

Open
@ImplOfAnImpl

Description

@ImplOfAnImpl

Time and TimeGetter are used frequently in p2p to represent/obtain some abstract point in time, whose only purpose is to be able to calculate a time difference later. The problem is that the "production" implementation of the time getter uses the wall clock, which is not monotonic, so there is a possibility for the difference to become negative (and even if it's positive, a forward time jump can mess timeouts).

One solution for this is to use Instant::now() instead of the time getter in such situations; the downside is that this will make the time much harder to mock in tests (via tokio::time::advance) and some existing tests will have to be refactored to certain extent.

On the other hand, p2p sometimes uses tokio::time::timeout, which is basically the same as using Instant, and it's inconsistent with the TimeGetter-based approach.

The proper solution is probably to have two separate time getters or to make the current one be able to provide both kinds of time.
Also, the usages of tokio::time::timeout should probably be replaced with a custom timeout function that would use the monotonic time getter. But this will require some refactoring of existing tests, because advancing the timer getter will now be required for p2p components to make progress.

Metadata

Metadata

Assignees

No one assigned

    Labels

    p2pp2p related issues

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions