From 2ab7de917301799486cc4623124d74034c6d9cbc Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Thu, 10 Oct 2024 15:35:35 +0100 Subject: [PATCH] raft: don't panic in Inflights.Add In lazy replication mode, the inflights struct should not enforce the in-flight limit. Instead, this policy is implemented at the higher level (RACv2). The tracker nevertheless correctly tracks all the in-flight state, so that it can correctly switch in/out of the lazy replication. Epic: none Release note: none --- pkg/raft/tracker/inflights.go | 16 +++++++++------- pkg/raft/tracker/inflights_test.go | 17 ++++++----------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/pkg/raft/tracker/inflights.go b/pkg/raft/tracker/inflights.go index 6a09ff37a21e..b9bb090ed9ed 100644 --- a/pkg/raft/tracker/inflights.go +++ b/pkg/raft/tracker/inflights.go @@ -64,13 +64,15 @@ func (in *Inflights) Clone() *Inflights { } // Add notifies the Inflights that a new message with the given index and byte -// size is being dispatched. Full() must be called prior to Add() to verify that -// there is room for one more message, and consecutive calls to Add() must -// provide a monotonic sequence of indexes. +// size is being dispatched. Consecutive calls to Add() must provide a monotonic +// sequence of log indices. +// +// The caller should check that the tracker is not Full(), before calling Add(). +// If the tracker is full, the caller should hold off sending entries to the +// peer. However, Add() is still valid and allowed, for cases when this pacing +// is implemented at the higher app level. The tracker correctly tracks all the +// in-flight entries. func (in *Inflights) Add(index, bytes uint64) { - if in.Full() { - panic("cannot add into a Full inflights") - } next := in.start + in.count size := in.size if next >= size { @@ -134,7 +136,7 @@ func (in *Inflights) FreeLE(to uint64) { // Full returns true if no more messages can be sent at the moment. func (in *Inflights) Full() bool { - return in.count == in.size || (in.maxBytes != 0 && in.bytes >= in.maxBytes) + return in.count >= in.size || (in.maxBytes != 0 && in.bytes >= in.maxBytes) } // Count returns the number of inflight messages. diff --git a/pkg/raft/tracker/inflights_test.go b/pkg/raft/tracker/inflights_test.go index a7b1c7f5c2db..dafcd15d2a08 100644 --- a/pkg/raft/tracker/inflights_test.go +++ b/pkg/raft/tracker/inflights_test.go @@ -1,3 +1,6 @@ +// This code has been modified from its original form by The Cockroach Authors. +// All modifications are Copyright 2024 The Cockroach Authors. +// // Copyright 2019 The etcd Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -203,26 +206,18 @@ func TestInflightsFull(t *testing.T) { addUntilFull := func(begin, end int) { for i := begin; i < end; i++ { - if in.Full() { - t.Fatalf("full at %d, want %d", i, end) - } + require.False(t, in.Full(), "full at %d, want %d", i, end) in.Add(uint64(i), uint64(100+i)) } - if !in.Full() { - t.Fatalf("not full at %d", end) - } + require.True(t, in.Full()) } addUntilFull(0, tc.fullAt) in.FreeLE(tc.freeLE) addUntilFull(tc.fullAt, tc.againAt) - defer func() { - if r := recover(); r == nil { - t.Errorf("Add() did not panic") - } - }() in.Add(100, 1024) + require.True(t, in.Full()) // the full tracker remains full }) } }