Skip to content

Commit feb845a

Browse files
Log and ignore the error in reading udfs in schema tracker (vitessio#16630)
Signed-off-by: Manan Gupta <[email protected]>
1 parent 538dd4c commit feb845a

File tree

4 files changed

+32
-6
lines changed

4 files changed

+32
-6
lines changed

go/mysql/fakesqldb/server.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,13 @@ func (db *DB) AddQueryPattern(queryPattern string, expectedResult *sqltypes.Resu
592592
db.patternData[queryPattern] = exprResult{queryPattern: queryPattern, expr: expr, result: &result}
593593
}
594594

595+
// RemoveQueryPattern removes a query pattern that was previously added.
596+
func (db *DB) RemoveQueryPattern(queryPattern string) {
597+
db.mu.Lock()
598+
defer db.mu.Unlock()
599+
delete(db.patternData, queryPattern)
600+
}
601+
595602
// RejectQueryPattern allows a query pattern to be rejected with an error
596603
func (db *DB) RejectQueryPattern(queryPattern, error string) {
597604
expr := regexp.MustCompile("(?is)^" + queryPattern + "$")

go/vt/logutil/throttled.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ var (
5454
errorDepth = log.ErrorDepth
5555
)
5656

57+
// GetLastLogTime gets the last log time for the throttled logger.
58+
func (tl *ThrottledLogger) GetLastLogTime() time.Time {
59+
return tl.lastlogTime
60+
}
61+
5762
func (tl *ThrottledLogger) log(logF logFunc, format string, v ...any) {
5863
now := time.Now()
5964

go/vt/vttablet/tabletserver/schema/engine.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
"vitess.io/vitess/go/vt/dbconfigs"
4141
"vitess.io/vitess/go/vt/dbconnpool"
4242
"vitess.io/vitess/go/vt/log"
43+
"vitess.io/vitess/go/vt/logutil"
4344
"vitess.io/vitess/go/vt/mysqlctl/tmutils"
4445
"vitess.io/vitess/go/vt/schema"
4546
"vitess.io/vitess/go/vt/servenv"
@@ -85,9 +86,10 @@ type Engine struct {
8586

8687
historian *historian
8788

88-
conns *connpool.Pool
89-
ticks *timer.Timer
90-
reloadTimeout time.Duration
89+
conns *connpool.Pool
90+
ticks *timer.Timer
91+
reloadTimeout time.Duration
92+
throttledLogger *logutil.ThrottledLogger
9193

9294
// dbCreationFailed is for preventing log spam.
9395
dbCreationFailed bool
@@ -109,7 +111,8 @@ func NewEngine(env tabletenv.Env) *Engine {
109111
Size: 3,
110112
IdleTimeout: env.Config().OltpReadPool.IdleTimeout,
111113
}),
112-
ticks: timer.NewTimer(reloadTime),
114+
ticks: timer.NewTimer(reloadTime),
115+
throttledLogger: logutil.NewThrottledLogger("schema-tracker", 1*time.Minute),
113116
}
114117
se.schemaCopy = env.Config().SignalWhenSchemaChange
115118
_ = env.Exporter().NewGaugeDurationFunc("SchemaReloadTime", "vttablet keeps table schemas in its own memory and periodically refreshes it from MySQL. This config controls the reload time.", se.ticks.Interval)
@@ -448,7 +451,7 @@ func (se *Engine) reload(ctx context.Context, includeStats bool) error {
448451

449452
udfsChanged, err := getChangedUserDefinedFunctions(ctx, conn.Conn, shouldUseDatabase)
450453
if err != nil {
451-
return err
454+
se.throttledLogger.Errorf("error in getting changed UDFs: %v", err)
452455
}
453456

454457
rec := concurrency.AllErrorRecorder{}

go/vt/vttablet/tabletserver/schema/engine_test.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1331,7 +1331,8 @@ func TestEngineReload(t *testing.T) {
13311331
}
13321332

13331333
// adding query pattern for udfs
1334-
db.AddQueryPattern("SELECT name.*", &sqltypes.Result{})
1334+
udfQueryPattern := "SELECT name.*"
1335+
db.AddQueryPattern(udfQueryPattern, &sqltypes.Result{})
13351336

13361337
// Verify the list of created, altered and dropped tables seen.
13371338
se.RegisterNotifier("test", func(full map[string]*Table, created, altered, dropped []*Table, _ bool) {
@@ -1344,6 +1345,16 @@ func TestEngineReload(t *testing.T) {
13441345
err = se.reload(context.Background(), false)
13451346
require.NoError(t, err)
13461347
require.NoError(t, db.LastError())
1348+
require.Zero(t, se.throttledLogger.GetLastLogTime())
1349+
1350+
// Now if we remove the query pattern for udfs, schema engine shouldn't fail.
1351+
// Instead we should see a log message with the error.
1352+
db.RemoveQueryPattern(udfQueryPattern)
1353+
se.UnregisterNotifier("test")
1354+
err = se.reload(context.Background(), false)
1355+
require.NoError(t, err)
1356+
// Check for the udf error being logged. The last log time should be less than a second.
1357+
require.Less(t, time.Since(se.throttledLogger.GetLastLogTime()), 1*time.Second)
13471358
}
13481359

13491360
// TestEngineReload tests the vreplication specific GetTableForPos function to ensure

0 commit comments

Comments
 (0)