Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

br: explicitly set analyze sample number if tidb_analyze_version = 2 #27402

Merged
merged 8 commits into from
Aug 24, 2021
Merged
Changes from 5 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
47 changes: 45 additions & 2 deletions br/pkg/lightning/restore/table_restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ package restore

import (
"context"
"fmt"
"sort"
"strconv"
"sync"
"time"

Expand All @@ -33,12 +35,13 @@ import (
"github.com/pingcap/tidb/br/pkg/lightning/mydump"
verify "github.com/pingcap/tidb/br/pkg/lightning/verification"
"github.com/pingcap/tidb/br/pkg/lightning/worker"
"github.com/pingcap/tidb/br/pkg/logutil"
"github.com/pingcap/tidb/br/pkg/utils"
"github.com/pingcap/tidb/meta/autoid"
"github.com/pingcap/tidb/table"
"github.com/pingcap/tidb/table/tables"
"go.uber.org/multierr"
"go.uber.org/zap"
"github.com/pingcap/tidb/br/pkg/utils"
)

type TableRestore struct {
Expand Down Expand Up @@ -886,9 +889,49 @@ func (tr *TableRestore) compareChecksum(remoteChecksum *RemoteChecksum, localChe
return nil
}

func (tr *TableRestore) genAnalyzeSQL(ctx context.Context, g glue.SQLExecutor) string {
// TODO: because tidb v5.2 analyze full table OOM easily, we fallback to add `WITH NUM 10000` parameter
// if there are more than 600 regions in this table
sql := "ANALYZE TABLE "+tr.tableName
result, err := g.QueryStringsWithLog(ctx, "SHOW VARIABLES LIKE 'tidb_analyze_version'", "fetch tidb analyze version", tr.logger)
Copy link
Contributor

@kennytm kennytm Aug 23, 2021

Choose a reason for hiding this comment

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

why not just select @@tidb_analyze_version 😕 (fallback to 1 on error)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally want to be compatible with v4.0, but It's simplier to fallback to 1 on error.

if err != nil {
tr.logger.Warn("fetch tidb analyze version failed, will fallback to naive analyze", logutil.ShortError(err))
return sql
}
analyzeVer := ""
for _, params := range result {
if len(params) != 2 {
tr.logger.Warn("ignore invalid show variables result", zap.Strings("row", params))
continue
}
if params[0] == "tidb_analyze_version" {
analyzeVer = params[1]
break
}
}
if analyzeVer != "2" {
return sql
}
query := fmt.Sprintf("SELECT COUNT(*) FROM information_schema.TIKV_REGION_STATUS WHERE TABLE_ID = %d", tr.tableInfo.ID)
countStr, err := g.ObtainStringWithLog(ctx, query, "fetch table regions count", tr.logger)
if err != nil {
log.L().Warn("fetch table regions count failed", logutil.ShortError(err))
return sql
}
count, err := strconv.ParseInt(countStr, 10, 64)
if err != nil {
log.L().Warn("parse table regions count failed", zap.String("count", countStr), logutil.ShortError(err))
return sql
}
if count >= 600 {
sql += " WITH 10000 SAMPLES"
}
return sql
}

func (tr *TableRestore) analyzeTable(ctx context.Context, g glue.SQLExecutor) error {
task := tr.logger.Begin(zap.InfoLevel, "analyze")
err := g.ExecuteWithLog(ctx, "ANALYZE TABLE "+tr.tableName, "analyze table", tr.logger)
err := g.ExecuteWithLog(ctx, tr.genAnalyzeSQL(ctx, g), "analyze table", tr.logger)
task.End(zap.ErrorLevel, err)
return err
}
Expand Down