refactor(bigtable): 1x heartbeat + drop `active := 1` code smell
Two follow-ups after the java-bigtable heartbeat cross-check:
1) handleSessionParameters: drop 3x multiplier on nextHeartbeatDeadline
(already done in resetHeartbeatDeadline in the prior PR-2 commit).
Now 1 * interval, matching Java's SessionImpl.java:440 which sets
nextHeartbeat = now + heartbeatInterval on every reset. Wake-comment
updated to reflect the new arithmetic.
2) heartBeatLoop:
- Delete the `active := 1 // multiplex=1; kept as %d so a future >1
stays greppable` code smell. Inline `1` was documenting itself;
the variable existed only for its comment. Drop it — when
multiplex > 1 lands, the caller will read `s.activeVRPCCount()`
(or similar), not this stale scaffolding.
- `lastFrameAge = interval - remaining` (was `3*interval - remaining`)
to reflect the new 1x deadline.
- Debugf/recordEvent format strings drop the `in_flight=%d` field
that was carrying the constant `1`.
Test adjustments:
- TestHandleSessionParameters_UpdatesIntervalAndDeadline: expect
deadline ~= before+interval (1x) instead of before+3*interval.
- TestHeartBeatLoop_HeartbeatsKeepInflightVRPCAlive: seed deadline
at now+30ms (1x) instead of now+90ms (was 3x).
diff --git a/bigtable/internal/transport/session_lifecycle.go b/bigtable/internal/transport/session_lifecycle.go
index 255a794..0591774 100644
--- a/bigtable/internal/transport/session_lifecycle.go
+++ b/bigtable/internal/transport/session_lifecycle.go
@@ -335,11 +335,11 @@
return
}
s.heartbeatIntervalNano.Store(int64(interval))
- s.nextHeartbeatDeadlineNano.Store(time.Now().Add(3 * interval).UnixNano())
+ s.nextHeartbeatDeadlineNano.Store(time.Now().Add(interval).UnixNano())
// Wake the watchdog: this is the sole path that changes the interval
// itself (not just the deadline), so the Timer must re-evaluate
- // against 3×interval instead of the initialHeartbeatGrace bootstrap
- // it was armed to at NewSession.
+ // against the new interval instead of the initialHeartbeatGrace
+ // bootstrap it was armed to at NewSession.
s.wakeHeartbeatLoop()
}
@@ -551,20 +551,20 @@
timer.Reset(interval)
continue
}
- active := 1 // multiplex=1; kept as %d so a future >1 stays greppable.
remaining := time.Until(time.Unix(0, s.nextHeartbeatDeadlineNano.Load()))
- // last-frame age = (deadline - now) inverted into "how long
- // since the last frame extended us" = 3*interval - remaining.
- lastFrameAge := 3*interval - remaining
+ // last-frame age = interval - remaining (deadline is set to
+ // now+interval on every inbound/outbound frame via
+ // resetHeartbeatDeadline).
+ lastFrameAge := interval - remaining
if remaining > 0 {
// Deadline was pushed out while we were sleeping; re-arm.
// Only record when last_frame_age has crossed one interval —
// otherwise every healthy session would spam the UI ring
- // buffer ~3x/second and drown out close/missed events.
+ // buffer and drown out close/missed events.
if lastFrameAge >= interval {
- s.recordEvent("hb-alive", "in_flight=%d last_frame_age=%v remaining=%v interval=%v",
- active, lastFrameAge, remaining, interval)
+ s.recordEvent("hb-alive", "last_frame_age=%v remaining=%v interval=%v",
+ lastFrameAge, remaining, interval)
}
timer.Reset(remaining)
continue
@@ -574,10 +574,10 @@
// ForceClose so we have a definitive marker even if downstream
// cancel races.
recordDebugTag(tagSessionHeartbeatMissed)
- s.debugf("heartbeat MISSED — forcing close in_flight=%d last_frame_age=%v interval=%v",
- active, lastFrameAge, interval)
- s.recordEvent("hb-missed", "in_flight=%d last_frame_age=%v interval=%v",
- active, lastFrameAge, interval)
+ s.debugf("heartbeat MISSED — forcing close last_frame_age=%v interval=%v",
+ lastFrameAge, interval)
+ s.recordEvent("hb-missed", "last_frame_age=%v interval=%v",
+ lastFrameAge, interval)
s.ForceClose(&spb.CloseSessionRequest{
Reason: spb.CloseSessionRequest_CLOSE_SESSION_REASON_MISSED_HEARTBEAT,
Description: "client terminated session due to missed server heartbeats",
diff --git a/bigtable/internal/transport/session_lifecycle_test.go b/bigtable/internal/transport/session_lifecycle_test.go
index 7d3a41c..3d1aa49 100644
--- a/bigtable/internal/transport/session_lifecycle_test.go
+++ b/bigtable/internal/transport/session_lifecycle_test.go
@@ -191,7 +191,10 @@
if gotInterval != 2*time.Second {
t.Errorf("heartbeatInterval = %v, want 2s", gotInterval)
}
- wantMin := before.Add(5 * time.Second)
+ // Deadline should be about before+interval (1x, matching
+ // resetHeartbeatDeadline). Below-interval floor tolerates the
+ // scheduling gap between capturing `before` and the atomic store.
+ wantMin := before.Add(time.Second)
if gotDeadline.Before(wantMin) {
t.Errorf("nextHeartbeatDeadline = %v, want >= %v", gotDeadline, wantMin)
}
@@ -432,7 +435,7 @@
func TestHeartBeatLoop_HeartbeatsKeepInflightVRPCAlive(t *testing.T) {
s, _ := makeActive(t, SessionHooks{})
s.heartbeatIntervalNano.Store(int64(30 * time.Millisecond))
- s.nextHeartbeatDeadlineNano.Store(time.Now().Add(90 * time.Millisecond).UnixNano()) // 3 * interval
+ s.nextHeartbeatDeadlineNano.Store(time.Now().Add(30 * time.Millisecond).UnixNano()) // 1 * interval
s.setSlotForTest(&vrpcImpl{id: 1, resultChan: make(chan vrpcResult, 1)})
ctx, cancel := context.WithCancel(context.Background())