Skip to content

[CELEBORN-1594] Refine dynamicConfig template and prevent NPE#2737

Closed
turboFei wants to merge 6 commits intoapache:mainfrom
turboFei:dynamic_config
Closed

[CELEBORN-1594] Refine dynamicConfig template and prevent NPE#2737
turboFei wants to merge 6 commits intoapache:mainfrom
turboFei:dynamic_config

Conversation

@turboFei
Copy link
Member

@turboFei turboFei commented Sep 13, 2024

What changes were proposed in this pull request?

  1. seems the quota.yaml is outdated, remove it
  2. the config item set in dynamicConfig.yaml.template is not dynamic config, remove the one
  3. prevent NPE when loading dynamicConfig.yaml

Why are the changes needed?

As title.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing UT.

@turboFei turboFei changed the title Dynamic config [CELEBORN-1594] Remove unused conf template Sep 13, 2024
#
- level: SYSTEM
config:
celeborn.worker.directMemoryRatioToPauseReceive: 0.75
Copy link
Member Author

Choose a reason for hiding this comment

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

seems this config item is not dynamic, I wonder whether it works now

Copy link
Member Author

Choose a reason for hiding this comment

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

image

Currently, there are only 4 dynamic configs for quota management.

Copy link
Contributor

Choose a reason for hiding this comment

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

seems this config item is not dynamic, I wonder whether it works now

yes,seems can be dynamic,but not supported yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

Raised ticket to support more dynamic configs: https://issues.apache.org/jira/browse/CELEBORN-1598

@turboFei turboFei changed the title [CELEBORN-1594] Remove unused conf template [CELEBORN-1594] Remove unused conf template and prevent NPE Sep 13, 2024
@turboFei turboFei changed the title [CELEBORN-1594] Remove unused conf template and prevent NPE [CELEBORN-1594] Remove unused conf template and prevent NPE with empty dynamic config Sep 13, 2024
@turboFei turboFei changed the title [CELEBORN-1594] Remove unused conf template and prevent NPE with empty dynamic config [CELEBORN-1594] Remove unused conf template and prevent NPE Sep 13, 2024
@turboFei turboFei changed the title [CELEBORN-1594] Remove unused conf template and prevent NPE [CELEBORN-1594] Refine dynamicConfig template and prevent NPE Sep 13, 2024
Copy link
Member

@SteNicholas SteNicholas left a comment

Choose a reason for hiding this comment

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

LGTM.

@RexXiong RexXiong closed this in d335c64 Sep 15, 2024
RexXiong pushed a commit that referenced this pull request Sep 15, 2024
### What changes were proposed in this pull request?

1. seems the quota.yaml is outdated, remove it
2. the config item set in dynamicConfig.yaml.template is not dynamic config, remove the one
3. prevent  NPE when loading dynamicConfig.yaml

### Why are the changes needed?
As title.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Existing UT.

Closes #2737 from turboFei/dynamic_config.

Authored-by: Wang, Fei <fwang12@ebay.com>
Signed-off-by: Shuang <lvshuang.xjs@alibaba-inc.com>
(cherry picked from commit d335c64)
Signed-off-by: Shuang <lvshuang.xjs@alibaba-inc.com>
@RexXiong
Copy link
Contributor

Thanks. merge to main(v0.6.0) and branch-0.5(v0.5.2)

s0nskar pushed a commit to s0nskar/celeborn that referenced this pull request Sep 16, 2024
### What changes were proposed in this pull request?

1. seems the quota.yaml is outdated, remove it
2. the config item set in dynamicConfig.yaml.template is not dynamic config, remove the one
3. prevent  NPE when loading dynamicConfig.yaml

### Why are the changes needed?
As title.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Existing UT.

Closes apache#2737 from turboFei/dynamic_config.

Authored-by: Wang, Fei <fwang12@ebay.com>
Signed-off-by: Shuang <lvshuang.xjs@alibaba-inc.com>
wankunde pushed a commit to wankunde/celeborn that referenced this pull request Oct 11, 2024
### What changes were proposed in this pull request?

1. seems the quota.yaml is outdated, remove it
2. the config item set in dynamicConfig.yaml.template is not dynamic config, remove the one
3. prevent  NPE when loading dynamicConfig.yaml

### Why are the changes needed?
As title.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Existing UT.

Closes apache#2737 from turboFei/dynamic_config.

Authored-by: Wang, Fei <fwang12@ebay.com>
Signed-off-by: Shuang <lvshuang.xjs@alibaba-inc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants