Skip to content

Commit 471350d

Browse files
committed
refactor/search: panic at corrupt offset source
Once corruption deliberately uses the crashed-shard recovery path, retaining mutable provider errors and later checkpoints only creates opportunities to forget propagation. Panic with shard, repository, document, and invariant context at detection so recovery remains immediate and valid searches carry no error-state bookkeeping. Amp-Thread-ID: https://ampcode.com/threads/T-019fd1ab-1ad4-77f5-9b59-c65232f5b3f1
1 parent a1732f4 commit 471350d

4 files changed

Lines changed: 44 additions & 36 deletions

File tree

index/contentprovider.go

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ type contentProvider struct {
3737
stats *zoekt.Stats
3838

3939
// mutable
40-
err error
4140
idx uint32
4241
_data []byte
4342
_nl []uint32
@@ -51,7 +50,6 @@ type contentProvider struct {
5150
func (p *contentProvider) setDocument(docID uint32) {
5251
fileStart := p.id.boundaries[docID]
5352

54-
p.err = nil
5553
p.idx = docID
5654
p.fileSize = p.id.boundaries[docID+1] - fileStart
5755

@@ -60,18 +58,34 @@ func (p *contentProvider) setDocument(docID uint32) {
6058
p._data = nil
6159
}
6260

63-
func (p *contentProvider) setError(err error) {
64-
if p.err == nil {
65-
p.err = err
61+
// panicCorrupt stops a search as soon as it detects a corrupt shard invariant.
62+
// searchOneShard recovers the panic at the shard boundary, logs the error with
63+
// the query and stack trace, and increments Stats.Crashes. Callers therefore get
64+
// partial results with an explicit crashed-shard signal rather than a silent
65+
// non-match.
66+
func (p *contentProvider) panicCorrupt(err error) {
67+
shard := "unknown"
68+
if p.id.file != nil {
69+
shard = p.id.file.Name()
70+
}
71+
repo := "unknown"
72+
if p.idx < uint32(len(p.id.repos)) {
73+
repoID := p.id.repos[p.idx]
74+
if int(repoID) < len(p.id.repoMetaData) {
75+
repo = p.id.repoMetaData[repoID].Name
76+
}
6677
}
78+
panic(fmt.Errorf("corrupt shard %q while searching repository %q, document %d: %w", shard, repo, p.idx, err))
6779
}
6880

6981
func (p *contentProvider) docSections() []DocumentSection {
7082
if p._sects == nil {
7183
var sz uint32
7284
var err error
7385
p._sects, sz, err = p.id.readDocSections(p.idx, p._sectBuf)
74-
p.setError(err)
86+
if err != nil {
87+
p.panicCorrupt(fmt.Errorf("reading document sections: %w", err))
88+
}
7589
p.stats.ContentBytesLoaded += int64(sz)
7690
p._sectBuf = p._sects
7791
}
@@ -83,7 +97,9 @@ func (p *contentProvider) newlines() newlines {
8397
var sz uint32
8498
var err error
8599
p._nl, sz, err = p.id.readNewlines(p.idx, p._nlBuf)
86-
p.setError(err)
100+
if err != nil {
101+
p.panicCorrupt(fmt.Errorf("reading newline offsets: %w", err))
102+
}
87103
p._nlBuf = p._nl
88104
p.stats.ContentBytesLoaded += int64(sz)
89105
}
@@ -98,7 +114,9 @@ func (p *contentProvider) data(fileName bool) []byte {
98114
if p._data == nil {
99115
var err error
100116
p._data, err = p.id.readContents(p.idx)
101-
p.setError(err)
117+
if err != nil {
118+
p.panicCorrupt(fmt.Errorf("reading content: %w", err))
119+
}
102120
p.stats.FilesLoaded++
103121
p.stats.ContentBytesLoaded += int64(len(p._data))
104122
}
@@ -128,7 +146,7 @@ func (p *contentProvider) findOffset(filename bool, r uint32) uint32 {
128146

129147
if p.id.metaData.PlainASCII {
130148
if r > fileEndByte-fileStartByte {
131-
p.setError(fmt.Errorf("corrupt index: document %d %s rune offset %d is after file size %d", p.idx, kind, r, fileEndByte-fileStartByte))
149+
p.panicCorrupt(fmt.Errorf("%s rune offset %d is after file size %d", kind, r, fileEndByte-fileStartByte))
132150
return 0
133151
}
134152
return r
@@ -139,7 +157,7 @@ func (p *contentProvider) findOffset(filename bool, r uint32) uint32 {
139157
absR64 += uint64(runeEnds[p.idx-1])
140158
}
141159
if absR64 > uint64(^uint32(0)) {
142-
p.setError(fmt.Errorf("corrupt index: document %d %s rune offset %d overflows the corpus rune offset", p.idx, kind, r))
160+
p.panicCorrupt(fmt.Errorf("%s rune offset %d overflows the corpus rune offset", kind, r))
143161
return 0
144162
}
145163
absR := uint32(absR64)
@@ -150,26 +168,26 @@ func (p *contentProvider) findOffset(filename bool, r uint32) uint32 {
150168

151169
if filename {
152170
if byteOff > uint64(len(p.id.fileNameContent)) {
153-
p.setError(fmt.Errorf("corrupt index: document %d filename rune offset %d maps to byte offset %d past filename data size %d", p.idx, absR, byteOff, len(p.id.fileNameContent)))
171+
p.panicCorrupt(fmt.Errorf("filename rune offset %d maps to byte offset %d past filename data size %d", absR, byteOff, len(p.id.fileNameContent)))
154172
return 0
155173
}
156174
data = p.id.fileNameContent[uint32(byteOff):]
157175
} else {
158176
corpusEnd := p.id.boundaries[len(p.id.boundaries)-1]
159177
if byteOff > uint64(corpusEnd) {
160-
p.setError(fmt.Errorf("corrupt index: document %d content rune offset %d maps to byte offset %d past content data size %d", p.idx, absR, byteOff, corpusEnd))
178+
p.panicCorrupt(fmt.Errorf("content rune offset %d maps to byte offset %d past content data size %d", absR, byteOff, corpusEnd))
161179
return 0
162180
}
163181
var err error
164182
data, err = p.id.readContentSlice(uint32(byteOff), 3*runeOffsetFrequency)
165183
if err != nil {
166-
p.setError(fmt.Errorf("corrupt index: document %d content rune offset %d cannot load bytes at offset %d: %w", p.idx, absR, byteOff, err))
184+
p.panicCorrupt(fmt.Errorf("content rune offset %d cannot load bytes at offset %d: %w", absR, byteOff, err))
167185
return 0
168186
}
169187
}
170188
for left > 0 {
171189
if len(data) == 0 {
172-
p.setError(fmt.Errorf("corrupt index: document %d %s rune offset %d has no decode bytes at byte offset %d", p.idx, kind, absR, byteOff))
190+
p.panicCorrupt(fmt.Errorf("%s rune offset %d has no decode bytes at byte offset %d", kind, absR, byteOff))
173191
return 0
174192
}
175193
_, sz := utf8.DecodeRune(data)
@@ -179,11 +197,11 @@ func (p *contentProvider) findOffset(filename bool, r uint32) uint32 {
179197
}
180198

181199
if byteOff < uint64(fileStartByte) {
182-
p.setError(fmt.Errorf("corrupt index: document %d %s rune offset %d maps to byte offset %d before file start %d", p.idx, kind, absR, byteOff, fileStartByte))
200+
p.panicCorrupt(fmt.Errorf("%s rune offset %d maps to byte offset %d before file start %d", kind, absR, byteOff, fileStartByte))
183201
return 0
184202
}
185203
if byteOff > uint64(fileEndByte) {
186-
p.setError(fmt.Errorf("corrupt index: document %d %s rune offset %d maps to byte offset %d after file end %d", p.idx, kind, absR, byteOff, fileEndByte))
204+
p.panicCorrupt(fmt.Errorf("%s rune offset %d maps to byte offset %d after file end %d", kind, absR, byteOff, fileEndByte))
187205
return 0
188206
}
189207

index/eval.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,6 @@ nextFileMatch:
275275

276276
for cost := costMin; cost <= costMax; cost++ {
277277
state := evalMatchTree(cp, cost, known, mt)
278-
if cp.err != nil {
279-
panic(fmt.Errorf("corrupt shard %q while searching repository %q, document %d (%q): %w", d.file.Name(), md.Name, nextDoc, d.fileName(nextDoc), cp.err))
280-
}
281278
switch state {
282279
case matchesRequiresHigherCost:
283280
if cost == costMax {
@@ -343,9 +340,6 @@ nextFileMatch:
343340
if opts.Whole {
344341
fileMatch.Content = cp.data(false)
345342
}
346-
if cp.err != nil {
347-
panic(fmt.Errorf("corrupt shard %q while searching repository %q, document %d (%q): %w", d.file.Name(), md.Name, nextDoc, d.fileName(nextDoc), cp.err))
348-
}
349343

350344
matchedChunkRanges := 0
351345
for _, cm := range fileMatch.ChunkMatches {

index/matchiter_test.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
package index
1414

1515
import (
16+
"fmt"
1617
"reflect"
1718
"strings"
1819
"testing"
@@ -161,9 +162,14 @@ func TestFindOffsetReportsInvalidMappings(t *testing.T) {
161162
} {
162163
t.Run(tc.name, func(t *testing.T) {
163164
cp := contentProvider{id: &tc.id, idx: tc.idx}
164-
cp.findOffset(true, tc.r)
165-
if cp.err == nil || !strings.Contains(cp.err.Error(), tc.want) {
166-
t.Fatalf("findOffset error = %v, want error containing %q", cp.err, tc.want)
165+
var recovered any
166+
func() {
167+
defer func() { recovered = recover() }()
168+
cp.findOffset(true, tc.r)
169+
}()
170+
panicText := fmt.Sprint(recovered)
171+
if recovered == nil || !strings.Contains(panicText, tc.want) {
172+
t.Fatalf("findOffset panic = %q, want panic containing %q", panicText, tc.want)
167173
}
168174
})
169175
}

index/matchtree.go

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -740,9 +740,6 @@ nextLine:
740740
for len(children[j]) > 0 {
741741
candidate := children[j][0]
742742
bo := int(cp.findOffset(false, candidate.runeOffset))
743-
if cp.err != nil {
744-
return matchesNone
745-
}
746743
if bo < lines[i].start {
747744
children[j] = children[j][1:]
748745
continue nextCandidate
@@ -979,18 +976,11 @@ func (t *substrMatchTree) matches(cp *contentProvider, cost int, known map[match
979976
for _, m := range t.current {
980977
if m.byteOffset == 0 && m.runeOffset > 0 {
981978
m.byteOffset = cp.findOffset(m.fileName, m.runeOffset)
982-
if cp.err != nil {
983-
return matchesNone
984-
}
985979
}
986980
content := cp.data(m.fileName)
987-
if cp.err != nil {
988-
return matchesNone
989-
}
990981
matched, err := m.matchContent(content)
991982
if err != nil {
992-
cp.setError(err)
993-
return matchesNone
983+
cp.panicCorrupt(err)
994984
}
995985
if matched {
996986
pruned = append(pruned, m)

0 commit comments

Comments
 (0)