Skip to content

Commit

Permalink
fix more stats races
Browse files Browse the repository at this point in the history
  • Loading branch information
max-hoffman committed Sep 10, 2024
1 parent 9450b29 commit c4bf019
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
6 changes: 4 additions & 2 deletions go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1954,6 +1954,7 @@ func TestStatsAutoRefreshConcurrency(t *testing.T) {
// create engine
harness := newDoltHarness(t)
harness.Setup(setup.MydbData)
harness.configureStats = true
engine := mustNewEngine(t, harness)
defer engine.Close()

Expand All @@ -1973,8 +1974,9 @@ func TestStatsAutoRefreshConcurrency(t *testing.T) {
// it is important to use new sessions for this test, to avoid working root conflicts
readCtx := enginetest.NewSession(harness)
writeCtx := enginetest.NewSession(harness)
refreshCtx := enginetest.NewSession(harness)
newCtx := func(context.Context) (*sql.Context, error) {
return enginetest.NewSession(harness), nil
return refreshCtx, nil
}

err := statsProv.InitAutoRefreshWithParams(newCtx, sqlDb.Name(), bThreads, intervalSec, thresholdf64, branches)
Expand All @@ -1988,7 +1990,7 @@ func TestStatsAutoRefreshConcurrency(t *testing.T) {
require.NoError(t, err)
}

iters := 1_000
iters := 50
{
// 3 threads to test auto-refresh/DML concurrency safety
// - auto refresh (read + write)
Expand Down
13 changes: 9 additions & 4 deletions go/libraries/doltcore/sqle/statsnoms/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,6 @@ func (n *NomsStatsDatabase) LoadBranchStats(ctx *sql.Context, branch string) err
}

func (n *NomsStatsDatabase) getBranchStats(branch string) dbStats {
n.mu.Lock()
defer n.mu.Unlock()

for i, b := range n.branches {
if strings.EqualFold(b, branch) {
return n.stats[i]
Expand All @@ -163,12 +160,16 @@ func (n *NomsStatsDatabase) getBranchStats(branch string) dbStats {
}

func (n *NomsStatsDatabase) GetStat(branch string, qual sql.StatQualifier) (*statspro.DoltStats, bool) {
n.mu.Lock()
defer n.mu.Unlock()
stats := n.getBranchStats(branch)
ret, ok := stats[qual]
return ret, ok
}

func (n *NomsStatsDatabase) ListStatQuals(branch string) []sql.StatQualifier {
n.mu.Lock()
defer n.mu.Unlock()
stats := n.getBranchStats(branch)
var ret []sql.StatQualifier
for qual, _ := range stats {
Expand Down Expand Up @@ -286,7 +287,11 @@ func (n *NomsStatsDatabase) ReplaceChunks(ctx context.Context, branch string, qu
if err != nil {
return err
}
dbStat[qual].Hist = targetBuckets
newStat, err := dbStat[qual].WithHistogram(targetBuckets)
if err != nil {
return err
}
dbStat[qual] = newStat.(*statspro.DoltStats)
} else {
dbStat[qual] = statspro.NewDoltStats()
}
Expand Down
4 changes: 4 additions & 0 deletions go/libraries/doltcore/sqle/statspro/dolt_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ func (s *DoltStats) ToInterface() (interface{}, error) {
}

func (s *DoltStats) WithHistogram(h sql.Histogram) (sql.Statistic, error) {
s.mu.Lock()
defer s.mu.Unlock()
ret := *s
ret.Hist = nil
for _, b := range h {
Expand All @@ -174,6 +176,8 @@ func (s *DoltStats) WithHistogram(h sql.Histogram) (sql.Statistic, error) {
}

func (s *DoltStats) Histogram() sql.Histogram {
s.mu.Lock()
defer s.mu.Unlock()
return s.Hist
}

Expand Down

0 comments on commit c4bf019

Please sign in to comment.