-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
bindinfo: add warning message when the memory usage of the cache exceeds its capacity #32866
Merged
Merged
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
3f152ae
bindinfo: add warning message when the memory usage of the binding_ca…
Reminiscent 0062a52
Merge branch 'master' of github.com:pingcap/tidb into addWarning4bind…
Reminiscent 7b37080
add more test cases
Reminiscent a40f644
remove useless line
Reminiscent 10bf341
fix ut
Reminiscent 03f6b3a
Merge branch 'master' of github.com:pingcap/tidb into addWarning4bind…
Reminiscent 538182c
Merge branch 'master' of github.com:pingcap/tidb into addWarning4bind…
Reminiscent fa9ef08
rename TiDBMemQuotaBindCache to TiDBMemQuotaBindingCache
Reminiscent 0dfbe1a
fix ut
Reminiscent 3a1ea99
fix ut
Reminiscent 8a09095
Merge branch 'master' of github.com:pingcap/tidb into addWarning4bind…
Reminiscent 1da4ed2
address comments
Reminiscent ae595fb
Merge branch 'master' of github.com:pingcap/tidb into addWarning4bind…
Reminiscent 7b1a3f7
fix ut
Reminiscent 7fd22f7
fix ut
Reminiscent 1094fd3
Merge branch 'master' of github.com:pingcap/tidb into addWarning4bind…
Reminiscent 985f3af
address comments
Reminiscent afe84a7
address comments
Reminiscent 77598b4
Merge branch 'master' of github.com:pingcap/tidb into addWarning4bind…
Reminiscent 3691299
address comments
Reminiscent 5525271
Merge branch 'master' of github.com:pingcap/tidb into addWarning4bind…
Reminiscent 6ddf3b2
Merge branch 'master' into addWarning4bindCache
ti-chi-bot c08a031
fix ut
Reminiscent 3ecd695
Merge branch 'master' of github.com:pingcap/tidb into addWarning4bind…
Reminiscent 9cdbb67
Merge remote-tracking branch 'origin/addWarning4bindCache' into addWa…
Reminiscent 43db335
Merge branch 'master' into addWarning4bindCache
ti-chi-bot ee3f7ef
Merge branch 'master' into addWarning4bindCache
ti-chi-bot 8ef10da
Merge branch 'master' of github.com:pingcap/tidb into addWarning4bind…
Reminiscent 8e209b5
fix ut
Reminiscent 4425015
Merge remote-tracking branch 'origin/addWarning4bindCache' into addWa…
Reminiscent File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -143,14 +143,18 @@ func (h *BindHandle) Update(fullLoad bool) (err error) { | |
return err | ||
} | ||
|
||
newCache := h.bindInfo.Value.Load().(*bindCache).Copy() | ||
newCache, memExceededErr := h.bindInfo.Value.Load().(*bindCache).Copy() | ||
defer func() { | ||
h.bindInfo.lastUpdateTime = lastUpdateTime | ||
h.bindInfo.Value.Store(newCache) | ||
h.bindInfo.Unlock() | ||
}() | ||
|
||
for _, row := range rows { | ||
// If the memory usage of the binding_cache exceeds its capacity, we will break and do not handle. | ||
if memExceededErr != nil { | ||
break | ||
} | ||
// Skip the builtin record which is designed for binding synchronization. | ||
if row.GetString(0) == BuiltinPseudoSQL4BindLock { | ||
continue | ||
|
@@ -171,12 +175,21 @@ func (h *BindHandle) Update(fullLoad bool) (err error) { | |
oldRecord := newCache.GetBindRecord(hash, meta.OriginalSQL, meta.Db) | ||
newRecord := merge(oldRecord, meta).removeDeletedBindings() | ||
if len(newRecord.Bindings) > 0 { | ||
newCache.SetBindRecord(hash, newRecord) | ||
err = newCache.SetBindRecord(hash, newRecord) | ||
if err != nil { | ||
memExceededErr = err | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we break this loop if meet this error? |
||
} | ||
} else { | ||
newCache.RemoveBindRecord(hash, newRecord) | ||
} | ||
updateMetrics(metrics.ScopeGlobal, oldRecord, newCache.GetBindRecord(hash, meta.OriginalSQL, meta.Db), true) | ||
} | ||
if memExceededErr != nil { | ||
// When the memory capacity of bing_cache is not enough, | ||
// there will be some memory-related errors in multiple places. | ||
// Only needs to be handled once. | ||
logutil.BgLogger().Warn("[sql-bind] ", zap.Error(err)) | ||
} | ||
return nil | ||
} | ||
|
||
|
@@ -580,27 +593,43 @@ func (h *BindHandle) newBindRecord(row chunk.Row) (string, *BindRecord, error) { | |
// setBindRecord sets the BindRecord to the cache, if there already exists a BindRecord, | ||
// it will be overridden. | ||
func (h *BindHandle) setBindRecord(hash string, meta *BindRecord) { | ||
newCache := h.bindInfo.Value.Load().(*bindCache).Copy() | ||
newCache, err0 := h.bindInfo.Value.Load().(*bindCache).Copy() | ||
if err0 != nil { | ||
logutil.BgLogger().Warn("[sql-bind] ", zap.Error(err0)) | ||
} | ||
oldRecord := newCache.GetBindRecord(hash, meta.OriginalSQL, meta.Db) | ||
newCache.SetBindRecord(hash, meta) | ||
err1 := newCache.SetBindRecord(hash, meta) | ||
if err1 != nil && err0 == nil { | ||
logutil.BgLogger().Warn("[sql-bind] ", zap.Error(err1)) | ||
} | ||
h.bindInfo.Value.Store(newCache) | ||
updateMetrics(metrics.ScopeGlobal, oldRecord, meta, false) | ||
} | ||
|
||
// appendBindRecord addes the BindRecord to the cache, all the stale BindRecords are | ||
// removed from the cache after this operation. | ||
func (h *BindHandle) appendBindRecord(hash string, meta *BindRecord) { | ||
newCache := h.bindInfo.Value.Load().(*bindCache).Copy() | ||
newCache, err0 := h.bindInfo.Value.Load().(*bindCache).Copy() | ||
if err0 != nil { | ||
logutil.BgLogger().Warn("[sql-bind] ", zap.Error(err0)) | ||
} | ||
oldRecord := newCache.GetBindRecord(hash, meta.OriginalSQL, meta.Db) | ||
newRecord := merge(oldRecord, meta) | ||
newCache.SetBindRecord(hash, newRecord) | ||
err1 := newCache.SetBindRecord(hash, newRecord) | ||
if err1 != nil && err0 == nil { | ||
// Only need to handle the error once. | ||
logutil.BgLogger().Warn("[sql-bind] ", zap.Error(err1)) | ||
} | ||
h.bindInfo.Value.Store(newCache) | ||
updateMetrics(metrics.ScopeGlobal, oldRecord, newRecord, false) | ||
} | ||
|
||
// removeBindRecord removes the BindRecord from the cache. | ||
func (h *BindHandle) removeBindRecord(hash string, meta *BindRecord) { | ||
newCache := h.bindInfo.Value.Load().(*bindCache).Copy() | ||
newCache, err := h.bindInfo.Value.Load().(*bindCache).Copy() | ||
if err != nil { | ||
logutil.BgLogger().Warn("[sql-bind] ", zap.Error(err)) | ||
} | ||
oldRecord := newCache.GetBindRecord(hash, meta.OriginalSQL, meta.Db) | ||
newCache.RemoveBindRecord(hash, meta) | ||
h.bindInfo.Value.Store(newCache) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bug here, But for now, the return value has not been used yet.