Skip to content

Commit e8d4425

Browse files
authored
MB-66112: fix an edge case related to index convergence (#2200)
- As an edge case, the segment layout can be such that the merge planning generates tasks having only a single non-empty segment. - When this task is processed and introduced into the scorch system, the root doesn't change and you'd be stuck in an infinite loop because the same segment is still present in the system and the plan keeps generating the task with the same single segment, which doesn't converge the index to a steady state.
1 parent 2fea10b commit e8d4425

File tree

2 files changed

+47
-2
lines changed

2 files changed

+47
-2
lines changed

index/scorch/mergeplan/merge_plan.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,10 @@ func plan(segmentsIn []Segment, o *MergePlanOptions) (*MergePlan, error) {
295295
if len(bestRoster) == 0 {
296296
return rv, nil
297297
}
298-
299-
rv.Tasks = append(rv.Tasks, &MergeTask{Segments: bestRoster})
298+
// create tasks with valid merges - i.e. there should be atleast 2 non-empty segments
299+
if len(bestRoster) > 1 {
300+
rv.Tasks = append(rv.Tasks, &MergeTask{Segments: bestRoster})
301+
}
300302

301303
eligibles = removeSegments(eligibles, bestRoster)
302304
}

index/scorch/mergeplan/merge_plan_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,3 +719,46 @@ func TestPlanMaxSegmentFileSize(t *testing.T) {
719719
})
720720
}
721721
}
722+
723+
func TestSingleTaskMergePlan(t *testing.T) {
724+
o := &DefaultMergePlanOptions
725+
o.FloorSegmentFileSize = 209715200
726+
727+
// borrowing the spec values from MB-66112
728+
//
729+
// both segments are eligible, but the roster with a single segment is scored
730+
// higher than the roster with two segments
731+
// in this case the merge plan returns task with a single segment non-empty
732+
// segment which when introduced into the scorch system doesn't cause any change
733+
// and you'd be stuck in an infinite loop where the plan keeps generating the
734+
// same task with the same single segment, which doesn't converge the index to
735+
// a steady state
736+
spec := testCyclesSpec{
737+
descrip: "mssswdbm",
738+
verbose: os.Getenv("VERBOSE") == "mssswdbm" || os.Getenv("VERBOSE") == "y",
739+
o: o,
740+
segments: []Segment{
741+
&segment{
742+
MyId: 2,
743+
MyFullSize: 78059,
744+
MyLiveSize: 78059,
745+
MyFileSize: 129475914,
746+
},
747+
&segment{
748+
MyId: 1,
749+
MyFullSize: 3959,
750+
MyLiveSize: 3959,
751+
MyFileSize: 24805725,
752+
},
753+
},
754+
}
755+
756+
plan, err := Plan(spec.segments, spec.o)
757+
if err != nil {
758+
t.Fatalf("Plan failed, err: %v", err)
759+
}
760+
761+
if len(plan.Tasks) > 0 {
762+
t.Fatalf("expected 0 tasks, got: %d", len(plan.Tasks))
763+
}
764+
}

0 commit comments

Comments
 (0)