-
-
Notifications
You must be signed in to change notification settings - Fork 818
[stats] Rewrite stat management to use single threaded event loop #8815
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
Changes from 129 commits
796564b
5034635
8de66e4
d6882e4
4c5bd3f
c2876de
4d8d8f0
bb6ab3c
542bc40
14cf9fd
d949b3d
397aaa9
16ff4ff
1d04f74
2be37c1
ee16cf1
d18b524
ee2286b
4a91332
128efd5
347d3f5
9bdb958
d503c4e
76a45ff
373aa9a
c56dd06
14eae29
6ab5193
474a85f
d8e6c09
31d3780
c2c4f05
e23cf1f
4da767b
8d3c07f
6578011
aafeec7
6dd1fb4
4635cfa
ec8ed11
c95fcda
2e424eb
e849f27
8224b09
43e739d
b209532
043c174
7291054
40cdce0
d040cfa
6df2999
fe72f62
1be0f49
7a35c3d
468dafc
9c595c9
efa0412
e3811ee
f46a1c4
ad9ed8e
4910909
c31dd08
6d2ea07
f171a50
8d8398a
d637699
3c5f1a9
aa23dde
4f926d3
9e260cb
8a64097
9b3a8cb
ff64cfb
99c4e91
f070e05
791dff0
c394276
f53de92
7e58b09
8e48425
e666a48
40930aa
28947eb
1823dcc
9535ffb
a20401f
2c3c271
2d02535
258f5e9
5775bfe
002ece8
f656924
39ee000
380c51b
657c032
2a32486
a28f36f
e3295a6
4e12e77
7e1d1b7
10f8bc3
10e67a7
6b36433
58879d6
e9d58f2
4d76ab2
0e4c2e5
11bbba7
f07fef3
bb69370
687f497
e73931a
7864cf1
b13e8cf
6c8a0db
c36381b
d1631d4
9e15822
7b68907
7579620
15d51a6
66e8c71
a35b9c7
9923a6e
cd6fb69
73db71f
0772a00
3f238d0
ac16aca
8591963
6ba5996
06d3419
ca41655
22d7801
0fb4128
91f0cf6
a8fd924
de47ba3
2a75378
4d0bab5
05bcf2c
c701726
cb57493
f71fbd5
a4d4f72
252efd0
8fd7a05
bc7f15a
73fcedc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2110,8 +2110,8 @@ func (ddb *DoltDB) AddStash(ctx context.Context, head *Commit, stash RootValue, | |
| return err | ||
| } | ||
|
|
||
| func (ddb *DoltDB) SetStatisics(ctx context.Context, branch string, addr hash.Hash) error { | ||
| statsDs, err := ddb.db.GetDataset(ctx, ref.NewStatsRef(branch).String()) | ||
| func (ddb *DoltDB) SetStatistics(ctx context.Context, branch string, addr hash.Hash) error { | ||
| statsDs, err := ddb.db.GetDataset(ctx, ref.NewStatsRef().String()) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -2120,7 +2120,7 @@ func (ddb *DoltDB) SetStatisics(ctx context.Context, branch string, addr hash.Ha | |
| } | ||
|
|
||
| func (ddb *DoltDB) DropStatisics(ctx context.Context, branch string) error { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is branch now unused? |
||
| statsDs, err := ddb.db.GetDataset(ctx, ref.NewStatsRef(branch).String()) | ||
| statsDs, err := ddb.db.GetDataset(ctx, ref.NewStatsRef().String()) | ||
|
|
||
| _, err = ddb.db.Delete(ctx, statsDs, "") | ||
| if err != nil { | ||
|
|
@@ -2132,8 +2132,8 @@ func (ddb *DoltDB) DropStatisics(ctx context.Context, branch string) error { | |
| var ErrNoStatistics = errors.New("no statistics found") | ||
|
|
||
| // GetStatistics returns the value of the singleton ref.StatsRef for this database | ||
| func (ddb *DoltDB) GetStatistics(ctx context.Context, branch string) (prolly.Map, error) { | ||
| ds, err := ddb.db.GetDataset(ctx, ref.NewStatsRef(branch).String()) | ||
| func (ddb *DoltDB) GetStatistics(ctx context.Context) (prolly.Map, error) { | ||
| ds, err := ddb.db.GetDataset(ctx, ref.NewStatsRef().String()) | ||
| if err != nil { | ||
| return prolly.Map{}, err | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -273,8 +273,13 @@ type prollyIndex struct { | |
| } | ||
|
|
||
| // ProllyMapFromIndex unwraps the Index and returns the underlying prolly.Map. | ||
| func ProllyMapFromIndex(i Index) prolly.Map { | ||
| return i.(prollyIndex).index | ||
| func ProllyMapFromIndex(i Index) (prolly.Map, error) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kind of a large change. Not necessarily opposed, but is this strictly to accommodate VectorIndexes? And if so, is there a less intrusive change that accomplishes it? Seems like a panic is as good as an error for catching this kind of mismatch in logic where it needs to be updated.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean i can undo it and handle it separately for stats if you'd prefer. I'd be surprised if this doesn't also affect merges and schema changes |
||
| switch i := i.(type) { | ||
| case prollyIndex: | ||
| return i.index, nil | ||
| default: | ||
| return prolly.Map{}, fmt.Errorf("expected prollyIndex, found: %T", i) | ||
| } | ||
| } | ||
|
|
||
| // xxx: don't use this, temporary fix waiting for bigger | ||
|
|
@@ -369,7 +374,10 @@ func (i prollyIndex) AddColumnToRows(ctx context.Context, newCol string, newSche | |
| } | ||
|
|
||
| // If not, then we have to iterate over this table's rows and update all the offsets for the new column | ||
| rowMap := ProllyMapFromIndex(i) | ||
| rowMap, err := ProllyMapFromIndex(i) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| mutator := rowMap.Mutate() | ||
|
|
||
| iter, err := mutator.IterAll(ctx) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should pull this logic into its own method