Skip to content

bug(client): coalesce network sampling so SyncHost cannot delay AnnounceHost #2020

Description

@hhhhsdxxxx

Summary

With many concurrent SyncHost streams, a client can delay its scheduler AnnounceHost heartbeat by minutes even when scheduler.announceInterval is configured to one minute.

SchedulerAnnouncer::make_announce_host_request() and every upload-side SyncHost stream call the same SystemMonitor.network.get_stats().await. In Network::get_stats(), a shared tokio::sync::Mutex is held across a real one-second sleep while obtaining two counter snapshots for bandwidth calculation.

The upload SyncHost loop has no independent sampling interval: it calls get_stats() again after each send. Consequently, each active stream can occupy one second of the shared sampler, and AnnounceHost queues behind those samples before it can enter SchedulerClient::announce_host(). With hundreds of streams this readily becomes a multi-minute heartbeat delay.

This logic is present in v1.5.3 and in current main; the same pattern also exists in v1.3.2.

Sanitized observation

The following is a representative, redacted observation. Timestamps, node identifiers, peer IDs, URLs, and request metadata are intentionally removed.

DEBUG dragonfly_client::grpc::dfdaemon_upload: received sync host request
DEBUG dragonfly_client::grpc::dfdaemon_upload: received sync host request
DEBUG dragonfly_client::grpc::dfdaemon_upload: received sync host request
...
# no scheduler-client "announce host to ..." log was emitted during the following several minutes

The scheduler metric that counts RPC entry likewise did not increase during the observation window. This points to waiting before the AnnounceHost RPC, rather than scheduler-side rejection or de-duplication.

Relevant code

Expected behavior

Concurrent SyncHost streams should not make the scheduler heartbeat wait proportionally to their count. The heartbeat should normally be sent at its configured interval, using a recent network snapshot when one is available.

Proposed direction

Use one coalescing network sampler rather than allowing every caller to perform an independent one-second sample:

  1. Keep a shared snapshot containing NetworkStats, sampled_at, and a generation/version.
  2. A caller reads and returns the snapshot immediately when it is within the freshness window.
  3. When the snapshot is stale and no sampling pass is in flight, that caller triggers exactly one real pass (two counter reads separated by the required one-second sleep).
  4. When stale data is requested while that pass is in flight, other callers wait for the same pass to complete, then consume its snapshot instead of starting additional samples.
  5. SyncHost should wait for the next generation (or receive a watch/notification update) instead of tight-looping and repeatedly sending a still-fresh snapshot.
  6. AnnounceHost can use the most recent snapshot while a refresh is in progress, if bounded heartbeat latency is preferred over waiting for a fresh bandwidth sample.

This preserves actual interval-based bandwidth measurement while preventing N concurrent streams from turning into N serialized seconds.

Useful observability

Metrics or debug counters for sampler-in-flight, sampler wait duration, active SyncHost streams, and AnnounceHost scheduling/sending latency would make this regressible and easier to diagnose.

Environment

Observed with client v1.5.3 and scheduler v2.4.4. Source inspection indicates the same behavior in client v1.3.2 and current main.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions