Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions pkg/embed/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ type fileServiceCloser interface {
const (
defaultHAKeeperRunningTimeout = 2 * time.Minute
testingHAKeeperRunningTimeout = 5 * time.Minute
clusterConditionCheckInterval = 100 * time.Millisecond
)

func newService(
Expand Down Expand Up @@ -456,6 +457,7 @@ func (op *operator) waitHAKeeperRunning(
client logservice.CNHAKeeperClient,
) error {
// wait HAKeeper running
lastLogTime := time.Now().Add(-time.Second)
for {
state, err := client.GetClusterState(ctx)
if errors.Is(err, context.DeadlineExceeded) {
Expand All @@ -464,8 +466,11 @@ func (op *operator) waitHAKeeperRunning(
if moerr.IsMoErrCode(err, moerr.ErrNoHAKeeper) ||
state.State != logpb.HAKeeperRunning {
// not ready
op.reset.logger.Info("hakeeper not ready, retry")
if err := waitStartupRetry(ctx, op.cfg.HAKeeperRunningRetryInterval.Duration); err != nil {
if time.Since(lastLogTime) >= time.Second {
op.reset.logger.Info("hakeeper not ready, retry")
lastLogTime = time.Now()
}
if err := waitStartupRetry(ctx, op.clusterConditionCheckInterval()); err != nil {
return err
}
continue
Expand All @@ -481,6 +486,10 @@ func (op *operator) hakeeperRunningTimeout() time.Duration {
return defaultHAKeeperRunningTimeout
}

func (op *operator) clusterConditionCheckInterval() time.Duration {
return clusterConditionCheckInterval
}

func (op *operator) waitAnyShardReadyLocked(client logservice.CNHAKeeperClient) error {
ctx, cancel := context.WithTimeoutCause(context.TODO(), time.Second*30, moerr.CauseWaitAnyShardReadyLocked)
defer cancel()
Expand All @@ -489,6 +498,7 @@ func (op *operator) waitAnyShardReadyLocked(client logservice.CNHAKeeperClient)

func (op *operator) waitAnyShardReady(ctx context.Context, client logservice.CNHAKeeperClient) error {
// wait shard ready
lastLogTime := time.Now().Add(-time.Second)
for {
if ok, err := func() (bool, error) {
details, err := client.GetClusterDetails(ctx)
Expand All @@ -512,15 +522,18 @@ func (op *operator) waitAnyShardReady(ctx context.Context, client logservice.CNH
return true, nil
}
}
op.reset.logger.Info("shard not ready")
if time.Since(lastLogTime) >= time.Second {
op.reset.logger.Info("shard not ready")
lastLogTime = time.Now()
}
return false, nil
}(); err != nil {
return err
} else if ok {
op.reset.logger.Info("shard ready")
return nil
}
if err := waitStartupRetry(ctx, op.cfg.TNShardReadyRetryInterval.Duration); err != nil {
if err := waitStartupRetry(ctx, op.clusterConditionCheckInterval()); err != nil {
return err
}
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/embed/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,12 @@ func TestHAKeeperRunningTimeout(t *testing.T) {
assert.Equal(t, 5*time.Minute, (&operator{testing: true}).hakeeperRunningTimeout())
}

func TestClusterConditionCheckInterval(t *testing.T) {
interval := (&operator{}).clusterConditionCheckInterval()
assert.Greater(t, interval, time.Duration(0))
assert.Less(t, interval, time.Second)
}

func TestWaitClusterConditionClosesHAKeeperClient(t *testing.T) {
waitErr := errors.New("wait failed")
closeErr := errors.New("close failed")
Expand Down
3 changes: 3 additions & 0 deletions pkg/logservice/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ type Service struct {
stopper *stopper.Stopper
haClient LogHAKeeperClient
fileService fileservice.FileService
heartbeatC chan struct{}
shutdownC chan struct{}
Comment on lines 74 to 78

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Updated the PR title to fix: speed up hakeeper bootstrap and adjusted the body to describe the runtime bootstrap responsiveness and steady-state interval behavior.


options struct {
Expand Down Expand Up @@ -116,6 +117,7 @@ func NewService(
cfg: cfg,
stopper: stopper.NewStopper("log-service"),
fileService: fileService,
heartbeatC: make(chan struct{}, 1),
shutdownC: shutdownC,
}
for _, opt := range opts {
Expand All @@ -136,6 +138,7 @@ func NewService(
service.runtime.Logger().Error("failed to create log store", zap.Error(err))
return nil, err
}
store.bootstrapCommandsAdded = service.requestHeartbeat
if err := store.loadMetadata(); err != nil {
_ = store.close()
return nil, err
Expand Down
11 changes: 11 additions & 0 deletions pkg/logservice/service_bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,16 @@ func (s *Service) BootstrapHAKeeper(ctx context.Context, cfg Config) error {
return nil
default:
}
ready, err := s.store.waitHAKeeperLeaderReady(ctx, hakeeperDefaultTimeout)
if err != nil {
if restoreConfigured {
return err
}
return nil
}
if !ready {
continue
}
s.runtime.SubLogger(runtime.SystemInit).Info("before initial cluster info")
applied, err := s.store.setInitialClusterInfoWithRecoveryResult(
numOfLogShards,
Expand Down Expand Up @@ -184,6 +194,7 @@ func (s *Service) BootstrapHAKeeper(ctx context.Context, cfg Config) error {
initialClusterProposed = true
s.runtime.SubLogger(runtime.SystemInit).Info("initial cluster info set",
zap.Bool("applied", applied))
s.requestHeartbeat()
break
}
if backup != nil {
Expand Down
12 changes: 12 additions & 0 deletions pkg/logservice/service_commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,8 @@ func (s *Service) heartbeatWorker(ctx context.Context) {
select {
case <-ctx.Done():
return
case <-s.heartbeatC:
s.heartbeat(ctx)
case <-ticker.C:
s.heartbeat(ctx)
// I'd call this an ugly hack to just workaround select's
Expand All @@ -195,6 +197,16 @@ func (s *Service) heartbeatWorker(ctx context.Context) {
}
}

func (s *Service) requestHeartbeat() {
if s.heartbeatC == nil {
return
}
select {
case s.heartbeatC <- struct{}{}:
default:
}
}

func (s *Service) checkReplicaHealth(ctx context.Context) {
details, err := s.store.getClusterDetails(ctx)
if err != nil {
Expand Down
42 changes: 36 additions & 6 deletions pkg/logservice/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,10 @@ type store struct {
tickerStopper *stopper.Stopper
runtime runtime.Runtime

bootstrapCheckCycles uint64
bootstrapMgr *bootstrap.Manager
bootstrapCheckCycles uint64
bootstrapMgr *bootstrap.Manager
lastBootstrapLogTime time.Time
bootstrapCommandsAdded func()

taskScheduler hakeeper.TaskScheduler

Expand Down Expand Up @@ -1230,8 +1232,8 @@ func (l *store) ticker(ctx context.Context) {
defer func() {
l.runtime.Logger().Info("HAKeeper ticker stopped")
}()
haTicker := time.NewTicker(l.cfg.HAKeeperCheckInterval.Duration)
defer haTicker.Stop()
haTimer := time.NewTimer(l.initialHAKeeperCheckInterval())
defer haTimer.Stop()

// moving task schedule from the ticker normal routine to a
// separate goroutine can avoid the hakeeper's health check and tick update
Expand All @@ -1245,8 +1247,9 @@ func (l *store) ticker(ctx context.Context) {
select {
case <-ticker.C:
l.hakeeperTick()
case <-haTicker.C:
l.hakeeperCheck()
case <-haTimer.C:
state := l.hakeeperCheck()
haTimer.Reset(l.nextHAKeeperCheckInterval(state))
case <-ctx.Done():
return
}
Expand All @@ -1268,6 +1271,33 @@ func (l *store) isLeaderHAKeeper() (bool, uint64, error) {
return ok && replicaID != 0 && leaderID == replicaID, term, nil
}

func (l *store) waitHAKeeperLeaderReady(ctx context.Context, maxWait time.Duration) (bool, error) {
if leaderID, _, ok, err := l.nh.GetLeaderID(hakeeper.DefaultHAKeeperShardID); err == nil && ok && leaderID != 0 {
return true, nil
}
if maxWait <= 0 {
return false, nil
}

ticker := time.NewTicker(time.Millisecond * 20)
defer ticker.Stop()
timer := time.NewTimer(maxWait)
defer timer.Stop()
for {
leaderID, _, ok, err := l.nh.GetLeaderID(hakeeper.DefaultHAKeeperShardID)
if err == nil && ok && leaderID != 0 {
return true, nil
}
select {
case <-ctx.Done():
return false, moerr.AttachCause(ctx, ctx.Err())
case <-timer.C:
return false, nil
case <-ticker.C:
}
}
}

// TODO: add test for this
func (l *store) hakeeperTick() {
isLeader, _, err := l.isLeaderHAKeeper()
Expand Down
53 changes: 46 additions & 7 deletions pkg/logservice/store_hakeeper_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"os"
"strings"
"sync/atomic"
"time"

Expand All @@ -31,9 +32,10 @@ import (
)

const (
minIDAllocCapacity uint64 = 1024
defaultIDBatchSize uint64 = 1024 * 10
checkBootstrapCycles = 100
minIDAllocCapacity uint64 = 1024
defaultIDBatchSize uint64 = 1024 * 10
checkBootstrapCycles = 100
bootstrapHAKeeperCheckInterval = 100 * time.Millisecond
)

var (
Expand Down Expand Up @@ -252,16 +254,37 @@ func (l *store) getCheckerStateFromLeader() (*pb.CheckerState, uint64) {

var debugPrintHAKeeperState atomic.Bool

func (l *store) hakeeperCheck() {
func (l *store) initialHAKeeperCheckInterval() time.Duration {
interval := l.cfg.HAKeeperCheckInterval.Duration
if interval > bootstrapHAKeeperCheckInterval {
return bootstrapHAKeeperCheckInterval
}
return interval
}

func (l *store) nextHAKeeperCheckInterval(state *pb.CheckerState) time.Duration {
interval := l.cfg.HAKeeperCheckInterval.Duration
if state != nil && state.State != pb.HAKeeperRunning {
if interval > bootstrapHAKeeperCheckInterval {
return bootstrapHAKeeperCheckInterval
}
}
return interval
}
Comment on lines +265 to +273

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 33cdfa7. nil checker state now keeps the configured HAKeeper check interval, so non-leader LogServices do not stay on the 100ms bootstrap cadence. Verified with GOPROXY=https://goproxy.cn,direct .agents/skills/mo-dev/scripts/mo-cgo-test -v -run '^(TestServiceBootstrapRestoresHAKeeperAndWAL|TestSetInitialClusterInfo|TestNextHAKeeperCheckIntervalUsesFastBootstrapInterval|TestBootstrap|TestFailedBootstrap)$' -count=1 -timeout=180s ./pkg/logservice.


func (l *store) hakeeperCheck() *pb.CheckerState {
state, term := l.getCheckerStateFromLeader()
if state == nil {
return
return nil
}

switch state.State {
case pb.HAKeeperCreated:
l.runtime.Logger().Warn("waiting for initial cluster info to be set, check skipped")
return
if time.Since(l.lastBootstrapLogTime) >= time.Second {
l.runtime.Logger().Warn("waiting for initial cluster info to be set, check skipped")
l.lastBootstrapLogTime = time.Now()
}
return state
Comment on lines 282 to +287

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 33cdfa7. The HAKeeperCreated warning is now throttled to once per second using the existing bootstrap log timestamp. Verified with GOPROXY=https://goproxy.cn,direct .agents/skills/mo-dev/scripts/mo-cgo-test -v -run '^(TestServiceBootstrapRestoresHAKeeperAndWAL|TestSetInitialClusterInfo|TestNextHAKeeperCheckIntervalUsesFastBootstrapInterval|TestBootstrap|TestFailedBootstrap)$' -count=1 -timeout=180s ./pkg/logservice.

case pb.HAKeeperBootstrapping:
l.bootstrap(term, state)
case pb.HAKeeperBootstrapCommandsReceived:
Expand All @@ -277,6 +300,7 @@ func (l *store) hakeeperCheck() {
default:
panic("unknown HAKeeper state")
}
return state
}

func (l *store) assertHAKeeperState(s pb.HAKeeperState) {
Expand Down Expand Up @@ -358,6 +382,13 @@ func (l *store) bootstrap(term uint64, state *pb.CheckerState) {
}
cmds, err := l.getScheduleCommand(false, term, state)
if err != nil {
if isBootstrapWaitingForLogStores(err) {
if time.Since(l.lastBootstrapLogTime) >= time.Second {
l.runtime.Logger().Info("waiting for log stores before bootstrap", zap.Error(err))
l.lastBootstrapLogTime = time.Now()
}
return
}
l.runtime.Logger().Error("failed to get bootstrap schedule commands", zap.Error(err))
return
}
Expand Down Expand Up @@ -398,9 +429,17 @@ func (l *store) bootstrap(term uint64, state *pb.CheckerState) {
l.bootstrapCheckCycles = checkBootstrapCycles
l.bootstrapMgr = bootstrap.NewBootstrapManager(state.ClusterInfo)
l.assertHAKeeperState(pb.HAKeeperBootstrapCommandsReceived)
if l.bootstrapCommandsAdded != nil {
l.bootstrapCommandsAdded()
}
}
}

func isBootstrapWaitingForLogStores(err error) bool {
return moerr.IsMoErrCode(err, moerr.ErrInternal) &&
strings.Contains(err.Error(), "not enough log stores")
}

func (l *store) checkBootstrap(state *pb.CheckerState) {
l.checkBootstrapWithSetter(state, l.setBootstrapState)
}
Expand Down
29 changes: 29 additions & 0 deletions pkg/logservice/store_hakeeper_check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/matrixorigin/matrixone/pkg/pb/metadata"
"github.com/matrixorigin/matrixone/pkg/pb/task"
"github.com/matrixorigin/matrixone/pkg/taskservice"
"github.com/matrixorigin/matrixone/pkg/util/toml"
)

func TestIDAllocatorDefaultState(t *testing.T) {
Expand All @@ -46,6 +47,29 @@ func TestIDAllocatorDefaultState(t *testing.T) {
assert.Equal(t, uint64(0), v)
}

func TestNextHAKeeperCheckIntervalUsesFastBootstrapInterval(t *testing.T) {
s := &store{
cfg: Config{
HAKeeperCheckInterval: toml.Duration{Duration: 3 * time.Second},
},
}

require.Equal(t, bootstrapHAKeeperCheckInterval, s.initialHAKeeperCheckInterval())
require.Equal(t, 3*time.Second, s.nextHAKeeperCheckInterval(nil))
require.Equal(t, bootstrapHAKeeperCheckInterval, s.nextHAKeeperCheckInterval(&pb.CheckerState{
State: pb.HAKeeperCreated,
}))
require.Equal(t, bootstrapHAKeeperCheckInterval, s.nextHAKeeperCheckInterval(&pb.CheckerState{
State: pb.HAKeeperBootstrapping,
}))
require.Equal(t, bootstrapHAKeeperCheckInterval, s.nextHAKeeperCheckInterval(&pb.CheckerState{
State: pb.HAKeeperBootstrapCommandsReceived,
}))
require.Equal(t, 3*time.Second, s.nextHAKeeperCheckInterval(&pb.CheckerState{
State: pb.HAKeeperRunning,
}))
}
Comment on lines +50 to +71

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 33cdfa7. The test now asserts nil uses the configured interval, adds an explicit HAKeeperCreated bootstrap case, and checks the separate initial fast interval helper. Verified with GOPROXY=https://goproxy.cn,direct .agents/skills/mo-dev/scripts/mo-cgo-test -v -run '^(TestServiceBootstrapRestoresHAKeeperAndWAL|TestSetInitialClusterInfo|TestNextHAKeeperCheckIntervalUsesFastBootstrapInterval|TestBootstrap|TestFailedBootstrap)$' -count=1 -timeout=180s ./pkg/logservice.


func TestIDAllocatorCapacity(t *testing.T) {
tests := []struct {
next uint64
Expand Down Expand Up @@ -940,11 +964,16 @@ func testBootstrap(t *testing.T, fail bool, remoteRecoveryPending bool) {

state, err = store.getCheckerState()
require.NoError(t, err)
bootstrapCommandsAdded := false
store.bootstrapCommandsAdded = func() {
bootstrapCommandsAdded = true
}
store.bootstrap(term, state)

state, err = store.getCheckerState()
require.NoError(t, err)
assert.Equal(t, pb.HAKeeperBootstrapCommandsReceived, state.State)
assert.True(t, bootstrapCommandsAdded)
assert.Equal(t, uint64(checkBootstrapCycles), store.bootstrapCheckCycles)
require.NotNil(t, store.bootstrapMgr)
assert.False(t, store.bootstrapMgr.CheckBootstrap(state.LogState))
Expand Down
Loading
Loading