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

Optimize endpoint index getter and also rename some methods to be mor… #783

Merged
merged 12 commits into from
Sep 2, 2024

Conversation

hzxuzhonghu
Copy link
Member

@hzxuzhonghu hzxuzhonghu commented Aug 28, 2024

…e readable

What type of PR is this?

/kind cleanup
/kind enhancement

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #775

Previously we iterate all the endpoint map when delete a workload. Now the index are cached in userspace

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


@kmesh-bot kmesh-bot added kind/enhancement New feature or request size/L labels Aug 28, 2024
@hzxuzhonghu hzxuzhonghu changed the title WIP: Optimize endpoint index getter and also rename some methods to be mor… Optimize endpoint index getter and also rename some methods to be mor… Aug 28, 2024
Copy link

codecov bot commented Aug 28, 2024

Codecov Report

Attention: Patch coverage is 74.43182% with 45 lines in your changes missing coverage. Please review.

Project coverage is 52.00%. Comparing base (9db2d28) to head (50b4405).
Report is 21 commits behind head on main.

Files with missing lines Patch % Lines
pkg/controller/workload/workload_processor.go 70.65% 20 Missing and 7 partials ⚠️
pkg/controller/workload/bpfcache/endpoint.go 80.70% 6 Missing and 5 partials ⚠️
pkg/controller/workload/bpfcache/factory.go 66.66% 1 Missing and 1 partial ⚠️
pkg/controller/workload/cache/workload_cache.go 88.23% 1 Missing and 1 partial ⚠️
pkg/controller/workload/workload_controller.go 0.00% 2 Missing ⚠️
pkg/controller/workload/workload_hash.go 50.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
pkg/controller/workload/workload_hash.go 82.45% <50.00%> (ø)
pkg/controller/workload/bpfcache/factory.go 75.00% <66.66%> (-25.00%) ⬇️
pkg/controller/workload/cache/workload_cache.go 67.69% <88.23%> (-7.31%) ⬇️
pkg/controller/workload/workload_controller.go 62.00% <0.00%> (-2.59%) ⬇️
pkg/controller/workload/bpfcache/endpoint.go 84.72% <80.70%> (-5.76%) ⬇️
pkg/controller/workload/workload_processor.go 66.83% <70.65%> (+2.22%) ⬆️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1136f54...50b4405. Read the comment docs.

@hzxuzhonghu
Copy link
Member Author

cc @LiZhenCheng9527

Copy link
Collaborator

@LiZhenCheng9527 LiZhenCheng9527 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

pkg/controller/workload/bpfcache/endpoint.go Show resolved Hide resolved
pkg/controller/workload/workload_processor.go Outdated Show resolved Hide resolved
@@ -330,14 +330,14 @@ func (p *Processor) addNewServicesWithWorkload(workload *workloadapi.Workload, n
return nil
}

log.Infof("addNewServicesWithWorkload: %v", newServices)
backend_uid := p.hashName.StrToNum(workload.GetUid())
log.Infof("handleWorkloadNewBoundServices %s: %v", workload.Uid, newServices)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@@ -90,7 +90,7 @@ func (h *HashName) flushDelta(str string, num uint32) error {
return err
}

func (h *HashName) StrToNum(str string) uint32 {
func (h *HashName) Hash(str string) uint32 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function name seems a bit abstract and easy to confuse.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on the contrary, i think this is consistent with go hash pkg

@kmesh-bot kmesh-bot removed the lgtm label Aug 29, 2024
…e readable

Signed-off-by: Zhonghu Xu <xuzhonghu@huawei.com>
Signed-off-by: Zhonghu Xu <xuzhonghu@huawei.com>
Signed-off-by: Zhonghu Xu <xuzhonghu@huawei.com>
Signed-off-by: Zhonghu Xu <xuzhonghu@huawei.com>
Signed-off-by: Zhonghu Xu <xuzhonghu@huawei.com>
Signed-off-by: Zhonghu Xu <xuzhonghu@huawei.com>
Signed-off-by: Zhonghu Xu <xuzhonghu@huawei.com>
Signed-off-by: Zhonghu Xu <xuzhonghu@huawei.com>
Signed-off-by: Zhonghu Xu <xuzhonghu@huawei.com>
Signed-off-by: Zhonghu Xu <xuzhonghu@huawei.com>
Signed-off-by: Zhonghu Xu <xuzhonghu@huawei.com>
Signed-off-by: Zhonghu Xu <xuzhonghu@huawei.com>
@hzxuzhonghu hzxuzhonghu force-pushed the endpointIndex-optimize branch from 830e9cf to 50b4405 Compare August 31, 2024 06:18
@nlgwcy
Copy link
Contributor

nlgwcy commented Aug 31, 2024

/lgtm
/approve

@kmesh-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nlgwcy

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@nlgwcy nlgwcy enabled auto-merge August 31, 2024 12:52
@hzxuzhonghu hzxuzhonghu disabled auto-merge September 2, 2024 01:13
@hzxuzhonghu hzxuzhonghu merged commit 3bb72e2 into kmesh-net:main Sep 2, 2024
7 of 9 checks passed
@hzxuzhonghu hzxuzhonghu deleted the endpointIndex-optimize branch September 2, 2024 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhance workload deletion processing
6 participants