Skip to content

Commit 30181d4

Browse files
zeripathtechknowlogick
authored andcommitted
Wrap the code indexer (#9476)
* Wrap the code indexer In order to prevent a data race in the code indexer it must be wrapped with a holder otherwise it is possible to Search/Index on an incompletely initialised indexer, and search will fail with a nil pointer until the repository indexer is initialised. Further a completely initialised repository indexer should not be closed until Termination otherwise actions in Hammer/Shutdown phases could block or be lost. Finally, there is a complex dance of shutdown etiquette should the index initialisation fail. This PR restores that. * Always return err if closed whilst waiting Co-authored-by: techknowlogick <[email protected]>
1 parent 34688e1 commit 30181d4

File tree

3 files changed

+132
-13
lines changed

3 files changed

+132
-13
lines changed

modules/indexer/code/indexer.go

+38-11
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,15 @@
55
package code
66

77
import (
8+
"context"
9+
"os"
810
"time"
911

1012
"code.gitea.io/gitea/modules/graceful"
1113
"code.gitea.io/gitea/modules/log"
1214
"code.gitea.io/gitea/modules/setting"
1315
)
1416

15-
var (
16-
indexer Indexer
17-
)
18-
1917
// SearchResult result of performing a search in a repo
2018
type SearchResult struct {
2119
RepoID int64
@@ -36,28 +34,45 @@ type Indexer interface {
3634
// Init initialize the repo indexer
3735
func Init() {
3836
if !setting.Indexer.RepoIndexerEnabled {
37+
indexer.Close()
3938
return
4039
}
4140

41+
ctx, cancel := context.WithCancel(context.Background())
42+
43+
graceful.GetManager().RunAtTerminate(ctx, func() {
44+
log.Debug("Closing repository indexer")
45+
indexer.Close()
46+
log.Info("PID: %d Repository Indexer closed", os.Getpid())
47+
})
48+
4249
waitChannel := make(chan time.Duration)
4350
go func() {
4451
start := time.Now()
45-
log.Info("Initializing Repository Indexer")
46-
var created bool
47-
var err error
48-
indexer, created, err = NewBleveIndexer(setting.Indexer.RepoPath)
52+
log.Info("PID: %d Initializing Repository Indexer at: %s", os.Getpid(), setting.Indexer.RepoPath)
53+
bleveIndexer, created, err := NewBleveIndexer(setting.Indexer.RepoPath)
4954
if err != nil {
55+
if bleveIndexer != nil {
56+
bleveIndexer.Close()
57+
}
58+
cancel()
5059
indexer.Close()
51-
log.Fatal("indexer.Init: %v", err)
60+
close(waitChannel)
61+
log.Fatal("PID: %d Unable to initialize the Repository Indexer at path: %s Error: %v", os.Getpid(), setting.Indexer.RepoPath, err)
5262
}
63+
indexer.set(bleveIndexer)
5364

5465
go processRepoIndexerOperationQueue(indexer)
5566

5667
if created {
5768
go populateRepoIndexer()
5869
}
70+
select {
71+
case waitChannel <- time.Since(start):
72+
case <-graceful.GetManager().IsShutdown():
73+
}
5974

60-
waitChannel <- time.Since(start)
75+
close(waitChannel)
6176
}()
6277

6378
if setting.Indexer.StartupTimeout > 0 {
@@ -67,9 +82,21 @@ func Init() {
6782
timeout += setting.GracefulHammerTime
6883
}
6984
select {
70-
case duration := <-waitChannel:
85+
case <-graceful.GetManager().IsShutdown():
86+
log.Warn("Shutdown before Repository Indexer completed initialization")
87+
cancel()
88+
indexer.Close()
89+
case duration, ok := <-waitChannel:
90+
if !ok {
91+
log.Warn("Repository Indexer Initialization failed")
92+
cancel()
93+
indexer.Close()
94+
return
95+
}
7196
log.Info("Repository Indexer Initialization took %v", duration)
7297
case <-time.After(timeout):
98+
cancel()
99+
indexer.Close()
73100
log.Fatal("Repository Indexer Initialization Timed-Out after: %v", timeout)
74101
}
75102
}()

modules/indexer/code/queue.go

-2
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ type repoIndexerOperation struct {
2222
var repoIndexerOperationQueue chan repoIndexerOperation
2323

2424
func processRepoIndexerOperationQueue(indexer Indexer) {
25-
defer indexer.Close()
26-
2725
repoIndexerOperationQueue = make(chan repoIndexerOperation, setting.Indexer.UpdateQueueLength)
2826
for {
2927
select {

modules/indexer/code/wrapped.go

+94
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
// Copyright 2019 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package code
6+
7+
import (
8+
"fmt"
9+
"sync"
10+
)
11+
12+
var (
13+
indexer = newWrappedIndexer()
14+
)
15+
16+
// ErrWrappedIndexerClosed is the error returned if the indexer was closed before it was ready
17+
var ErrWrappedIndexerClosed = fmt.Errorf("Indexer closed before ready")
18+
19+
type wrappedIndexer struct {
20+
internal Indexer
21+
lock sync.RWMutex
22+
cond *sync.Cond
23+
closed bool
24+
}
25+
26+
func newWrappedIndexer() *wrappedIndexer {
27+
w := &wrappedIndexer{}
28+
w.cond = sync.NewCond(w.lock.RLocker())
29+
return w
30+
}
31+
32+
func (w *wrappedIndexer) set(indexer Indexer) {
33+
w.lock.Lock()
34+
defer w.lock.Unlock()
35+
if w.closed {
36+
// Too late!
37+
indexer.Close()
38+
}
39+
w.internal = indexer
40+
w.cond.Broadcast()
41+
}
42+
43+
func (w *wrappedIndexer) get() (Indexer, error) {
44+
w.lock.RLock()
45+
defer w.lock.RUnlock()
46+
if w.internal == nil {
47+
if w.closed {
48+
return nil, ErrWrappedIndexerClosed
49+
}
50+
w.cond.Wait()
51+
if w.closed {
52+
return nil, ErrWrappedIndexerClosed
53+
}
54+
}
55+
return w.internal, nil
56+
}
57+
58+
func (w *wrappedIndexer) Index(repoID int64) error {
59+
indexer, err := w.get()
60+
if err != nil {
61+
return err
62+
}
63+
return indexer.Index(repoID)
64+
}
65+
66+
func (w *wrappedIndexer) Delete(repoID int64) error {
67+
indexer, err := w.get()
68+
if err != nil {
69+
return err
70+
}
71+
return indexer.Delete(repoID)
72+
}
73+
74+
func (w *wrappedIndexer) Search(repoIDs []int64, keyword string, page, pageSize int) (int64, []*SearchResult, error) {
75+
indexer, err := w.get()
76+
if err != nil {
77+
return 0, nil, err
78+
}
79+
return indexer.Search(repoIDs, keyword, page, pageSize)
80+
81+
}
82+
83+
func (w *wrappedIndexer) Close() {
84+
w.lock.Lock()
85+
defer w.lock.Unlock()
86+
if w.closed {
87+
return
88+
}
89+
w.closed = true
90+
w.cond.Broadcast()
91+
if w.internal != nil {
92+
w.internal.Close()
93+
}
94+
}

0 commit comments

Comments
 (0)