openyurt icon indicating copy to clipboard operation
openyurt copied to clipboard

[WIP]refactor yurthub cache to adapt different storages

Open Congrool opened this issue 2 years ago • 6 comments

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:

  1. 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 resource nodes should be minions instead.
  2. 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.
  3. The storage.Store interface is not explict enough. We should define the operation of each function before implementing other storages.
  4. 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.

Congrool avatar Jun 14 '22 15:06 Congrool

Currently, it's for preview and has never been tested.

Congrool avatar Jun 14 '22 15:06 Congrool

TODO List:

  • [x] revise unit tests
  • [x] check the cache ability in actual environment

Congrool avatar Jun 14 '22 16:06 Congrool

Proposal: #897

Congrool avatar Jun 27 '22 04:06 Congrool

[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.

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

openyurt-bot avatar Jul 17 '22 12:07 openyurt-bot

Codecov Report

Merging #882 (d7b5398) into master (775d69d) will increase coverage by 1.14%. The diff coverage is 63.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.

codecov[bot] avatar Jul 17 '22 16:07 codecov[bot]

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.

Congrool avatar Jul 25 '22 13:07 Congrool

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.

Congrool avatar Nov 04 '22 02:11 Congrool

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

Congrool avatar Nov 15 '22 06:11 Congrool

/lgtm /approve

rambohe-ch avatar Nov 21 '22 07:11 rambohe-ch

[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

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

openyurt-bot avatar Nov 21 '22 07:11 openyurt-bot