fix(search): contain faults while copying results - #1126
Conversation
keegancsmith
left a comment
keegancsmith
left a comment
There was a problem hiding this comment.
I've never seen this happen in practice. Are you seeing it cause of NFS? I wonder if there is a more fundamental change we can make such that we can treat the shards as proper immutable blobs that we can do a bit of validation on when we load it up.
I have a feeling for a normal FS if we successfully find a match, this part of the code should continue to work.
|
|
||
| logShardCrash("copy", shard, q, recovered, stack) | ||
| ss.shardRepairs.schedule(shard) | ||
| return |
There was a problem hiding this comment.
a repo can be in multiple shards.
There was a problem hiding this comment.
Right. I removed repository-based attribution. Each shard result now records one range covering its mmap-backed match pointers; the final copy resolves the exact searched shard after aggregation. Overlapping ranges are treated as ambiguous and do not repair either shard. There is a regression with the same repository present in two shards.
| sr.Stats.Crashes++ | ||
| metricSearchCrashesTotal.Inc() | ||
| if onFault != nil { | ||
| onFault(file, recovered, stack) | ||
| } | ||
| continue |
There was a problem hiding this comment.
definition of Stats.Crashes is the number of shards that crashed, this will increment per file that failed.
There was a problem hiding this comment.
Agreed. Crash deduplication now lives in the request-scoped provenance tracker, so multiple bad matches and multiple StreamSearch repository events from one shard contribute exactly one crash and one metric increment. Both cases are covered.
95b2a54 to
6b4b6c4
Compare
|
Yes, the observed failure is on AWS EFS over NFSv4.1; #1109 has the sequence and two-client control. I agree immutable generation paths are the root fix. Load-time validation cannot catch a shard whose backing object is invalidated later. This PR only contains the remaining copy window while publication lifetime is handled separately. |
6b4b6c4 to
8c40515
Compare
2 participants
Summary
FileMatchcopy after the per-shard search guard has returnedThis is stacked on #1125. The guard stays at the delayed-copy boundary, after global result limiting. A limited-result benchmark remained within low-single-digit percentage overhead.
Test plan
go test ./...go test -race ./searchgo vet ./search