openyurt
openyurt copied to clipboard
[WIP]refactor yurthub cache to adapt different storages
Signed-off-by: Congrool [email protected]
What type of PR is this?
/kind enhancement /sig storage
What this PR does / why we need it:
Currently, the cacheManager deeply couple with the diskStorage. In #778 , we'll add a new storage backend for cacheManager, called pool-coordiantor which actually an etcd storage at the view of cacheManager. There're some problems need to be solved before implement this feature:
- the format of key for disk storage is different from that of pool-coordinator, the former is
/component/resource/namespace/name
and the later is/registry/resource/namespace/name
. Also considering that there're some special case for pool-coordinator, such as resourcenodes
should beminions
instead. - For update operation, there's also difference. For disk storage, we have to decode the stored json data and retrieve the rv field, but for pool-coordinator we can directly compare the rv in etcd. Also considering that, the comparision and update should be atomic, so we should not compare the rv in cache manager and update in storage.
- The storage.Store interface is not explict enough. We should define the operation of each function before implementing other storages.
- We should make the in-memory cache at the level of cachemanager, which is used to optimize the cache query effiency, because there're two many kinds that can read/write storage, it's not easy to manage the in-memory cache. But at the cachemanager layer, only CacheResponse will write to the storage and QueryCache will read from the storage, the management of in-memory cache will be much more easier.
Currently, it's for preview and has never been tested.
TODO List:
- [x] revise unit tests
- [x] check the cache ability in actual environment
Proposal: #897
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: Congrool
To complete the pull request process, please assign huangyuqi
You can assign the PR to them by writing /assign @huangyuqi
in a comment when ready.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve
in a comment
Approvers can cancel approval by writing /approve cancel
in a comment
Codecov Report
Merging #882 (d7b5398) into master (775d69d) will increase coverage by
1.14%
. The diff coverage is63.90%
.
@@ Coverage Diff @@
## master #882 +/- ##
==========================================
+ Coverage 50.78% 51.92% +1.14%
==========================================
Files 96 98 +2
Lines 12795 13371 +576
==========================================
+ Hits 6498 6943 +445
- Misses 5754 5846 +92
- Partials 543 582 +39
Flag | Coverage Δ | |
---|---|---|
unittests | 51.92% <63.90%> (+1.14%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
pkg/yurthub/certificate/hubself/fake_cert_mgr.go | 0.00% <0.00%> (ø) |
|
pkg/yurthub/filter/servicetopology/filter.go | 0.00% <0.00%> (ø) |
|
pkg/yurthub/healthchecker/node_lease.go | 91.00% <ø> (ø) |
|
pkg/yurthub/proxy/remote/remote.go | 1.35% <0.00%> (ø) |
|
pkg/yurthub/util/util.go | 80.15% <ø> (+2.22%) |
:arrow_up: |
pkg/yurthub/certificate/hubself/cert_mgr.go | 20.43% <6.66%> (+0.38%) |
:arrow_up: |
pkg/yurthub/proxy/local/local.go | 70.13% <33.33%> (ø) |
|
pkg/yurthub/proxy/util/util.go | 80.07% <45.83%> (-7.55%) |
:arrow_down: |
pkg/yurthub/healthchecker/health_checker.go | 79.86% <52.00%> (-6.09%) |
:arrow_down: |
pkg/yurthub/kubernetes/meta/restmapper.go | 74.07% <55.17%> (-2.40%) |
:arrow_down: |
... and 13 more |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
I reconsider the timing when merge this pr. It seems more work need to be done than original thought. Is it better to merge this pr after v1.0?
Meanwhile, I'll continue my work about autonomy tests, #778 and #857 based on this pr.
I've rebased the pr of autonomy e2e tests, and seems that all tests can pass. I think it's ready for review, PTAL @rambohe-ch And the proposal #897 has been updated, motivations of most modifications are described in it.
Ensure the compability with old key format.
The disk storage will use old key format if it detects there's old key cache under the cache dir.
Users needs to manually delete all old cache before updating the yurthub, if they want to use the enhancement mode of yurthub which uses /<component>/<resource.version.group>/<namespace>/<name>
as key format.
@rambohe-ch PTAL
/lgtm /approve
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: Congrool, rambohe-ch
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [rambohe-ch]
Approvers can indicate their approval by writing /approve
in a comment
Approvers can cancel approval by writing /approve cancel
in a comment