[#1711] feat(server): Introduce the reconfigurable conf #1712
Conversation
|
I drafted this impl, please take a look. @jerqi |
common/src/main/java/org/apache/uniffle/common/ReconfigurableConfManager.java
Outdated
Show resolved
Hide resolved
zuston
changed the title
| @@ -569,7 +573,7 @@ public <T> RssConf set(ConfigOption<T> option, T value) { | |||
| return this; | |||
| } | |||
|
|
|||
| <T> void setValueInternal(String key, T value) { | |||
| public <T> void setValueInternal(String key, T value) { | |||
There was a problem hiding this comment.
This method should be used in the class inner from the name. Could we avoid using it?
There was a problem hiding this comment.
I don't use this, let me recover.
| } | ||
| } | ||
|
|
||
| private boolean isSame(Object v1, Object v2) { |
| @@ -57,6 +57,10 @@ public Set<String> getKeySet() { | |||
| return Sets.newHashSet(); | |||
| } | |||
|
|
|||
| public Object getObject(String key, Object defaultValue) { | |||
There was a problem hiding this comment.
Used by reconfigurableManager to check whether the prev value is same with the latest value, and then output the log if they are different.
There was a problem hiding this comment.
I don't see that this method is called this method in any place.
There was a problem hiding this comment.
My fault. I forgot to remove this after refactor.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1712 +/- ##
============================================
- Coverage 54.86% 54.13% -0.74%
- Complexity 2358 2928 +570
============================================
Files 368 432 +64
Lines 16379 23398 +7019
Branches 1504 2186 +682
============================================
+ Hits 8986 12666 +3680
- Misses 6862 9958 +3096
- Partials 531 774 +243 ☔ View full report in Codecov by Sentry. |
|
Merged. Thanks @jerqi review |
…#1712) ### What changes were proposed in this pull request? Introduce the reconfigurable conf for server or other componts. ### Why are the changes needed? Fix: apache#1711 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Unit tests.
What changes were proposed in this pull request?
Introduce the reconfigurable conf for server or other componts.
Why are the changes needed?
Fix: #1711
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Unit tests.