feat(common): LocalSystemParamManager for worker node
#8153
Conversation
Codecov Report
@@ Coverage Diff @@
## main #8153 +/- ##
=======================================
Coverage 71.63% 71.64%
=======================================
Files 1132 1133 +1
Lines 182213 182249 +36
=======================================
+ Hits 130536 130575 +39
+ Misses 51677 51674 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
|
||
| impl LocalSystemParamManager { | ||
| pub fn new(params: SystemParamsReader) -> Self { | ||
| let params = Arc::new(ArcSwap::from_pointee(params)); |
There was a problem hiding this comment.
Using Arc alone should be enough here?
There was a problem hiding this comment.
When the latest params arrive at the worker though notification service, the local manager needs a thread-safe way to update params, but Arc does not provide that functionality 🤔
There was a problem hiding this comment.
How will the subscribers get the param updates, through the watch channel to get the updates or simply hold a Arc<ArcSwap>? For the latter way, this sounds reasonable to me.
Oh, the PR body has elaborated on this. 😄
|
|
||
| impl LocalSystemParamManager { | ||
| pub fn new(params: SystemParamsReader) -> Self { | ||
| let params = Arc::new(ArcSwap::from_pointee(params)); |
There was a problem hiding this comment.
How will the subscribers get the param updates, through the watch channel to get the updates or simply hold a Arc<ArcSwap>? For the latter way, this sounds reasonable to me.
Oh, the PR body has elaborated on this. 😄
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
System params on worker nodes have 2 read patterns:
LocalSystemParamManagerprovides 2 interfaces for them respectivelyget_paramssubscribe_paramsChecklist For Contributors
./risedev check(or alias,./risedev c)Checklist For Reviewers
Documentation
Click here for Documentation
Types of user-facing changes
Please keep the types that apply to your changes, and remove the others.
Release note