Fix gossip client timeout: TimeSpan .Milliseconds vs .TotalMilliseconds

[tiagonapoli]

Summary

Fixes a TimeSpan component-vs-total misuse that silently disables the gossip client's operation timeout, plus related occurrences of the same defect pattern.

Bug 1 (behavioral) — gossip client timeout never fires

GarnetServerNode built the gossip GarnetClient with:

timeoutMilliseconds: opts.ClusterTimeout <= 0 ? 0 : TimeSpan.FromSeconds(opts.ClusterTimeout).Milliseconds

TimeSpan.Milliseconds returns the sub-second component (0–999), not the total duration. For any whole-second ClusterTimeout (the type is int seconds, default 60), this evaluates to 0, which disables GarnetClient's TimeoutChecker. Result: gossip operations effectively never time out on the per-operation client timeout.

Fix: compute the value with TotalMilliseconds, extracted into a testable helper:

internal static int GetClientTimeoutMilliseconds(int clusterTimeoutSeconds)
    => clusterTimeoutSeconds <= 0 ? 0 : (int)TimeSpan.FromSeconds(clusterTimeoutSeconds).TotalMilliseconds;

Same-pattern cleanups

• GarnetServerOptions.LoggingFrequency was TimeSpan.FromSeconds(5).Seconds — correct only because 5 < 60, but a latent trap. Simplified to the plain int-seconds literal 5 (value unchanged).
• RespAofTests (RMW + Upsert recover tests) asserted recoveredValueTtl.Value.Milliseconds < 8500. Since the milliseconds component is always 0–999, this assertion was a silent no-op. Corrected to TotalMilliseconds with the real ceiling of 10_000 (Key2 is set with a 10s expiry), so it now actually validates 0 < ttl ≤ 10s.

Tests

• New GarnetServerNodeTimeoutTests (7 cases) directly covers GetClientTimeoutMilliseconds, including the regression case 60 → 60000 (returns 0 under the old code).
• Verified by reverting the fix: 4/7 timeout tests fail on revert; both RespAofTests continue to pass with the strengthened assertions.

Scope

Isolated fix branch off main — contains only these four files. No functional changes beyond the timeout correctness fix and the same-pattern cleanups.

[Copilot]

Pull request overview

Fixes a TimeSpan component-vs-total misuse that caused the cluster gossip client operation timeout to be computed as 0 for whole-second values (disabling GarnetClient’s timeout checker). Also cleans up related same-pattern issues and adds regression coverage.

Changes:

• Add GarnetServerNode.GetClientTimeoutMilliseconds(int) and use it when constructing the gossip GarnetClient.
• Replace LoggingFrequency’s TimeSpan.FromSeconds(5).Seconds with the equivalent literal 5.
• Strengthen AOF recovery TTL assertions to use TotalMilliseconds and add a new regression test fixture for timeout computation.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
test/standalone/Garnet.test.scripting/RespAofTests.cs	Fix TTL assertions to use total duration rather than component milliseconds.
test/cluster/Garnet.test.cluster/GarnetServerNodeTimeoutTests.cs	Add regression tests for gossip client timeout computation.
libs/server/Servers/GarnetServerOptions.cs	Simplify LoggingFrequency initialization to a literal seconds value.
libs/cluster/Server/Gossip/GarnetServerNode.cs	Fix gossip client timeout computation by using total milliseconds via a helper.