Skip to content

Commit c26a257

Browse files
Validate scorch routine options when creating or opening a Bleve Index. (#2170)
- If a malformed setting is used in the persistor/merger options, an async error is thrown leading to a crash loop in a live system. - This PR adds a validation step to ensure that the options are valid before creating/opening the index.
1 parent 5040e2d commit c26a257

File tree

2 files changed

+116
-0
lines changed

2 files changed

+116
-0
lines changed

index/scorch/scorch.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,18 @@ func NewScorch(storeName string,
160160
if ok {
161161
rv.onAsyncError = RegistryAsyncErrorCallbacks[aecbName]
162162
}
163+
// validate any custom persistor options to
164+
// prevent an async error in the persistor routine
165+
_, err = rv.parsePersisterOptions()
166+
if err != nil {
167+
return nil, err
168+
}
169+
// validate any custom merge planner options to
170+
// prevent an async error in the merger routine
171+
_, err = rv.parseMergePlannerOptions()
172+
if err != nil {
173+
return nil, err
174+
}
163175

164176
return rv, nil
165177
}

index/scorch/scorch_test.go

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package scorch
1616

1717
import (
1818
"context"
19+
"encoding/json"
1920
"fmt"
2021
"log"
2122
"math/rand"
@@ -2706,3 +2707,106 @@ func BenchmarkAggregateFieldStats(b *testing.B) {
27062707
}
27072708
}
27082709
}
2710+
2711+
func TestPersistorMergerOptions(t *testing.T) {
2712+
type test struct {
2713+
config string
2714+
expectErr bool
2715+
}
2716+
tests := []test{
2717+
{
2718+
// valid config and no error expected
2719+
config: `{
2720+
"scorchPersisterOptions": {
2721+
"persisterNapTimeMSec": 1110,
2722+
"memoryPressurePauseThreshold" : 333
2723+
}
2724+
}`,
2725+
expectErr: false,
2726+
},
2727+
{
2728+
// valid json with invalid config values
2729+
// and error expected
2730+
config: `{
2731+
"scorchPersisterOptions": {
2732+
"persisterNapTimeMSec": "1110",
2733+
"memoryPressurePauseThreshold" : [333]
2734+
}
2735+
}`,
2736+
expectErr: true,
2737+
},
2738+
{
2739+
// valid json with invalid config values
2740+
// and error expected
2741+
config: `{
2742+
"scorchPersisterOptions": {
2743+
"persisterNapTimeMSec": 1110.2,
2744+
"memoryPressurePauseThreshold" : 333
2745+
}
2746+
}`,
2747+
expectErr: true,
2748+
},
2749+
{
2750+
// invalid setting for scorchMergePlanOptions
2751+
config: `{
2752+
"scorchPersisterOptions": {
2753+
"persisterNapTimeMSec": 1110,
2754+
"memoryPressurePauseThreshold" : 333
2755+
},
2756+
"scorchMergePlanOptions": [{
2757+
"maxSegmentSize": 10000,
2758+
"maxSegmentsPerTier": 10,
2759+
"segmentsPerMergeTask": 10
2760+
}]
2761+
}`,
2762+
expectErr: true,
2763+
},
2764+
{
2765+
// valid setting
2766+
config: `{
2767+
"scorchPersisterOptions": {
2768+
"persisterNapTimeMSec": 1110,
2769+
"memoryPressurePauseThreshold" : 333
2770+
},
2771+
"scorchMergePlanOptions": {
2772+
"maxSegmentSize": 10000,
2773+
"maxSegmentsPerTier": 10,
2774+
"segmentsPerMergeTask": 10
2775+
}
2776+
}`,
2777+
expectErr: false,
2778+
},
2779+
{
2780+
config: `{
2781+
"scorchPersisterOptions": {
2782+
"persisterNapTimeMSec": 1110,
2783+
"memoryPressurePauseThreshold" : 333
2784+
},
2785+
"scorchMergePlanOptions": {
2786+
"maxSegmentSize": 5.6,
2787+
"maxSegmentsPerTier": 10,
2788+
"segmentsPerMergeTask": 10
2789+
}
2790+
}`,
2791+
expectErr: true,
2792+
},
2793+
}
2794+
for i, test := range tests {
2795+
cfg := map[string]interface{}{}
2796+
err := json.Unmarshal([]byte(test.config), &cfg)
2797+
if err != nil {
2798+
t.Fatalf("test %d: error unmarshalling config: %v", i, err)
2799+
}
2800+
analysisQueue := index.NewAnalysisQueue(1)
2801+
_, err = NewScorch(Name, cfg, analysisQueue)
2802+
if test.expectErr {
2803+
if err == nil {
2804+
t.Errorf("test %d: expected error, got nil", i)
2805+
}
2806+
} else {
2807+
if err != nil {
2808+
t.Errorf("test %d: unexpected error: %v", i, err)
2809+
}
2810+
}
2811+
}
2812+
}

0 commit comments

Comments
 (0)