[enhancement]optimize approver of data filtering framework in yurthub #790
Conversation
|
@rambohe-ch: GitHub didn't allow me to assign the following users: your_reviewer. Note that only openyurtio members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@yingjianjian PTAL |
rambohe-ch
force-pushed
the
optimize-approver
branch
from
44b9bf6
to
9635f57
Compare
rambohe-ch
force-pushed
the
optimize-approver
branch
from
9635f57
to
4adc48d
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rambohe-ch, yingjianjian The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
| @@ -42,7 +42,7 @@ jobs: | |||
| with: | |||
| go-version: ${{ env.GO_VERSION }} | |||
| - name: Lint golang code | |||
| uses: golangci/golangci-lint-action@v2 | |||
| uses: golangci/golangci-lint-action@v3 | |||
There was a problem hiding this comment.
ok, i will rebase this pull request after #792 merged.
| defer a.Unlock() | ||
| // remove current user setting from reqKeyToName and left default setting only | ||
| for key := range a.reqKeyToName { | ||
| if _, ok := defaultReqKeyToName[key]; !ok { |
There was a problem hiding this comment.
This code may be som confuse. Delete not default keys mean only use default keys.
newReqKey := map[string]string{}
for key, name := range defaultReqKeyToName {
newReqKey[key] = name
}
for key, name := range keyToNameSetting {
// if filter setting is duplicated, only recognize the first setting.
if currentName, ok := newReqKey[key]; !ok {
newReqKey[key] = name
} else {
klog.Warningf("request %s has already used filter %s, so skip filter %s setting", key, currentName, name)
}
}
a.Lock()
defer a.Unlock()
a.reqKeyToName = newReqKey
| defer a.Unlock() | ||
| // remove current user setting from reqKeyToName and left default setting only | ||
| for key := range a.reqKeyToName { | ||
| if _, ok := defaultReqKeyToName[key]; !ok { |
There was a problem hiding this comment.
This code may be som confuse. Delete not default keys mean only use default keys.
newReqKey := map[string]string{}
for key, name := range defaultReqKeyToName {
newReqKey[key] = name
}
for key, name := range keyToNameSetting {
// if filter setting is duplicated, only recognize the first setting.
if currentName, ok := newReqKey[key]; !ok {
newReqKey[key] = name
} else {
klog.Warningf("request %s has already used filter %s, so skip filter %s setting", key, currentName, name)
}
}
a.Lock()
defer a.Unlock()
a.reqKeyToName = newReqKeyThere was a problem hiding this comment.
@huiwq1990 In order to increase efficiency of golang runtime GC, so i don't want to create a new map every time when yurt-hub-cfg filter setting changed. and the new reqKeyToName is based defaultReqKeyToName plus current filter setting in yurt-hub-cfg configmap.
|
Fixes #773 |
What type of PR is this?
/kind enhancement
What this PR does / why we need it:
approve() funcby iteratingnameToRequests, and the efficiency isN(the length of nameToRequests slice), in this pull request, optimize the efficiency toLog(N)by using areqKeyToNamemap to decide need filter or not.yurt-hub-cfgconfigmap more freely. for example: no filter setting inyurt-hub-cfgconfigmap will not effect the default filter setting.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?
other Note