From c5b65ee9f7582e085ab43021f768c1124de68ea7 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Fri, 11 Feb 2022 15:05:07 +0800 Subject: [PATCH 01/18] initial Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_api.go | 120 ++++++++++++++++++++++++++++++++++++++++ ddl/ddl_tiflash_test.go | 28 ++++++++++ 2 files changed, 148 insertions(+) diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index 2fdaac9b531ae..a1796244a4028 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -56,12 +56,123 @@ type TiFlashReplicaStatus struct { HighPriority bool } +type TickType float64 + +// PollTiFlashBackoffElement records backoff for each TiFlash Table. +// When `Counter` reached `Threshold`, `Threshold` shall grow. +type PollTiFlashBackoffElement struct { + Counter int + Threshold TickType +} + +// NewPollTiFlashBackoffElement initialize backoff element for a TiFlash table. +func NewPollTiFlashBackoffElement() *PollTiFlashBackoffElement { + return &PollTiFlashBackoffElement{ + Counter: 0, + Threshold: PollTiFlashBackoffMinTick, + } +} + +// PollTiFlashBackoffContext is a collection of all backoff states. +type PollTiFlashBackoffContext struct { + MinTick TickType + MaxTick TickType + // Capacity limits tables a backoff pool can handle, in order to limit handling of big tables. + Capacity int + Rate TickType + elements map[int64]*PollTiFlashBackoffElement +} + +// NewPollTiFlashBackoffContext creates an instance of PollTiFlashBackoffContext. +func NewPollTiFlashBackoffContext(MinTick, MaxTick TickType, Capacity int, Rate TickType) PollTiFlashBackoffContext { + return PollTiFlashBackoffContext{ + MinTick: MinTick, + MaxTick: MaxTick, + Capacity: Capacity, + elements: make(map[int64]*PollTiFlashBackoffElement), + } +} + + // TiFlashManagementContext is the context for TiFlash Replica Management type TiFlashManagementContext struct { TiFlashStores map[int64]helper.StoreStat HandlePdCounter uint64 UpdateTiFlashStoreCounter uint64 UpdateMap map[int64]bool + Backoff PollTiFlashBackoffContext +} + +// Tick will first check increase Counter. +func (b *PollTiFlashBackoffContext) Tick(ID int64) bool { + e, ok := b.Get(ID) + if !ok { + return false + } + growed := e.MaybeGrow(b) + e.Counter += 1 + return growed +} + +// Limit returns current limit +func (e *PollTiFlashBackoffElement) Limit() int { + return int(e.Threshold) +} + +// NeedGrow returns if we need to grow +func (e *PollTiFlashBackoffElement) NeedGrow() bool { + if e.Counter >= e.Limit() { + return true + } + return false +} + +func (e *PollTiFlashBackoffElement) doGrow(b *PollTiFlashBackoffContext) { + defer func() { + e.Counter = 0 + }() + if e.Threshold < b.MinTick { + e.Threshold = b.MinTick + } + if e.Threshold * b.Rate > b.MaxTick { + e.Threshold = b.MaxTick + } + e.Threshold *= b.Rate +} + +// MaybeGrow grows threshold and reset counter when need +func (e *PollTiFlashBackoffElement) MaybeGrow(b *PollTiFlashBackoffContext) bool { + if !e.NeedGrow() { + return false + } + e.doGrow(b) + return true +} + +// Remove will reset table from backoff +func (b *PollTiFlashBackoffContext) Remove(ID int64) { + delete(b.elements, ID) +} + +// Get returns pointer to inner PollTiFlashBackoffElement. +func (b *PollTiFlashBackoffContext) Get(ID int64) (*PollTiFlashBackoffElement, bool) { + res, ok := b.elements[ID] + return res, ok +} + +// Put will put table into backoff pool, if there are enough room, or returns false. +func (b *PollTiFlashBackoffContext) Put(ID int64) bool { + _, ok := b.elements[ID] + if ok || b.Len() < b.Capacity { + b.elements[ID] = NewPollTiFlashBackoffElement() + return true + } + return false +} + +// Len gets size of PollTiFlashBackoffContext. +func (b *PollTiFlashBackoffContext) Len() int { + return len(b.elements) } // NewTiFlashManagementContext creates an instance for TiFlashManagementContext. @@ -71,6 +182,7 @@ func NewTiFlashManagementContext() *TiFlashManagementContext { UpdateTiFlashStoreCounter: 0, TiFlashStores: make(map[int64]helper.StoreStat), UpdateMap: make(map[int64]bool), + Backoff: NewPollTiFlashBackoffContext(PollTiFlashBackoffMinTick, PollTiFlashBackoffMaxTick, PollTiFlashBackoffCapacity, PollTiFlashBackoffRate), } } @@ -81,6 +193,14 @@ var ( PullTiFlashPdTick = atomicutil.NewUint64(30 * 5) // UpdateTiFlashStoreTick indicates the number of intervals before we fully update TiFlash stores. UpdateTiFlashStoreTick = atomicutil.NewUint64(5) + // PollTiFlashBackoffMaxTick is the max tick before we try to update TiFlash replica availability for one table. + PollTiFlashBackoffMaxTick TickType = 10 + // PollTiFlashBackoffMinTick is the min tick before we try to update TiFlash replica availability for one table. + PollTiFlashBackoffMinTick TickType = 1 + // PollTiFlashBackoffCapacity is the cache size of backoff struct. + PollTiFlashBackoffCapacity int = 1000 + // PollTiFlashBackoffRate is growth rate of exponential backoff threshold. + PollTiFlashBackoffRate TickType = 1000 ) func getTiflashHTTPAddr(host string, statusAddr string) (string, error) { diff --git a/ddl/ddl_tiflash_test.go b/ddl/ddl_tiflash_test.go index ba7a68a5379f4..2a7506810debf 100644 --- a/ddl/ddl_tiflash_test.go +++ b/ddl/ddl_tiflash_test.go @@ -21,7 +21,9 @@ package ddl_test import ( "context" "fmt" + "github.com/stretchr/testify/require" "math" + "testing" "time" . "github.com/pingcap/check" @@ -569,3 +571,29 @@ func (s *tiflashDDLTestSuite) TestSetPlacementRuleFail(c *C) { res := s.CheckPlacementRule(*expectRule) c.Assert(res, Equals, false) } + +func TestTiFlashBackoff(t *testing.T) { + var maxTick ddl.TickType = 10 + var rate ddl.TickType = 1.5 + backoff := ddl.NewPollTiFlashBackoffContext(1, maxTick, 10, rate) + backoff.Put(1) + mustGet := func(ID int64) *ddl.PollTiFlashBackoffElement { + e, ok := backoff.Get(1) + require.True(t, ok) + return e + } + mustGrow := func(ID int64) { + e := mustGet(ID) + ori := e.Threshold + backoff.Tick(ID) + require.Equal(t, rate * ori, e.Threshold) + } + require.False(t, mustGet(1).NeedGrow()) + mustGrow(1) + + backoff.Put(2) + for i := 0; i < 10; i++ { + backoff.Tick(2) + } + require.Equal(t, mustGet(2).Threshold, maxTick) +} From 2f81e5b16bf1edca893723bf34b42734feb37279 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Fri, 11 Feb 2022 20:55:32 +0800 Subject: [PATCH 02/18] fix Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_api.go | 17 +++++++++-------- ddl/ddl_tiflash_test.go | 26 ++++++++++++++++++++------ 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index a1796244a4028..eb3618037b7bd 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -75,11 +75,11 @@ func NewPollTiFlashBackoffElement() *PollTiFlashBackoffElement { // PollTiFlashBackoffContext is a collection of all backoff states. type PollTiFlashBackoffContext struct { - MinTick TickType - MaxTick TickType + MinTick TickType + MaxTick TickType // Capacity limits tables a backoff pool can handle, in order to limit handling of big tables. Capacity int - Rate TickType + Rate TickType elements map[int64]*PollTiFlashBackoffElement } @@ -90,17 +90,17 @@ func NewPollTiFlashBackoffContext(MinTick, MaxTick TickType, Capacity int, Rate MaxTick: MaxTick, Capacity: Capacity, elements: make(map[int64]*PollTiFlashBackoffElement), + Rate: Rate, } } - // TiFlashManagementContext is the context for TiFlash Replica Management type TiFlashManagementContext struct { TiFlashStores map[int64]helper.StoreStat HandlePdCounter uint64 UpdateTiFlashStoreCounter uint64 UpdateMap map[int64]bool - Backoff PollTiFlashBackoffContext + Backoff PollTiFlashBackoffContext } // Tick will first check increase Counter. @@ -134,10 +134,11 @@ func (e *PollTiFlashBackoffElement) doGrow(b *PollTiFlashBackoffContext) { if e.Threshold < b.MinTick { e.Threshold = b.MinTick } - if e.Threshold * b.Rate > b.MaxTick { + if e.Threshold*b.Rate > b.MaxTick { e.Threshold = b.MaxTick + } else { + e.Threshold *= b.Rate } - e.Threshold *= b.Rate } // MaybeGrow grows threshold and reset counter when need @@ -182,7 +183,7 @@ func NewTiFlashManagementContext() *TiFlashManagementContext { UpdateTiFlashStoreCounter: 0, TiFlashStores: make(map[int64]helper.StoreStat), UpdateMap: make(map[int64]bool), - Backoff: NewPollTiFlashBackoffContext(PollTiFlashBackoffMinTick, PollTiFlashBackoffMaxTick, PollTiFlashBackoffCapacity, PollTiFlashBackoffRate), + Backoff: NewPollTiFlashBackoffContext(PollTiFlashBackoffMinTick, PollTiFlashBackoffMaxTick, PollTiFlashBackoffCapacity, PollTiFlashBackoffRate), } } diff --git a/ddl/ddl_tiflash_test.go b/ddl/ddl_tiflash_test.go index 2a7506810debf..b1419dc9a5cc7 100644 --- a/ddl/ddl_tiflash_test.go +++ b/ddl/ddl_tiflash_test.go @@ -578,22 +578,36 @@ func TestTiFlashBackoff(t *testing.T) { backoff := ddl.NewPollTiFlashBackoffContext(1, maxTick, 10, rate) backoff.Put(1) mustGet := func(ID int64) *ddl.PollTiFlashBackoffElement { - e, ok := backoff.Get(1) + e, ok := backoff.Get(ID) require.True(t, ok) return e } + mustNotGrow := func(ID int64) { + e := mustGet(ID) + ori := e.Threshold + c := e.Counter + growed := backoff.Tick(ID) + require.False(t, growed) + require.Equal(t, e.Threshold, ori) + require.Equal(t, e.Counter, c+1) + } mustGrow := func(ID int64) { e := mustGet(ID) ori := e.Threshold - backoff.Tick(ID) - require.Equal(t, rate * ori, e.Threshold) + growed := backoff.Tick(ID) + require.True(t, growed) + require.Equal(t, e.Threshold, rate*ori) + require.Equal(t, 1, e.Counter) } require.False(t, mustGet(1).NeedGrow()) + mustNotGrow(1) mustGrow(1) backoff.Put(2) - for i := 0; i < 10; i++ { - backoff.Tick(2) + for i := 0; i < 20; i++ { + d := backoff.Tick(2) + z := mustGet(2).Threshold + fmt.Printf("%v %v\n", z, d) } - require.Equal(t, mustGet(2).Threshold, maxTick) + require.Equal(t, maxTick, mustGet(2).Threshold) } From 4d4e07beecab64e4e5fb06db2a53a04b638b4e7f Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Mon, 14 Feb 2022 11:35:08 +0800 Subject: [PATCH 03/18] add Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_api.go | 36 ++++++++++++++++++++++++++++-------- ddl/ddl_tiflash_test.go | 39 ++++++++++++++++++++++++++++++++------- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index eb3618037b7bd..1d9a2c4fe0ea3 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -84,14 +84,26 @@ type PollTiFlashBackoffContext struct { } // NewPollTiFlashBackoffContext creates an instance of PollTiFlashBackoffContext. -func NewPollTiFlashBackoffContext(MinTick, MaxTick TickType, Capacity int, Rate TickType) PollTiFlashBackoffContext { - return PollTiFlashBackoffContext{ +func NewPollTiFlashBackoffContext(MinTick, MaxTick TickType, Capacity int, Rate TickType) (*PollTiFlashBackoffContext, error) { + if MaxTick < MinTick { + return nil, fmt.Errorf("`MaxTick` should always be larger than `MinTick`") + } + if MinTick <= 1 { + return nil, fmt.Errorf("`MinTick` should always be larger than 1") + } + if Capacity < 0 { + return nil, fmt.Errorf("negative `Capacity`") + } + if Rate <= 1 { + return nil, fmt.Errorf("`Rate` shoule always larger than 1") + } + return &PollTiFlashBackoffContext{ MinTick: MinTick, MaxTick: MaxTick, Capacity: Capacity, elements: make(map[int64]*PollTiFlashBackoffElement), Rate: Rate, - } + }, nil } // TiFlashManagementContext is the context for TiFlash Replica Management @@ -100,7 +112,7 @@ type TiFlashManagementContext struct { HandlePdCounter uint64 UpdateTiFlashStoreCounter uint64 UpdateMap map[int64]bool - Backoff PollTiFlashBackoffContext + Backoff *PollTiFlashBackoffContext } // Tick will first check increase Counter. @@ -156,6 +168,7 @@ func (b *PollTiFlashBackoffContext) Remove(ID int64) { } // Get returns pointer to inner PollTiFlashBackoffElement. +// Only exported for test. func (b *PollTiFlashBackoffContext) Get(ID int64) (*PollTiFlashBackoffElement, bool) { res, ok := b.elements[ID] return res, ok @@ -177,14 +190,18 @@ func (b *PollTiFlashBackoffContext) Len() int { } // NewTiFlashManagementContext creates an instance for TiFlashManagementContext. -func NewTiFlashManagementContext() *TiFlashManagementContext { +func NewTiFlashManagementContext() (*TiFlashManagementContext, error) { + c, err := NewPollTiFlashBackoffContext(PollTiFlashBackoffMinTick, PollTiFlashBackoffMaxTick, PollTiFlashBackoffCapacity, PollTiFlashBackoffRate) + if err != nil { + return nil, err + } return &TiFlashManagementContext{ HandlePdCounter: 0, UpdateTiFlashStoreCounter: 0, TiFlashStores: make(map[int64]helper.StoreStat), UpdateMap: make(map[int64]bool), - Backoff: NewPollTiFlashBackoffContext(PollTiFlashBackoffMinTick, PollTiFlashBackoffMaxTick, PollTiFlashBackoffCapacity, PollTiFlashBackoffRate), - } + Backoff: c, + }, nil } var ( @@ -533,7 +550,10 @@ func HandlePlacementRuleRoutine(ctx sessionctx.Context, d *ddl, tableList []TiFl } func (d *ddl) PollTiFlashRoutine() { - pollTiflashContext := NewTiFlashManagementContext() + pollTiflashContext, err := NewTiFlashManagementContext() + if err != nil { + logutil.BgLogger().Fatal("TiFlashManagement init failed", zap.Error(err)) + } for { select { case <-d.ctx.Done(): diff --git a/ddl/ddl_tiflash_test.go b/ddl/ddl_tiflash_test.go index b1419dc9a5cc7..73db34af6634a 100644 --- a/ddl/ddl_tiflash_test.go +++ b/ddl/ddl_tiflash_test.go @@ -575,7 +575,9 @@ func (s *tiflashDDLTestSuite) TestSetPlacementRuleFail(c *C) { func TestTiFlashBackoff(t *testing.T) { var maxTick ddl.TickType = 10 var rate ddl.TickType = 1.5 - backoff := ddl.NewPollTiFlashBackoffContext(1, maxTick, 10, rate) + cap := 2 + backoff, err := ddl.NewPollTiFlashBackoffContext(1, maxTick, cap, rate) + require.NoError(t, err) backoff.Put(1) mustGet := func(ID int64) *ddl.PollTiFlashBackoffElement { e, ok := backoff.Get(ID) @@ -599,15 +601,38 @@ func TestTiFlashBackoff(t *testing.T) { require.Equal(t, e.Threshold, rate*ori) require.Equal(t, 1, e.Counter) } + // Test grow require.False(t, mustGet(1).NeedGrow()) - mustNotGrow(1) - mustGrow(1) - + mustNotGrow(1) // 0;1 -> 1;1 + mustGrow(1) // 1;1 -> 0;1.5 -> 1;1.5 + mustGrow(1) // 1;1.5 -> 0;2.25 -> 1;2.25 + mustNotGrow(1) // 1;2.25 -> 2;2.25 + mustGrow(1) // 2;2.25 -> 0;3.375 -> 1;3.375 + mustNotGrow(1) // 1;3.375 -> 2;3.375 + mustNotGrow(1) // 2;3.375 -> 3;3.375 + mustGrow(1) // 3;3.375 -> 0;5.0625 + + // Test converge backoff.Put(2) for i := 0; i < 20; i++ { - d := backoff.Tick(2) - z := mustGet(2).Threshold - fmt.Printf("%v %v\n", z, d) + backoff.Tick(2) } require.Equal(t, maxTick, mustGet(2).Threshold) + + // Test context + require.False(t, backoff.Put(3)) + backoff.Remove(1) + require.Equal(t, 1, backoff.Len()) + + // Test error context + _, err = ddl.NewPollTiFlashBackoffContext(0.5, 1, 1, 1) + require.Error(t, err) + _, err = ddl.NewPollTiFlashBackoffContext(10, 1, 1, 1) + require.Error(t, err) + _, err = ddl.NewPollTiFlashBackoffContext(1, 10, 0, 1) + require.Error(t, err) + _, err = ddl.NewPollTiFlashBackoffContext(1, 10, 1, 0.5) + require.Error(t, err) + _, err = ddl.NewPollTiFlashBackoffContext(1, 10, 1, -1) + require.Error(t, err) } From 3297a12f035392f6aa6264da4675eb7319797738 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Mon, 14 Feb 2022 13:52:47 +0800 Subject: [PATCH 04/18] fix Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_api.go | 30 ++++++++++++++++-------------- ddl/ddl_tiflash_test.go | 20 ++++++++++++++------ 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index 1d9a2c4fe0ea3..91687ec49d122 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -88,8 +88,8 @@ func NewPollTiFlashBackoffContext(MinTick, MaxTick TickType, Capacity int, Rate if MaxTick < MinTick { return nil, fmt.Errorf("`MaxTick` should always be larger than `MinTick`") } - if MinTick <= 1 { - return nil, fmt.Errorf("`MinTick` should always be larger than 1") + if MinTick < 1 { + return nil, fmt.Errorf("`MinTick` should not be less than 1") } if Capacity < 0 { return nil, fmt.Errorf("negative `Capacity`") @@ -116,24 +116,23 @@ type TiFlashManagementContext struct { } // Tick will first check increase Counter. -func (b *PollTiFlashBackoffContext) Tick(ID int64) bool { - e, ok := b.Get(ID) +func (b *PollTiFlashBackoffContext) Tick(ID int64) (bool, bool) { + e, ok := b.Put(ID) if !ok { - return false + return false, false } growed := e.MaybeGrow(b) e.Counter += 1 - return growed + return growed, true } -// Limit returns current limit -func (e *PollTiFlashBackoffElement) Limit() int { +func (e *PollTiFlashBackoffElement) limit() int { return int(e.Threshold) } // NeedGrow returns if we need to grow func (e *PollTiFlashBackoffElement) NeedGrow() bool { - if e.Counter >= e.Limit() { + if e.Counter >= e.limit() { return true } return false @@ -175,13 +174,16 @@ func (b *PollTiFlashBackoffContext) Get(ID int64) (*PollTiFlashBackoffElement, b } // Put will put table into backoff pool, if there are enough room, or returns false. -func (b *PollTiFlashBackoffContext) Put(ID int64) bool { - _, ok := b.elements[ID] - if ok || b.Len() < b.Capacity { +// Only exported for test. +func (b *PollTiFlashBackoffContext) Put(ID int64) (*PollTiFlashBackoffElement, bool) { + e, ok := b.elements[ID] + if ok { + return e, true + } else if b.Len() < b.Capacity { b.elements[ID] = NewPollTiFlashBackoffElement() - return true + return b.elements[ID], true } - return false + return nil, false } // Len gets size of PollTiFlashBackoffContext. diff --git a/ddl/ddl_tiflash_test.go b/ddl/ddl_tiflash_test.go index 73db34af6634a..cc0c77b078ecf 100644 --- a/ddl/ddl_tiflash_test.go +++ b/ddl/ddl_tiflash_test.go @@ -578,7 +578,6 @@ func TestTiFlashBackoff(t *testing.T) { cap := 2 backoff, err := ddl.NewPollTiFlashBackoffContext(1, maxTick, cap, rate) require.NoError(t, err) - backoff.Put(1) mustGet := func(ID int64) *ddl.PollTiFlashBackoffElement { e, ok := backoff.Get(ID) require.True(t, ok) @@ -588,20 +587,25 @@ func TestTiFlashBackoff(t *testing.T) { e := mustGet(ID) ori := e.Threshold c := e.Counter - growed := backoff.Tick(ID) + growed, ok := backoff.Tick(ID) + require.True(t, ok) require.False(t, growed) - require.Equal(t, e.Threshold, ori) - require.Equal(t, e.Counter, c+1) + require.Equal(t, ori, e.Threshold) + require.Equal(t, c+1, e.Counter) } mustGrow := func(ID int64) { e := mustGet(ID) ori := e.Threshold - growed := backoff.Tick(ID) + growed, ok := backoff.Tick(ID) + require.True(t, ok) require.True(t, growed) require.Equal(t, e.Threshold, rate*ori) require.Equal(t, 1, e.Counter) } // Test grow + z, ok := backoff.Put(1) + fmt.Printf("zzz %v\n", z) + require.True(t, ok) require.False(t, mustGet(1).NeedGrow()) mustNotGrow(1) // 0;1 -> 1;1 mustGrow(1) // 1;1 -> 0;1.5 -> 1;1.5 @@ -620,7 +624,11 @@ func TestTiFlashBackoff(t *testing.T) { require.Equal(t, maxTick, mustGet(2).Threshold) // Test context - require.False(t, backoff.Put(3)) + _, ok = backoff.Put(3) + require.False(t, ok) + _, ok = backoff.Tick(3) + require.False(t, ok) + backoff.Remove(1) require.Equal(t, 1, backoff.Len()) From 938325861d48c3693d1fe5924808a7647a2363a7 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Mon, 14 Feb 2022 14:09:57 +0800 Subject: [PATCH 05/18] fix Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_api.go | 13 +++++++++---- ddl/ddl_tiflash_test.go | 3 ++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index 91687ec49d122..bd3a3b29d0747 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -117,7 +117,7 @@ type TiFlashManagementContext struct { // Tick will first check increase Counter. func (b *PollTiFlashBackoffContext) Tick(ID int64) (bool, bool) { - e, ok := b.Put(ID) + e, ok := b.Get(ID) if !ok { return false, false } @@ -173,8 +173,7 @@ func (b *PollTiFlashBackoffContext) Get(ID int64) (*PollTiFlashBackoffElement, b return res, ok } -// Put will put table into backoff pool, if there are enough room, or returns false. -// Only exported for test. +// Put will record table into backoff pool, if there is enough room, or returns false. func (b *PollTiFlashBackoffContext) Put(ID int64) (*PollTiFlashBackoffElement, bool) { e, ok := b.elements[ID] if ok { @@ -220,7 +219,7 @@ var ( // PollTiFlashBackoffCapacity is the cache size of backoff struct. PollTiFlashBackoffCapacity int = 1000 // PollTiFlashBackoffRate is growth rate of exponential backoff threshold. - PollTiFlashBackoffRate TickType = 1000 + PollTiFlashBackoffRate TickType = 1.5 ) func getTiflashHTTPAddr(host string, statusAddr string) (string, error) { @@ -397,6 +396,10 @@ func (d *ddl) pollTiFlashReplicaStatus(ctx sessionctx.Context, pollTiFlashContex // We only check unavailable tables here, so doesn't include blocked add partition case. if !available { allReplicaReady = false + growed, inqueue := pollTiFlashContext.Backoff.Tick(tb.ID) + if inqueue && !growed { + logutil.BgLogger().Info("Escape checking available status due to backoff", zap.Int64("tableId", tb.ID)) + } // We don't need to set accelerate schedule for this table, since it is already done in DDL, when // 1. Add partition @@ -426,12 +429,14 @@ func (d *ddl) pollTiFlashReplicaStatus(ctx sessionctx.Context, pollTiFlashContex if !avail { logutil.BgLogger().Info("Tiflash replica is not available", zap.Int64("id", tb.ID), zap.Uint64("region need", uint64(regionCount)), zap.Uint64("region have", uint64(flashRegionCount))) + pollTiFlashContext.Backoff.Put(tb.ID) err := infosync.UpdateTiFlashTableSyncProgress(context.Background(), tb.ID, float64(flashRegionCount)/float64(regionCount)) if err != nil { return false, err } } else { logutil.BgLogger().Info("Tiflash replica is available", zap.Int64("id", tb.ID), zap.Uint64("region need", uint64(regionCount))) + pollTiFlashContext.Backoff.Remove(tb.ID) err := infosync.DeleteTiFlashTableSyncProgress(tb.ID) if err != nil { return false, err diff --git a/ddl/ddl_tiflash_test.go b/ddl/ddl_tiflash_test.go index cc0c77b078ecf..0d0301225f0c5 100644 --- a/ddl/ddl_tiflash_test.go +++ b/ddl/ddl_tiflash_test.go @@ -572,7 +572,8 @@ func (s *tiflashDDLTestSuite) TestSetPlacementRuleFail(c *C) { c.Assert(res, Equals, false) } -func TestTiFlashBackoff(t *testing.T) { +// Test standalone backoffer +func TestTiFlashBackoffer(t *testing.T) { var maxTick ddl.TickType = 10 var rate ddl.TickType = 1.5 cap := 2 From 0d06260eab0d9ec7c75b71129c54641982c76aa1 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Mon, 14 Feb 2022 15:57:21 +0800 Subject: [PATCH 06/18] fix Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_api.go | 20 ++++++++++--------- ddl/ddl_tiflash_test.go | 44 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index bd3a3b29d0747..4e49935c12954 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -56,13 +56,14 @@ type TiFlashReplicaStatus struct { HighPriority bool } -type TickType float64 +// TiFlashTick is type for backoff threshold. +type TiFlashTick float64 // PollTiFlashBackoffElement records backoff for each TiFlash Table. // When `Counter` reached `Threshold`, `Threshold` shall grow. type PollTiFlashBackoffElement struct { Counter int - Threshold TickType + Threshold TiFlashTick } // NewPollTiFlashBackoffElement initialize backoff element for a TiFlash table. @@ -75,16 +76,16 @@ func NewPollTiFlashBackoffElement() *PollTiFlashBackoffElement { // PollTiFlashBackoffContext is a collection of all backoff states. type PollTiFlashBackoffContext struct { - MinTick TickType - MaxTick TickType + MinTick TiFlashTick + MaxTick TiFlashTick // Capacity limits tables a backoff pool can handle, in order to limit handling of big tables. Capacity int - Rate TickType + Rate TiFlashTick elements map[int64]*PollTiFlashBackoffElement } // NewPollTiFlashBackoffContext creates an instance of PollTiFlashBackoffContext. -func NewPollTiFlashBackoffContext(MinTick, MaxTick TickType, Capacity int, Rate TickType) (*PollTiFlashBackoffContext, error) { +func NewPollTiFlashBackoffContext(MinTick, MaxTick TiFlashTick, Capacity int, Rate TiFlashTick) (*PollTiFlashBackoffContext, error) { if MaxTick < MinTick { return nil, fmt.Errorf("`MaxTick` should always be larger than `MinTick`") } @@ -213,13 +214,13 @@ var ( // UpdateTiFlashStoreTick indicates the number of intervals before we fully update TiFlash stores. UpdateTiFlashStoreTick = atomicutil.NewUint64(5) // PollTiFlashBackoffMaxTick is the max tick before we try to update TiFlash replica availability for one table. - PollTiFlashBackoffMaxTick TickType = 10 + PollTiFlashBackoffMaxTick TiFlashTick = 10 // PollTiFlashBackoffMinTick is the min tick before we try to update TiFlash replica availability for one table. - PollTiFlashBackoffMinTick TickType = 1 + PollTiFlashBackoffMinTick TiFlashTick = 1 // PollTiFlashBackoffCapacity is the cache size of backoff struct. PollTiFlashBackoffCapacity int = 1000 // PollTiFlashBackoffRate is growth rate of exponential backoff threshold. - PollTiFlashBackoffRate TickType = 1.5 + PollTiFlashBackoffRate TiFlashTick = 1.5 ) func getTiflashHTTPAddr(host string, statusAddr string) (string, error) { @@ -399,6 +400,7 @@ func (d *ddl) pollTiFlashReplicaStatus(ctx sessionctx.Context, pollTiFlashContex growed, inqueue := pollTiFlashContext.Backoff.Tick(tb.ID) if inqueue && !growed { logutil.BgLogger().Info("Escape checking available status due to backoff", zap.Int64("tableId", tb.ID)) + continue } // We don't need to set accelerate schedule for this table, since it is already done in DDL, when diff --git a/ddl/ddl_tiflash_test.go b/ddl/ddl_tiflash_test.go index c8ef29b522707..e36c5365ef70f 100644 --- a/ddl/ddl_tiflash_test.go +++ b/ddl/ddl_tiflash_test.go @@ -21,7 +21,6 @@ package ddl_test import ( "context" "fmt" - "github.com/stretchr/testify/require" "math" "testing" "time" @@ -599,8 +598,8 @@ func TestSetPlacementRuleFail(t *testing.T) { // Test standalone backoffer func TestTiFlashBackoffer(t *testing.T) { - var maxTick ddl.TickType = 10 - var rate ddl.TickType = 1.5 + var maxTick ddl.TiFlashTick = 10 + var rate ddl.TiFlashTick = 1.5 cap := 2 backoff, err := ddl.NewPollTiFlashBackoffContext(1, maxTick, cap, rate) require.NoError(t, err) @@ -670,3 +669,42 @@ func TestTiFlashBackoffer(t *testing.T) { _, err = ddl.NewPollTiFlashBackoffContext(1, 10, 1, -1) require.Error(t, err) } + +// Test backoffer in TiFlash. +func TestTiFlashBackoff(t *testing.T) { + s, teardown := createTiFlashContext(t) + defer teardown() + tk := testkit.NewTestKit(t, s.store) + + tk.MustExec("use test") + tk.MustExec("drop table if exists ddltiflash") + tk.MustExec("create table ddltiflash(z int)") + + // Not available for all tables + ddl.DisableTiFlashPoll(s.dom.DDL()) + require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplacePrevAvailableValue", `return(false)`)) + require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplaceCurAvailableValue", `return(false)`)) + ddl.EnableTiFlashPoll(s.dom.DDL()) + defer func() { + failpoint.Disable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplacePrevAvailableValue") + failpoint.Disable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplaceCurAvailableValue") + }() + tk.MustExec("alter table ddltiflash set tiflash replica 1") + + // 1, 1.5, 2.25, 3.375, 5.5625 + // (1), 1, 1, 2, 3, 5 + time.Sleep(ddl.PollTiFlashInterval * 5) + tb, err := s.dom.InfoSchema().TableByName(model.NewCIStr("test"), model.NewCIStr("ddltiflash")) + require.NoError(t, err) + require.NotNil(t, tb) + require.False(t, tb.Meta().TiFlashReplica.Available) + + failpoint.Disable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplacePrevAvailableValue") + failpoint.Disable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplaceCurAvailableValue") + + time.Sleep(ddl.PollTiFlashInterval * 3) + tb, err = s.dom.InfoSchema().TableByName(model.NewCIStr("test"), model.NewCIStr("ddltiflash")) + require.NoError(t, err) + require.NotNil(t, tb) + require.True(t, tb.Meta().TiFlashReplica.Available) +} From d055b5cc56d0bd877a3166d6538590e870798705 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Mon, 14 Feb 2022 16:33:48 +0800 Subject: [PATCH 07/18] change log Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_api.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index 4e49935c12954..3011378be294d 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -416,8 +416,6 @@ func (d *ddl) pollTiFlashReplicaStatus(ctx sessionctx.Context, pollTiFlashContex } } - logutil.BgLogger().Info("CollectTiFlashStatus", zap.Any("regionReplica", regionReplica), zap.Int64("tb.ID", tb.ID)) - var stats helper.PDRegionStats if err := infosync.GetTiFlashPDRegionRecordStats(context.Background(), tb.ID, &stats); err != nil { return allReplicaReady, err @@ -428,6 +426,7 @@ func (d *ddl) pollTiFlashReplicaStatus(ctx sessionctx.Context, pollTiFlashContex failpoint.Inject("PollTiFlashReplicaStatusReplaceCurAvailableValue", func(val failpoint.Value) { avail = val.(bool) }) + logutil.BgLogger().Info("CollectTiFlashStatus", zap.Int("regionCount", regionCount), zap.Int("flashRegionCount", flashRegionCount), zap.Int64("tb.ID", tb.ID)) if !avail { logutil.BgLogger().Info("Tiflash replica is not available", zap.Int64("id", tb.ID), zap.Uint64("region need", uint64(regionCount)), zap.Uint64("region have", uint64(flashRegionCount))) From bbf20c030af96d790d875f3771d35fd951c7d7f9 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Mon, 14 Feb 2022 18:20:36 +0800 Subject: [PATCH 08/18] fix Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_api.go | 2 +- domain/infosync/info.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index 3011378be294d..d09407427cc32 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -426,7 +426,7 @@ func (d *ddl) pollTiFlashReplicaStatus(ctx sessionctx.Context, pollTiFlashContex failpoint.Inject("PollTiFlashReplicaStatusReplaceCurAvailableValue", func(val failpoint.Value) { avail = val.(bool) }) - logutil.BgLogger().Info("CollectTiFlashStatus", zap.Int("regionCount", regionCount), zap.Int("flashRegionCount", flashRegionCount), zap.Int64("tb.ID", tb.ID)) + logutil.BgLogger().Info("CollectTiFlashStatus", zap.Int64("tb.ID", tb.ID), zap.Any("regionReplica", regionReplica)) if !avail { logutil.BgLogger().Info("Tiflash replica is not available", zap.Int64("id", tb.ID), zap.Uint64("region need", uint64(regionCount)), zap.Uint64("region have", uint64(flashRegionCount))) diff --git a/domain/infosync/info.go b/domain/infosync/info.go index 41a56c1f6a2c8..49bf8cf2eedb9 100644 --- a/domain/infosync/info.go +++ b/domain/infosync/info.go @@ -237,6 +237,7 @@ func initTiFlashPlacementManager(addrs []string) TiFlashPlacementManager { m := mockTiFlashPlacementManager{} return &m } + logutil.BgLogger().Warn("init TiFlashPlacementManager", zap.Strings("pd addrs", addrs)) return &TiFlashPDPlacementManager{addrs: addrs} } From b4abc79e4e400a53cd5ea38bd12cffada21e146d Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Tue, 15 Feb 2022 10:13:04 +0800 Subject: [PATCH 09/18] fix Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_api.go | 5 +---- ddl/ddl_tiflash_test.go | 4 ++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index d09407427cc32..feb8accef3bbc 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -133,10 +133,7 @@ func (e *PollTiFlashBackoffElement) limit() int { // NeedGrow returns if we need to grow func (e *PollTiFlashBackoffElement) NeedGrow() bool { - if e.Counter >= e.limit() { - return true - } - return false + return e.Counter >= e.limit() } func (e *PollTiFlashBackoffElement) doGrow(b *PollTiFlashBackoffContext) { diff --git a/ddl/ddl_tiflash_test.go b/ddl/ddl_tiflash_test.go index e36c5365ef70f..a3f2f4d3c4dda 100644 --- a/ddl/ddl_tiflash_test.go +++ b/ddl/ddl_tiflash_test.go @@ -600,8 +600,8 @@ func TestSetPlacementRuleFail(t *testing.T) { func TestTiFlashBackoffer(t *testing.T) { var maxTick ddl.TiFlashTick = 10 var rate ddl.TiFlashTick = 1.5 - cap := 2 - backoff, err := ddl.NewPollTiFlashBackoffContext(1, maxTick, cap, rate) + c := 2 + backoff, err := ddl.NewPollTiFlashBackoffContext(1, maxTick, c, rate) require.NoError(t, err) mustGet := func(ID int64) *ddl.PollTiFlashBackoffElement { e, ok := backoff.Get(ID) From 946d7d0d3afcbb7d123aded995cfefa158e7f54f Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Wed, 16 Feb 2022 11:32:21 +0800 Subject: [PATCH 10/18] fix Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_api.go | 4 +++- ddl/ddl_tiflash_test.go | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index feb8accef3bbc..553c33ddcdee9 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -160,8 +160,10 @@ func (e *PollTiFlashBackoffElement) MaybeGrow(b *PollTiFlashBackoffContext) bool } // Remove will reset table from backoff -func (b *PollTiFlashBackoffContext) Remove(ID int64) { +func (b *PollTiFlashBackoffContext) Remove(ID int64) bool { + _, ok := b.elements[ID] delete(b.elements, ID) + return ok } // Get returns pointer to inner PollTiFlashBackoffElement. diff --git a/ddl/ddl_tiflash_test.go b/ddl/ddl_tiflash_test.go index a3f2f4d3c4dda..86acbe1b7c2b4 100644 --- a/ddl/ddl_tiflash_test.go +++ b/ddl/ddl_tiflash_test.go @@ -628,8 +628,7 @@ func TestTiFlashBackoffer(t *testing.T) { require.Equal(t, 1, e.Counter) } // Test grow - z, ok := backoff.Put(1) - fmt.Printf("zzz %v\n", z) + _, ok := backoff.Put(1) require.True(t, ok) require.False(t, mustGet(1).NeedGrow()) mustNotGrow(1) // 0;1 -> 1;1 @@ -654,7 +653,8 @@ func TestTiFlashBackoffer(t *testing.T) { _, ok = backoff.Tick(3) require.False(t, ok) - backoff.Remove(1) + require.True(t, backoff.Remove(1)) + require.False(t, backoff.Remove(1)) require.Equal(t, 1, backoff.Len()) // Test error context From ef99368c6ea277af05e7d772c826b440e88c33e4 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Wed, 16 Feb 2022 15:31:52 +0800 Subject: [PATCH 11/18] add Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_api.go | 19 +++++++++++-------- ddl/ddl_tiflash_test.go | 18 ++++++++++++------ 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index 553c33ddcdee9..85a23e302dce7 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -62,15 +62,17 @@ type TiFlashTick float64 // PollTiFlashBackoffElement records backoff for each TiFlash Table. // When `Counter` reached `Threshold`, `Threshold` shall grow. type PollTiFlashBackoffElement struct { - Counter int - Threshold TiFlashTick + Counter int + Threshold TiFlashTick + TotalCounter int } // NewPollTiFlashBackoffElement initialize backoff element for a TiFlash table. func NewPollTiFlashBackoffElement() *PollTiFlashBackoffElement { return &PollTiFlashBackoffElement{ - Counter: 0, - Threshold: PollTiFlashBackoffMinTick, + Counter: 0, + Threshold: PollTiFlashBackoffMinTick, + TotalCounter: 0, } } @@ -117,14 +119,15 @@ type TiFlashManagementContext struct { } // Tick will first check increase Counter. -func (b *PollTiFlashBackoffContext) Tick(ID int64) (bool, bool) { +func (b *PollTiFlashBackoffContext) Tick(ID int64) (bool, bool, int) { e, ok := b.Get(ID) if !ok { - return false, false + return false, false, 0 } growed := e.MaybeGrow(b) e.Counter += 1 - return growed, true + e.TotalCounter += 1 + return growed, true, e.TotalCounter } func (e *PollTiFlashBackoffElement) limit() int { @@ -396,7 +399,7 @@ func (d *ddl) pollTiFlashReplicaStatus(ctx sessionctx.Context, pollTiFlashContex // We only check unavailable tables here, so doesn't include blocked add partition case. if !available { allReplicaReady = false - growed, inqueue := pollTiFlashContext.Backoff.Tick(tb.ID) + growed, inqueue, _ := pollTiFlashContext.Backoff.Tick(tb.ID) if inqueue && !growed { logutil.BgLogger().Info("Escape checking available status due to backoff", zap.Int64("tableId", tb.ID)) continue diff --git a/ddl/ddl_tiflash_test.go b/ddl/ddl_tiflash_test.go index 86acbe1b7c2b4..31273264b3f9a 100644 --- a/ddl/ddl_tiflash_test.go +++ b/ddl/ddl_tiflash_test.go @@ -611,21 +611,25 @@ func TestTiFlashBackoffer(t *testing.T) { mustNotGrow := func(ID int64) { e := mustGet(ID) ori := e.Threshold + oriTotal := e.TotalCounter c := e.Counter - growed, ok := backoff.Tick(ID) + growed, ok, total := backoff.Tick(ID) require.True(t, ok) require.False(t, growed) require.Equal(t, ori, e.Threshold) require.Equal(t, c+1, e.Counter) + require.Equal(t, oriTotal+1, total) } mustGrow := func(ID int64) { e := mustGet(ID) ori := e.Threshold - growed, ok := backoff.Tick(ID) + oriTotal := e.TotalCounter + growed, ok, total := backoff.Tick(ID) require.True(t, ok) require.True(t, growed) require.Equal(t, e.Threshold, rate*ori) require.Equal(t, 1, e.Counter) + require.Equal(t, oriTotal+1, total) } // Test grow _, ok := backoff.Put(1) @@ -639,6 +643,7 @@ func TestTiFlashBackoffer(t *testing.T) { mustNotGrow(1) // 1;3.375 -> 2;3.375 mustNotGrow(1) // 2;3.375 -> 3;3.375 mustGrow(1) // 3;3.375 -> 0;5.0625 + require.Equal(t, 8, mustGet(1).TotalCounter) // Test converge backoff.Put(2) @@ -646,6 +651,7 @@ func TestTiFlashBackoffer(t *testing.T) { backoff.Tick(2) } require.Equal(t, maxTick, mustGet(2).Threshold) + require.Equal(t, 20, mustGet(2).TotalCounter) // Test context _, ok = backoff.Put(3) @@ -686,8 +692,8 @@ func TestTiFlashBackoff(t *testing.T) { require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplaceCurAvailableValue", `return(false)`)) ddl.EnableTiFlashPoll(s.dom.DDL()) defer func() { - failpoint.Disable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplacePrevAvailableValue") - failpoint.Disable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplaceCurAvailableValue") + require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplacePrevAvailableValue")) + require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplaceCurAvailableValue")) }() tk.MustExec("alter table ddltiflash set tiflash replica 1") @@ -699,8 +705,8 @@ func TestTiFlashBackoff(t *testing.T) { require.NotNil(t, tb) require.False(t, tb.Meta().TiFlashReplica.Available) - failpoint.Disable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplacePrevAvailableValue") - failpoint.Disable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplaceCurAvailableValue") + require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplacePrevAvailableValue")) + require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplaceCurAvailableValue")) time.Sleep(ddl.PollTiFlashInterval * 3) tb, err = s.dom.InfoSchema().TableByName(model.NewCIStr("test"), model.NewCIStr("ddltiflash")) From 24571b6ff78f64bc12bb3d96a6ed8443d236066f Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Wed, 16 Feb 2022 16:23:55 +0800 Subject: [PATCH 12/18] fix Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/ddl_tiflash_test.go b/ddl/ddl_tiflash_test.go index 31273264b3f9a..fecf9e175cf8f 100644 --- a/ddl/ddl_tiflash_test.go +++ b/ddl/ddl_tiflash_test.go @@ -656,7 +656,7 @@ func TestTiFlashBackoffer(t *testing.T) { // Test context _, ok = backoff.Put(3) require.False(t, ok) - _, ok = backoff.Tick(3) + _, ok, _ = backoff.Tick(3) require.False(t, ok) require.True(t, backoff.Remove(1)) From 3915ae90b12a37e0cf4252906e76b572ccf119b7 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Fri, 18 Feb 2022 17:01:18 +0800 Subject: [PATCH 13/18] typo Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index 85a23e302dce7..f7d0d9cdf0ad6 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -98,7 +98,7 @@ func NewPollTiFlashBackoffContext(MinTick, MaxTick TiFlashTick, Capacity int, Ra return nil, fmt.Errorf("negative `Capacity`") } if Rate <= 1 { - return nil, fmt.Errorf("`Rate` shoule always larger than 1") + return nil, fmt.Errorf("`Rate` should always be larger than 1") } return &PollTiFlashBackoffContext{ MinTick: MinTick, From 4ab19452e9685fdecd7a2b0eb6a98b94c80be109 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Fri, 18 Feb 2022 17:04:15 +0800 Subject: [PATCH 14/18] adapt master Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_api.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index 3822909826cb6..964032e0f2b62 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -419,6 +419,8 @@ func (d *ddl) pollTiFlashReplicaStatus(ctx sessionctx.Context, pollTiFlashContex } } + logutil.BgLogger().Debug("CollectTiFlashStatus", zap.Any("regionReplica", regionReplica), zap.Int64("tableID", tb.ID)) + var stats helper.PDRegionStats if err := infosync.GetTiFlashPDRegionRecordStats(context.Background(), tb.ID, &stats); err != nil { return allReplicaReady, err @@ -429,7 +431,6 @@ func (d *ddl) pollTiFlashReplicaStatus(ctx sessionctx.Context, pollTiFlashContex failpoint.Inject("PollTiFlashReplicaStatusReplaceCurAvailableValue", func(val failpoint.Value) { avail = val.(bool) }) - logutil.BgLogger().Info("CollectTiFlashStatus", zap.Int64("tb.ID", tb.ID), zap.Any("regionReplica", regionReplica)) if !avail { logutil.BgLogger().Info("Tiflash replica is not available", zap.Int64("tableID", tb.ID), zap.Uint64("region need", uint64(regionCount)), zap.Uint64("region have", uint64(flashRegionCount))) From 61230bd7a830a5fd602d39e8617d9100d68d2813 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Wed, 23 Feb 2022 11:37:34 +0800 Subject: [PATCH 15/18] fix Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/ddl/ddl_tiflash_test.go b/ddl/ddl_tiflash_test.go index b150295367141..6de7714e37b0d 100644 --- a/ddl/ddl_tiflash_test.go +++ b/ddl/ddl_tiflash_test.go @@ -596,7 +596,6 @@ func TestSetPlacementRuleFail(t *testing.T) { require.False(t, res) } -<<<<<<< HEAD // Test standalone backoffer func TestTiFlashBackoffer(t *testing.T) { var maxTick ddl.TiFlashTick = 10 From 34942acce8e48efb756d1594fc4c2da335ca8d4d Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Wed, 23 Feb 2022 20:33:58 +0800 Subject: [PATCH 16/18] fix Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ddl/ddl_tiflash_test.go b/ddl/ddl_tiflash_test.go index 6de7714e37b0d..18e7035e54947 100644 --- a/ddl/ddl_tiflash_test.go +++ b/ddl/ddl_tiflash_test.go @@ -691,10 +691,6 @@ func TestTiFlashBackoff(t *testing.T) { require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplacePrevAvailableValue", `return(false)`)) require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplaceCurAvailableValue", `return(false)`)) ddl.EnableTiFlashPoll(s.dom.DDL()) - defer func() { - require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplacePrevAvailableValue")) - require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/ddl/PollTiFlashReplicaStatusReplaceCurAvailableValue")) - }() tk.MustExec("alter table ddltiflash set tiflash replica 1") // 1, 1.5, 2.25, 3.375, 5.5625 From c1e2e52c5c483ffc9c26f96bdac18fe27de49e32 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Thu, 3 Mar 2022 16:58:59 +0800 Subject: [PATCH 17/18] addr review Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_api.go | 42 ++++++++++++++++++++--------------------- ddl/ddl_tiflash_test.go | 4 ++-- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index 964032e0f2b62..0345368f48b49 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -61,7 +61,8 @@ type TiFlashReplicaStatus struct { type TiFlashTick float64 // PollTiFlashBackoffElement records backoff for each TiFlash Table. -// When `Counter` reached `Threshold`, `Threshold` shall grow. +// `Counter` increases every `Tick`, if it reached `Threshold`, it will be reset to 0 while `Threshold` grows. +// `TotalCounter` records total `Tick`s this element has since created. type PollTiFlashBackoffElement struct { Counter int Threshold TiFlashTick @@ -120,30 +121,28 @@ type TiFlashManagementContext struct { } // Tick will first check increase Counter. +// It returns: +// 1. A bool indicates whether threshold is grown during this tick. +// 2. A bool indicates whether this ID exists. +// 3. A int indicates how many ticks ID has counted till now. func (b *PollTiFlashBackoffContext) Tick(ID int64) (bool, bool, int) { e, ok := b.Get(ID) if !ok { return false, false, 0 } - growed := e.MaybeGrow(b) + grew := e.MaybeGrow(b) e.Counter += 1 e.TotalCounter += 1 - return growed, true, e.TotalCounter + return grew, true, e.TotalCounter } -func (e *PollTiFlashBackoffElement) limit() int { - return int(e.Threshold) -} - -// NeedGrow returns if we need to grow +// NeedGrow returns if we need to grow. +// It is exported for testing. func (e *PollTiFlashBackoffElement) NeedGrow() bool { - return e.Counter >= e.limit() + return e.Counter >= int(e.Threshold) } func (e *PollTiFlashBackoffElement) doGrow(b *PollTiFlashBackoffContext) { - defer func() { - e.Counter = 0 - }() if e.Threshold < b.MinTick { e.Threshold = b.MinTick } @@ -152,9 +151,10 @@ func (e *PollTiFlashBackoffElement) doGrow(b *PollTiFlashBackoffContext) { } else { e.Threshold *= b.Rate } + e.Counter = 0 } -// MaybeGrow grows threshold and reset counter when need +// MaybeGrow grows threshold and reset counter when needed. func (e *PollTiFlashBackoffElement) MaybeGrow(b *PollTiFlashBackoffContext) bool { if !e.NeedGrow() { return false @@ -163,7 +163,7 @@ func (e *PollTiFlashBackoffElement) MaybeGrow(b *PollTiFlashBackoffContext) bool return true } -// Remove will reset table from backoff +// Remove will reset table from backoff. func (b *PollTiFlashBackoffContext) Remove(ID int64) bool { _, ok := b.elements[ID] delete(b.elements, ID) @@ -178,15 +178,15 @@ func (b *PollTiFlashBackoffContext) Get(ID int64) (*PollTiFlashBackoffElement, b } // Put will record table into backoff pool, if there is enough room, or returns false. -func (b *PollTiFlashBackoffContext) Put(ID int64) (*PollTiFlashBackoffElement, bool) { - e, ok := b.elements[ID] +func (b *PollTiFlashBackoffContext) Put(ID int64) bool { + _, ok := b.elements[ID] if ok { - return e, true + return true } else if b.Len() < b.Capacity { b.elements[ID] = NewPollTiFlashBackoffElement() - return b.elements[ID], true + return true } - return nil, false + return false } // Len gets size of PollTiFlashBackoffContext. @@ -400,8 +400,8 @@ func (d *ddl) pollTiFlashReplicaStatus(ctx sessionctx.Context, pollTiFlashContex // We only check unavailable tables here, so doesn't include blocked add partition case. if !available { allReplicaReady = false - growed, inqueue, _ := pollTiFlashContext.Backoff.Tick(tb.ID) - if inqueue && !growed { + grown, inqueue, _ := pollTiFlashContext.Backoff.Tick(tb.ID) + if inqueue && !grown { logutil.BgLogger().Info("Escape checking available status due to backoff", zap.Int64("tableId", tb.ID)) continue } diff --git a/ddl/ddl_tiflash_test.go b/ddl/ddl_tiflash_test.go index 18e7035e54947..de824cdb03c4b 100644 --- a/ddl/ddl_tiflash_test.go +++ b/ddl/ddl_tiflash_test.go @@ -632,7 +632,7 @@ func TestTiFlashBackoffer(t *testing.T) { require.Equal(t, oriTotal+1, total) } // Test grow - _, ok := backoff.Put(1) + ok := backoff.Put(1) require.True(t, ok) require.False(t, mustGet(1).NeedGrow()) mustNotGrow(1) // 0;1 -> 1;1 @@ -654,7 +654,7 @@ func TestTiFlashBackoffer(t *testing.T) { require.Equal(t, 20, mustGet(2).TotalCounter) // Test context - _, ok = backoff.Put(3) + ok = backoff.Put(3) require.False(t, ok) _, ok, _ = backoff.Tick(3) require.False(t, ok) From 97914714dbf37b48ba252d9734d8e30286283966 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Thu, 3 Mar 2022 17:53:55 +0800 Subject: [PATCH 18/18] fix Signed-off-by: CalvinNeo --- ddl/ddl_tiflash_api.go | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index 0345368f48b49..f17723e948f1e 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -80,8 +80,8 @@ func NewPollTiFlashBackoffElement() *PollTiFlashBackoffElement { // PollTiFlashBackoffContext is a collection of all backoff states. type PollTiFlashBackoffContext struct { - MinTick TiFlashTick - MaxTick TiFlashTick + MinThreshold TiFlashTick + MaxThreshold TiFlashTick // Capacity limits tables a backoff pool can handle, in order to limit handling of big tables. Capacity int Rate TiFlashTick @@ -89,12 +89,12 @@ type PollTiFlashBackoffContext struct { } // NewPollTiFlashBackoffContext creates an instance of PollTiFlashBackoffContext. -func NewPollTiFlashBackoffContext(MinTick, MaxTick TiFlashTick, Capacity int, Rate TiFlashTick) (*PollTiFlashBackoffContext, error) { - if MaxTick < MinTick { - return nil, fmt.Errorf("`MaxTick` should always be larger than `MinTick`") +func NewPollTiFlashBackoffContext(MinThreshold, MaxThreshold TiFlashTick, Capacity int, Rate TiFlashTick) (*PollTiFlashBackoffContext, error) { + if MaxThreshold < MinThreshold { + return nil, fmt.Errorf("`MaxThreshold` should always be larger than `MinThreshold`") } - if MinTick < 1 { - return nil, fmt.Errorf("`MinTick` should not be less than 1") + if MinThreshold < 1 { + return nil, fmt.Errorf("`MinThreshold` should not be less than 1") } if Capacity < 0 { return nil, fmt.Errorf("negative `Capacity`") @@ -103,11 +103,11 @@ func NewPollTiFlashBackoffContext(MinTick, MaxTick TiFlashTick, Capacity int, Ra return nil, fmt.Errorf("`Rate` should always be larger than 1") } return &PollTiFlashBackoffContext{ - MinTick: MinTick, - MaxTick: MaxTick, - Capacity: Capacity, - elements: make(map[int64]*PollTiFlashBackoffElement), - Rate: Rate, + MinThreshold: MinThreshold, + MaxThreshold: MaxThreshold, + Capacity: Capacity, + elements: make(map[int64]*PollTiFlashBackoffElement), + Rate: Rate, }, nil } @@ -143,11 +143,11 @@ func (e *PollTiFlashBackoffElement) NeedGrow() bool { } func (e *PollTiFlashBackoffElement) doGrow(b *PollTiFlashBackoffContext) { - if e.Threshold < b.MinTick { - e.Threshold = b.MinTick + if e.Threshold < b.MinThreshold { + e.Threshold = b.MinThreshold } - if e.Threshold*b.Rate > b.MaxTick { - e.Threshold = b.MaxTick + if e.Threshold*b.Rate > b.MaxThreshold { + e.Threshold = b.MaxThreshold } else { e.Threshold *= b.Rate } @@ -400,8 +400,8 @@ func (d *ddl) pollTiFlashReplicaStatus(ctx sessionctx.Context, pollTiFlashContex // We only check unavailable tables here, so doesn't include blocked add partition case. if !available { allReplicaReady = false - grown, inqueue, _ := pollTiFlashContext.Backoff.Tick(tb.ID) - if inqueue && !grown { + enabled, inqueue, _ := pollTiFlashContext.Backoff.Tick(tb.ID) + if inqueue && !enabled { logutil.BgLogger().Info("Escape checking available status due to backoff", zap.Int64("tableId", tb.ID)) continue }