Skip to content
This repository was archived by the owner on Jan 28, 2021. It is now read-only.

Commit 27587a5

Browse files
jfontanajnavarro
authored andcommitted
pilosa: use different holder per db/table pair
Pilosa holder cannot be opened more than once. It has a channel to signal opened state (currently unused) that is closed after the first Open. This causes panics in the next calls: panic: close of closed channel The driver was using one holder for all db/tables. This called holder.Open once per table. This works well if all indexes belong to the same db/table but called Open several times when the indexes belong to more than one db/table pair. Now it has a map of holders so it's only opened once. Signed-off-by: Javi Fontan <[email protected]> (cherry picked from commit 144b824)
1 parent 679d337 commit 27587a5

File tree

2 files changed

+51
-11
lines changed

2 files changed

+51
-11
lines changed

sql/index/pilosa/driver.go

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,19 +67,22 @@ type (
6767
timeMapping time.Duration
6868
}
6969

70+
tableMap map[string]*pilosa.Holder
71+
dbMap map[string]tableMap
72+
7073
// Driver implements sql.IndexDriver interface.
7174
Driver struct {
72-
root string
73-
holder *pilosa.Holder
75+
root string
76+
holders dbMap
7477
}
7578
)
7679

7780
// NewDriver returns a new instance of pilosa.Driver
7881
// which satisfies sql.IndexDriver interface
7982
func NewDriver(root string) *Driver {
8083
return &Driver{
81-
root: root,
82-
holder: pilosa.NewHolder(),
84+
root: root,
85+
holders: make(dbMap),
8386
}
8487
}
8588

@@ -114,8 +117,9 @@ func (d *Driver) Create(
114117
return nil, err
115118
}
116119

117-
d.holder.Path = d.pilosaDirPath(db, table)
118-
idx, err := d.holder.CreateIndexIfNotExists(
120+
holder := d.holder(db, table)
121+
holder.Path = d.pilosaDirPath(db, table)
122+
idx, err := holder.CreateIndexIfNotExists(
119123
indexName(db, table),
120124
pilosa.IndexOptions{},
121125
)
@@ -144,19 +148,20 @@ func (d *Driver) LoadAll(db, table string) ([]sql.Index, error) {
144148
root = filepath.Join(d.root, db, table)
145149
)
146150

147-
d.holder.Path = d.pilosaDirPath(db, table)
148-
if _, err := os.Stat(d.holder.Path); err != nil {
151+
holder := d.holder(db, table)
152+
holder.Path = d.pilosaDirPath(db, table)
153+
if _, err := os.Stat(holder.Path); err != nil {
149154
if os.IsNotExist(err) {
150155
return indexes, nil
151156
}
152157
return nil, err
153158
}
154159

155-
err := d.holder.Open()
160+
err := holder.Open()
156161
if err != nil {
157162
return nil, err
158163
}
159-
defer d.holder.Close()
164+
defer holder.Close()
160165

161166
dirs, err := ioutil.ReadDir(root)
162167
if err != nil {
@@ -185,9 +190,27 @@ func (d *Driver) LoadAll(db, table string) ([]sql.Index, error) {
185190
return indexes, nil
186191
}
187192

193+
func (d *Driver) holder(db, table string) *pilosa.Holder {
194+
tables, ok := d.holders[db]
195+
if !ok {
196+
tables = make(tableMap)
197+
d.holders[db] = tables
198+
}
199+
200+
holder, ok := tables[table]
201+
if !ok {
202+
holder = pilosa.NewHolder()
203+
tables[table] = holder
204+
d.holders[db] = tables
205+
}
206+
207+
return holder
208+
}
209+
188210
func (d *Driver) loadIndex(db, table, id string) (*pilosaIndex, error) {
189211
name := indexName(db, table)
190-
idx := d.holder.Index(name)
212+
holder := d.holder(db, table)
213+
idx := holder.Index(name)
191214
if idx == nil {
192215
return nil, errLoadingIndex.New(name)
193216
}

sql/index/pilosa/driver_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,23 @@ func TestLoadAll(t *testing.T) {
8080
require.True(ok)
8181

8282
require.Equal(i1.index.Name(), i2.index.Name())
83+
84+
// Load index from another table. Previously this panicked as the same
85+
// pilosa.Holder was used for all indexes.
86+
87+
idx3, err := d.Create("db", "table2", "id1", makeExpressions("table2", "hash1"), nil)
88+
require.NoError(err)
89+
it3 := &partitionKeyValueIter{
90+
partitions: 2,
91+
offset: 0,
92+
total: 64,
93+
expressions: idx3.Expressions(),
94+
location: randLocation,
95+
}
96+
require.NoError(d.Save(sql.NewEmptyContext(), idx3, it3))
97+
98+
indexes, err = d.LoadAll("db", "table2")
99+
require.NoError(err)
83100
}
84101

85102
type logLoc struct {

0 commit comments

Comments
 (0)