Skip to content

Commit

Permalink
[stats]: fix panic for empty table stats with one root chunk (#7507)
Browse files Browse the repository at this point in the history
* [stats]: fix panic for empty table stats with one root chunk

* log stats load errors as warnings

* splunk support for stats
  • Loading branch information
max-hoffman authored Feb 26, 2024
1 parent 45a4923 commit 2fbcb38
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 4 deletions.
3 changes: 2 additions & 1 deletion go/cmd/dolt/commands/engine/sqlengine.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package engine

import (
"context"
"fmt"
"os"
"runtime"
"strconv"
Expand Down Expand Up @@ -192,7 +193,7 @@ func NewSqlEngine(
// configuring stats depends on sessionBuilder
// sessionBuilder needs ref to statsProv
if err = statsPro.Configure(ctx, sqlEngine.NewDefaultContext, bThreads, pro, dbs); err != nil {
return nil, err
fmt.Fprintln(cli.CliErr, err)
}

// Load MySQL Db information
Expand Down
6 changes: 5 additions & 1 deletion go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2110,7 +2110,11 @@ func TestStatsFunctions(t *testing.T) {
harness.configureStats = true
for _, test := range StatProcTests {
t.Run(test.Name, func(t *testing.T) {
enginetest.TestScript(t, harness, test)
// reset engine so provider statistics are clean
harness.engine = nil
e := mustNewEngine(t, harness)
defer e.Close()
enginetest.TestScriptWithEngine(t, e, harness, test)
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion go/libraries/doltcore/sqle/enginetest/dolt_harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ func (d *DoltHarness) NewEngine(t *testing.T) (enginetest.QueryEngine, error) {
// Get a fresh session if we are reusing the engine
if !initializeEngine {
var err error
d.session, err = dsess.NewDoltSession(enginetest.NewBaseSession(), d.provider, d.multiRepoEnv.Config(), d.branchControl, nil)
d.session, err = dsess.NewDoltSession(enginetest.NewBaseSession(), d.provider, d.multiRepoEnv.Config(), d.branchControl, d.statsPro)
require.NoError(t, err)
}

Expand Down
63 changes: 63 additions & 0 deletions go/libraries/doltcore/sqle/enginetest/stats_queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,69 @@ var DoltStatsIOTests = []queries.ScriptTest{
}

var StatProcTests = []queries.ScriptTest{
{
Name: "deleting stats removes information_schema access point",
SetUpScript: []string{
"CREATE table xy (x bigint primary key, y int, z varchar(500), key(y,z));",
"insert into xy values (0,0,0)",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "analyze table xy",
},
{
Query: "select count(*) from information_schema.column_statistics",
Expected: []sql.Row{{2}},
},
{
Query: "call dolt_stats_drop()",
},
{
Query: "select count(*) from information_schema.column_statistics",
Expected: []sql.Row{{0}},
},
},
},
{
Name: "restart empty stats panic",
SetUpScript: []string{
"CREATE table xy (x bigint primary key, y int, z varchar(500), key(y,z));",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "analyze table xy",
},
{
Query: "select count(*) from dolt_statistics",
Expected: []sql.Row{{0}},
},
{
Query: "set @@GLOBAL.dolt_stats_auto_refresh_threshold = 0",
Expected: []sql.Row{{}},
},
{
Query: "set @@GLOBAL.dolt_stats_auto_refresh_interval = 0",
Expected: []sql.Row{{}},
},
{
// don't panic
Query: "call dolt_stats_restart()",
},
{
Query: "select sleep(.1)",
},
{
Query: "insert into xy values (0,0,0)",
},
{
Query: "select sleep(.1)",
},
{
Query: "select count(*) from dolt_statistics",
Expected: []sql.Row{{2}},
},
},
},
{
Name: "basic start, status, stop loop",
SetUpScript: []string{
Expand Down
5 changes: 5 additions & 0 deletions go/libraries/doltcore/sqle/stats/stats_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,11 @@ func (p *Provider) Load(ctx *sql.Context, dbs []dsess.SqlDatabase) error {
} else if err != nil {
return err
}
if cnt, err := m.Count(); err != nil {
return err
} else if cnt == 0 {
return nil
}
stats, err := loadStats(ctx, db, m)
if errors.Is(err, dtables.ErrIncompatibleVersion) {
ctx.Warn(0, err.Error())
Expand Down
5 changes: 4 additions & 1 deletion go/libraries/doltcore/sqle/stats/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ func updateStats(ctx *sql.Context, sqlTable sql.Table, dTab *doltdb.Table, index
} else if cnt == 0 {
// table is empty
ret[meta.qual] = NewDoltStats()
ret[meta.qual].chunks = meta.allAddrs
ret[meta.qual].CreatedAt = time.Now()
ret[meta.qual].Columns = meta.cols
ret[meta.qual].Types = types
Expand Down Expand Up @@ -192,6 +191,10 @@ func mergeStatUpdates(newStats *DoltStats, idxMeta indexMeta) *DoltStats {
}
}

if len(mergeHist) == 0 {
return newStats
}

newStats.Histogram = mergeHist
newStats.chunks = idxMeta.allAddrs
newStats.updateActive()
Expand Down
7 changes: 7 additions & 0 deletions go/store/types/serial_message.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,13 @@ func (sm SerialMessage) humanReadableStringAtIndentationLevel(level int) string
printWithIndendationLevel(level, ret, "\tHeadCommitAddr: #%s\n", hash.New(msg.HeadCommitAddrBytes()).String())
printWithIndendationLevel(level, ret, "}")
return ret.String()
case serial.StatisticFileID:
msg, _ := serial.TryGetRootAsStatistic(sm, serial.MessagePrefixSz)
ret := &strings.Builder{}
printWithIndendationLevel(level, ret, "{\n")
printWithIndendationLevel(level, ret, "\tStatsRoot: #%s\n", hash.New(msg.RootBytes()).String())
printWithIndendationLevel(level, ret, "}")
return ret.String()
case serial.TagFileID:
msg, _ := serial.TryGetRootAsTag(sm, serial.MessagePrefixSz)
ret := &strings.Builder{}
Expand Down

0 comments on commit 2fbcb38

Please sign in to comment.