Skip to content

[Bug] FailureDetector.samples gains an entry for every address that ever sends a frame while forget() is only reachable from member-removal paths, so a non-member address is never reclaimed #947

Description

@pathosDev

Problem

FailureDetector.samples gains an entry for every address the node ever hears from, but forget() is only ever called from member-removal paths. An address that never becomes a member is therefore never reclaimed, and the map grows for the lifetime of the process.

handleWire bumps the detector from the connection peer before it dispatches anything, so any address that completes a hello and sends one frame gets a sample — whether or not it goes on to join. onGossip adds a second entry keyed on the payload's from, which the sender fully controls and which need not be a member either. Neither call has a matching removal. forget is reachable from onLeave, down, evaluateDowning and failureDetectionTick — all four take a Member out of this.members, so the reclaim path is coupled to membership, and an address that is never in the map cannot leave it.

This is not the same shape as #138, which is about the member map filling up under adversarial gossip and which the tombstone TTL bounds. The detector's map has no TTL, no prune tick and no cap, and it fills from ordinary traffic — a client-style peer that dials, sends a frame and goes away, a node that starts and is reconfigured before joining, a scanner. Every entry is a { lastSeen, everSeen } object keyed on a string; the cost per entry is small, so the failure mode is a slow leak in a long-running node rather than a fast one.

Evidence

Every frame bumps the detector, before any dispatch decision:

src/cluster/Cluster.ts:525-535
  private handleWire(from: NodeAddress, message: WireMessage): void {
    this.failureDetector.heartbeat(from);

    match(message)
      .with({ kind: 'heartbeat' }, (m) => this.onHeartbeat(from, m))
      .with({ kind: 'heartbeat-ack' }, () => this.onHeartbeatAcknowledgment())
      .with({ kind: 'gossip' }, (m) => this.onGossip(from, m))
      .with({ kind: 'envelope' }, (m) => this.onEnvelope(from, m))
      .with({ kind: 'leave' }, (m) => this.onLeave(from, m))
      .otherwise((m) => this.onUnhandledWire(m, from));
  }

and gossip adds a second, payload-keyed one:

src/cluster/Cluster.ts:606-608
  private onGossip(from: NodeAddress, message: GossipMessage): void {
    const sender = NodeAddress.fromJSON(message.from);
    this.failureDetector.heartbeat(sender);

heartbeat inserts unconditionally; the only removal is forget:

src/cluster/FailureDetector.ts:35-50
  /** Record that a message was received from `peer` (any message counts). */
  heartbeat(peer: NodeAddress, now: number = Date.now()): void {
    const key = peer.toString();
    const prev = this.samples.get(key);
    this.samples.set(key, { lastSeen: now, everSeen: prev?.everSeen ?? true });
  }

  /** Record that we know about a peer even if we haven't heard from it yet. */
  register(peer: NodeAddress, now: number = Date.now()): void {
    const key = peer.toString();
    if (!this.samples.has(key)) this.samples.set(key, { lastSeen: now, everSeen: false });
  }

  forget(peer: NodeAddress): void {
    this.samples.delete(peer.toString());
  }

Every forget call site takes a Member out of the map first — onLeave (Cluster.ts:753), down (:405), evaluateDowning (:888), failureDetectionTick (:812). None of them can reach an address that was never a member.

Proposal

Give the detector its own reclaim rule instead of borrowing membership's. Two options, either sufficient:

  • Prune on decide. failureDetectionTick already walks the member map; add a pass that drops any sample whose key is not a member and whose lastSeen is older than, say, 2 × downAfterMs. The detector is only consulted for members, so a sample that belongs to no member has no reader.
  • Only record what is asked about. handleWire's unconditional heartbeat(from) could be conditional on from being a known member, since nothing reads a sample for a non-member. That keeps the map exactly as large as the membership.

The first is the smaller change and also cleans up entries left by an address that was a member on some other path. The second removes the growth at the source. The gossip-payload bump at :608 should go regardless — it is the authority defect filed separately in this batch, and removing it also removes the attacker-controlled half of this growth.

Acceptance sketch

  • A sample for an address that is not a member is reclaimed within a bounded number of failure-detector ticks.
  • Sample count after N one-shot senders returns to the member count.
  • Reclaiming does not disturb a member that is legitimately silent but not yet down.
  • A test asserts the sample map does not outgrow the member map under sustained non-member traffic.

Verification status

Found in the ten-lens production-readiness review of 2026-08-05 (v0.13.0) and re-verified before filing: reproduced by execution. A single-node cluster with a stub transport, fed frames from distinct source addresses:

samples at start: 0  members: 1
samples after 20000 one-shot senders: 20000  members: 1
samples after 2000 payload-claimed addresses: 22001  members: 2001
samples after 2s of failure-detection ticks: 20001
heapUsed (MB): 14.2

The 20 000 one-shot senders used heartbeat-ack, whose handler is a no-op, so none of them ever became a member — and all 20 000 samples survived. The 2 000 gossip-payload addresses did become members (through the unauthorised sender-insert path), which is why they were reclaimed when the detector downed them: the map fell from 22 001 to 20 001, leaving exactly the non-member samples behind. That is the mechanism stated plainly — reclaim is a side effect of membership, and traffic that never joins is never reclaimed.

Part of the production-readiness review batch — tracked in #913.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority: mediumUseful, not urgentproduction-goalBlocks or defines the path to production readiness

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions