amadan

carlos / platform Public

genselect: the unfinished run's walk passes over covered foreign links and refuses a covered local baseline (joint round 13 R13-1)

Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude d7621a29fbdec7c02dda2d23bd2a3f3811ba9e3e parent 574d946
2 files changed, +134 −10
  • internal/genselect/select.go +16 −8
  • internal/genselect/select_test.go +118 −2

CI passed — run details

diff --git a/internal/genselect/select.go b/internal/genselect/select.go
index 0a9c21368..70e6ad3c8 100644
--- a/internal/genselect/select.go
+++ b/internal/genselect/select.go
@@ -28,6 +28,7 @@ var (
ErrGap = errors.New("genselect: non-contiguous ltx files")
ErrEmpty = errors.New("genselect: no restorable data")
ErrRegion = errors.New("genselect: baseline belongs to another region")
+ ErrCovered = errors.New("genselect: local baseline is covered")
ErrInput = errors.New("genselect: invalid input")
ErrRoot = errors.New("genselect: invalid root")
)
@@ -93,7 +94,10 @@ type Plan struct {
MaxLevel0TXID uint64
}
-type candidate struct{ generation, region string }
+type candidate struct {
+ generation, region string
+ covered bool
+}
func origin(g string) string {
if g == "" {
@@ -243,10 +247,13 @@ func Select(ctx context.Context, s Reader, req Request) (*Plan, error) {
if err != nil {
return nil, err
}
- selected := candidate{markerGen, req.Region}
+ selected := candidate{generation: markerGen, region: req.Region}
for _, c := range run {
if c.region != "" && c.region != req.Region {
- return nil, ErrRegion
+ continue
+ }
+ if c.covered {
+ return nil, ErrCovered
}
if c.generation == markerGen || hasObject(latest, location, c.generation) {
selected = c
@@ -412,9 +419,10 @@ func selectFiles(ctx context.Context, s Reader, bucket, base string, versions []
func unfinished(h hostclaim.Holder, marker, region string) ([]candidate, error) {
gen := origin(h.Generation)
if gen == marker {
- return []candidate{{marker, region}}, nil
+ return []candidate{{generation: marker, region: region}}, nil
}
var out []candidate
+ covered := false
seen := map[string]bool{}
for i := len(h.Admissions) - 1; i >= 0; i-- {
a := h.Admissions[i]
@@ -427,14 +435,14 @@ func unfinished(h hostclaim.Holder, marker, region string) ([]candidate, error)
}
continue
}
- if a.Mode != "cold" || a.Covered || !generation(a.Generation) || seen[gen] {
+ if a.Mode != "cold" || !generation(a.Generation) || seen[gen] {
return nil, ErrDisagreement
}
seen[gen] = true
- out = append(out, candidate{gen, region})
- gen, region = origin(a.Predecessor), a.PredecessorRegion
+ out = append(out, candidate{generation: gen, region: region, covered: covered})
+ gen, region, covered = origin(a.Predecessor), a.PredecessorRegion, a.Covered
if gen == marker {
- return append(out, candidate{gen, region}), nil
+ return append(out, candidate{generation: gen, region: region, covered: covered}), nil
}
if !generation(gen) || region == "" {
return nil, ErrDisagreement
diff --git a/internal/genselect/select_test.go b/internal/genselect/select_test.go
index 82ea7a8c6..64e7a82ca 100644
--- a/internal/genselect/select_test.go
+++ b/internal/genselect/select_test.go
@@ -188,6 +188,123 @@ func (s altered) OpenVersion(ctx context.Context, bucket, key, id string) (io.Re
return s.Reader.OpenVersion(ctx, bucket, key, id)
}
+func TestR13_1UnfinishedRegionalWalk(t *testing.T) {
+ tests := []struct {
+ name string
+ setup func(*fixture) []File
+ gen string
+ want error
+ }{
+ {"interrupted_clean_return", func(f *fixture) []File {
+ return []File{f.pin}
+ }, "g0", nil},
+ {"another_interrupted_admission", func(f *fixture) []File {
+ f.req.Holder.Generation = "R2"
+ f.req.Holder.Admissions = append(f.req.Holder.Admissions,
+ hostclaim.AdmissionRecord{Generation: "R2", Predecessor: "R", Region: "ie", PredecessorRegion: "ie", Mode: "cold"})
+ return []File{f.pin}
+ }, "g0", nil},
+ {"covered_foreign_link_deeper_in_run", func(f *fixture) []File {
+ f.req.Holder.Admissions = []hostclaim.AdmissionRecord{
+ {Generation: "T", Predecessor: "g0", Region: "us", PredecessorRegion: "ie", Mode: "cold"},
+ {Generation: "U", Predecessor: "T", Region: "uk", PredecessorRegion: "us", Mode: "cold", Covered: true},
+ {Generation: "R", Predecessor: "U", Region: "ie", PredecessorRegion: "uk", Mode: "cold"},
+ }
+ return []File{f.pin}
+ }, "g0", nil},
+ {"uncovered_foreign_link", func(f *fixture) []File {
+ f.req.Holder.Admissions[1].Covered = false
+ return []File{f.pin}
+ }, "g0", nil},
+ {"foreign_objects_never_selected", func(f *fixture) []File {
+ f.req.Holder.Admissions[1].Covered = false
+ f.file("T", 9, 1, 99)
+ return []File{f.pin}
+ }, "g0", nil},
+ {"holder_objects_with_covered_foreign_predecessor", func(f *fixture) []File {
+ return []File{f.file("R", 0, 1, 1)}
+ }, "R", nil},
+ {"newest_local_objects", func(f *fixture) []File {
+ f.req.Holder.Generation = "R2"
+ f.req.Holder.Admissions = append(f.req.Holder.Admissions,
+ hostclaim.AdmissionRecord{Generation: "R2", Predecessor: "R", Region: "ie", PredecessorRegion: "ie", Mode: "cold"})
+ return []File{f.file("R", 0, 1, 1)}
+ }, "R", nil},
+ {"unsafe_covered_local_baseline", func(f *fixture) []File {
+ f.req.Holder.Generation = "R2"
+ f.req.Holder.Admissions = append(f.req.Holder.Admissions,
+ hostclaim.AdmissionRecord{Generation: "R2", Predecessor: "R", Region: "ie", PredecessorRegion: "ie", Mode: "cold", Covered: true})
+ f.file("R", 0, 1, 1)
+ return nil
+ }, "", ErrCovered},
+ {"unsafe_covered_empty_local_baseline", func(f *fixture) []File {
+ f.req.Holder.Generation = "R2"
+ f.req.Holder.Admissions = append(f.req.Holder.Admissions,
+ hostclaim.AdmissionRecord{Generation: "R2", Predecessor: "R", Region: "ie", PredecessorRegion: "ie", Mode: "cold", Covered: true})
+ return nil
+ }, "", ErrCovered},
+ {"unsafe_covered_local_marker", func(f *fixture) []File {
+ f.req.Holder.Admissions[0].Covered = true
+ return nil
+ }, "", ErrCovered},
+ {"older_covered_local_not_needed", func(f *fixture) []File {
+ f.req.Holder.Admissions[0].Covered = true
+ return []File{f.file("R", 0, 1, 1)}
+ }, "R", nil},
+ {"covered_warm_session_in_run", func(f *fixture) []File {
+ f.req.Holder.Admissions = append(f.req.Holder.Admissions,
+ hostclaim.AdmissionRecord{Generation: "R", Predecessor: "R", Region: "ie", PredecessorRegion: "ie", Mode: "warm", Covered: true})
+ return []File{f.pin}
+ }, "g0", nil},
+ {"absent_marker_origin", func(f *fixture) []File {
+ f.origin()
+ f.req.Holder.Generation = "R"
+ f.req.Holder.Admissions[0].Predecessor = hostclaim.RootGeneration
+ return []File{f.file("", 0, 1, 1)}
+ }, hostclaim.RootGeneration, nil},
+ {"foreign_predecessor_region_marker_fallback", func(f *fixture) []File {
+ f.chain("g0")
+ f.req.Holder.Admissions[0].PredecessorRegion = "us"
+ return []File{f.pin}
+ }, "g0", nil},
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ f := newFixture(t)
+ f.req.Holder.Generation = "R"
+ f.req.Holder.Admissions = []hostclaim.AdmissionRecord{
+ {Generation: "T", Predecessor: "g0", Region: "us", PredecessorRegion: "ie", Mode: "cold"},
+ {Generation: "R", Predecessor: "T", Region: "ie", PredecessorRegion: "us", Mode: "cold", Coverage: "clean-return", Covered: true},
+ }
+ want := tt.setup(f)
+ before := f.b.Snapshot()
+ holderBefore, _ := json.Marshal(f.req.Holder)
+ got, err := Select(t.Context(), f.store, f.req)
+ holderAfter, _ := json.Marshal(f.req.Holder)
+ if f.b.Snapshot() != before || string(holderAfter) != string(holderBefore) {
+ t.Fatal("selection mutated inputs or storage")
+ }
+ if !errors.Is(err, tt.want) {
+ t.Fatalf("got %+v / %v, want %v", got, err, tt.want)
+ }
+ if tt.want != nil {
+ if got != nil {
+ t.Fatal("partial plan")
+ }
+ return
+ }
+ if got == nil || got.Generation != tt.gen || !reflect.DeepEqual(got.Files, want) ||
+ got.Root == nil || *got.Root != want[0] || got.MaxTXID != want[len(want)-1].MaxTXID ||
+ got.MarkerVersion != f.markerVersion {
+ t.Fatalf("got %+v, want %s %+v", got, tt.gen, want)
+ }
+ if tt.gen == f.marker.Op && (got.Pin == nil || *got.Pin != f.marker.Pin) {
+ t.Fatalf("marker pin not preserved: %+v", got.Pin)
+ }
+ })
+ }
+}
+
func TestRefusals(t *testing.T) {
tests := []struct {
name string
@@ -196,10 +313,9 @@ func TestRefusals(t *testing.T) {
}{
{"marker_holder_disagreement", func(f *fixture) { f.req.Holder.Generation = "unknown" }, ErrDisagreement},
{"nonempty_unexplained_disagreement", func(f *fixture) { f.req.Holder.Generation = "unknown"; f.file("unknown", 0, 1, 1) }, ErrDisagreement},
- {"covered_chain", func(f *fixture) { f.chain("g0"); f.req.Holder.Admissions[0].Covered = true }, ErrDisagreement},
+ {"covered_local_marker", func(f *fixture) { f.chain("g0"); f.req.Holder.Admissions[0].Covered = true }, ErrCovered},
{"broken_chain", func(f *fixture) { f.chain("g0"); f.req.Holder.Admissions[1].Predecessor = "missing" }, ErrDisagreement},
{"cyclic_chain", func(f *fixture) { f.chain("g0"); f.req.Holder.Admissions[0].Predecessor = "g2" }, ErrDisagreement},
- {"foreign_baseline", func(f *fixture) { f.chain("g0"); f.req.Holder.Admissions[0].PredecessorRegion = "us" }, ErrRegion},
{"incomplete_listing", func(f *fixture) { f.b.FailListAfterPages = 1 }, ErrListing},
{"partial_data_with_error", func(f *fixture) {
f.store = altered{Reader: f.b, list: func(vs []lease.ObjectVersion) ([]lease.ObjectVersion, error) { return vs, io.ErrUnexpectedEOF }}