Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MB-64604: [BP] Fix interpreting scorch config: "fieldTFRCacheThreshold" #2119

Merged
merged 3 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading