Skip to content

bug(mq): DuplicateWindow() returns the shortest window; retention warning needs the longest #690

Open

Description

@EricAndrechek

Area: mq — external NATS retention/duplicate-window reporting

ExternalNATS.DuplicateWindow (internal/mq/external.go:571) returns the shortest duplicate_window among the partition streams:

func (e *ExternalNATS) DuplicateWindow(ctx context.Context) (time.Duration, error) {
    var shortest time.Duration
    for _, name := range e.partitions {
        s, err := e.js.Stream(ctx, name)
        ...
        if d := s.CachedInfo().Config.Duplicates; shortest == 0 || d < shortest {
            shortest = d
        }
    }
    return shortest, nil
}

Its only production caller, App.warnShortRetention (internal/app/wire_nats.go:100, invoked from wireNATS/wire_nats.go:67-73 at boot and again whenever tenants are (re)adopted), uses that value as the threshold below which a tenant's dedupe retention is considered dangerously short:

for table, r := range store.DedupeRetentions() {
    if r > 0 && r < window {
        slog.Warn("dedupe retention is shorter than the nats partitions' duplicate_window: ...")
    }
}

The risk warnShortRetention exists to catch is a tenant's dedupe idempotency key expiring before the queue's own duplicate-suppression window closes — at which point a retried publish is no longer deduped by the dedupe store, but is still deduped (silently dropped) by the queue, and the client is told its retry was accepted. That risk is driven by the longest partition's duplicate_window a tenant might publish to, not the shortest: a tenant's retention could be longer than the shortest partition's window but still shorter than another partition's, and still hit the failure mode for any table hashed to that partition. Using the shortest window as the threshold means r < window is checked against too small a value, so this warning can stay silent for a retention that is genuinely unsafe against a longer-windowed partition.

  • Evidence: inferred, from reading DuplicateWindow and warnShortRetention together; not reproduced against non-uniform partitions.
  • This only matters when partitions' duplicate_windows actually differ — under the generated topology (wavehouse mq manifests) every partition gets the same formula (max(2m, minDuplicateWindow(), dedupeDuplicateWindow()), nats_manifests.go:254), so it is a hazard for hand-edited or non-uniform partitions, similar in scope to mq: durable/partition topology checks miss backoff, pause_until, max_waiting, subject_transform, sources #689.

Reproduce/confirm: configure two partitions with different duplicate_window values (e.g. via a hand-edited nack CR), set a tenant's dedupe retention between the shortest and the longest, and check whether warnShortRetention logs a warning for tables hashed to the longer-windowed partition.

Found in review of #624.

Related: #624, #613, #689.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/configConfig file, config knobs, hot-reloadbugSomething isn't working

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions