Skip to content

Commit

Permalink
MB-64604: [BP] Fix interpreting scorch config: "fieldTFRCacheThreshol…
Browse files Browse the repository at this point in the history
…d" (#2119)

+ Setting the default to 0 on account of the panics caught in the MB.
+ Firstly, refactor FieldTFRCacheThreshold to fieldTFRCacheThreshold for
some naming consistency here.
+ This threshold can be used to toggle recycling of TermFieldReaders
on/off.
+ Couchbase users will have the ability to provide this setting within a
JSON payload, which when interpreted into a map[string]interface{} will
need to be interpreted as a float64.
+ Should library users set it as an int within the index config - we'll
honor that setting as well.
  • Loading branch information
abhinavdangeti authored Dec 17, 2024
1 parent fdaed7b commit 791ecd4
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 14 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ jobs:
test:
strategy:
matrix:
go-version: [1.16.x, 1.17.x, 1.18.x]
go-version: [1.20.x, 1.21.x, 1.22.x]
platform: [ubuntu-latest, macos-latest, windows-latest]
runs-on: ${{ matrix.platform }}
steps:
Expand Down
33 changes: 20 additions & 13 deletions index/scorch/snapshot_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,6 @@ type asynchSegmentResult struct {

var reflectStaticSizeIndexSnapshot int

// DefaultFieldTFRCacheThreshold limits the number of TermFieldReaders(TFR) for
// a field in an index snapshot. Without this limit, when recycling TFRs, it is
// possible that a very large number of TFRs may be added to the recycle
// cache, which could eventually lead to significant memory consumption.
// This threshold can be overwritten by users at the library level by changing the
// exported variable, or at the index level by setting the FieldTFRCacheThreshold
// in the kvConfig.
var DefaultFieldTFRCacheThreshold uint64 = 10

func init() {
var is interface{} = IndexSnapshot{}
reflectStaticSizeIndexSnapshot = int(reflect.TypeOf(is).Size())
Expand Down Expand Up @@ -567,10 +558,26 @@ func (i *IndexSnapshot) allocTermFieldReaderDicts(field string) (tfr *IndexSnaps
}
}

func (i *IndexSnapshot) getFieldTFRCacheThreshold() uint64 {
// DefaultFieldTFRCacheThreshold limits the number of TermFieldReaders(TFR) for
// a field in an index snapshot. Without this limit, when recycling TFRs, it is
// possible that a very large number of TFRs may be added to the recycle
// cache, which could eventually lead to significant memory consumption.
// This threshold can be overwritten by users at the library level by changing the
// exported variable, or at the index level by setting the "fieldTFRCacheThreshold"
// in the kvConfig.
var DefaultFieldTFRCacheThreshold int = 0 // disabled because it causes MB-64604

func (i *IndexSnapshot) getFieldTFRCacheThreshold() int {
if i.parent.config != nil {
if _, ok := i.parent.config["FieldTFRCacheThreshold"]; ok {
return i.parent.config["FieldTFRCacheThreshold"].(uint64)
if val, exists := i.parent.config["fieldTFRCacheThreshold"]; exists {
if x, ok := val.(float64); ok {
// JSON unmarshal-ed into a map[string]interface{} will default
// to float64 for numbers, so we need to check for float64 first.
return int(x)
} else if x, ok := val.(int); ok {
// If library users provided an int in the config, we'll honor it.
return x
}
}
}
return DefaultFieldTFRCacheThreshold
Expand All @@ -597,7 +604,7 @@ func (i *IndexSnapshot) recycleTermFieldReader(tfr *IndexSnapshotTermFieldReader
if i.fieldTFRs == nil {
i.fieldTFRs = map[string][]*IndexSnapshotTermFieldReader{}
}
if uint64(len(i.fieldTFRs[tfr.field])) < i.getFieldTFRCacheThreshold() {
if len(i.fieldTFRs[tfr.field]) < i.getFieldTFRCacheThreshold() {
i.fieldTFRs[tfr.field] = append(i.fieldTFRs[tfr.field], tfr)
}
i.m2.Unlock()
Expand Down

0 comments on commit 791ecd4

Please sign in to comment.