You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "leixm (via GitHub)" <gi...@apache.org> on 2024/10/16 08:59:37 UTC
[PR] [CELEBORN-1487][Phase2] CongestionController support dynamic config [celeborn]
leixm opened a new pull request, #2817:
URL: https://github.com/apache/celeborn/pull/2817
### What changes were proposed in this pull request?
CongestionController support dynamic config
### Why are the changes needed?
Currently, Celeborn only supports quota management based on disk file bytes/count, and this quota management cannot cope with sudden increases in traffic, which will cause corrupt to the cluster.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
UT.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1487][Phase2] CongestionController support dynamic config [celeborn]
Posted by "leixm (via GitHub)" <gi...@apache.org>.
leixm commented on PR #2817:
URL: https://github.com/apache/celeborn/pull/2817#issuecomment-2416298575
Can you help review? @RexXiong
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1487][Phase2] CongestionController support dynamic config [celeborn]
Posted by "RexXiong (via GitHub)" <gi...@apache.org>.
RexXiong closed pull request #2817: [CELEBORN-1487][Phase2] CongestionController support dynamic config
URL: https://github.com/apache/celeborn/pull/2817
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1487][Phase2] CongestionController support dynamic config [celeborn]
Posted by "leixm (via GitHub)" <gi...@apache.org>.
leixm commented on code in PR #2817:
URL: https://github.com/apache/celeborn/pull/2817#discussion_r1805728205
##########
worker/src/main/java/org/apache/celeborn/service/deploy/worker/congestcontrol/CongestionController.java:
##########
@@ -332,4 +340,17 @@ public UserCongestionControlContext getUserCongestionContext(UserIdentifier user
public BufferStatusHub getConsumedBufferStatusHub() {
return consumedBufferStatusHub;
}
+
+ private void updateQuota(ConfigService configService) {
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1487][Phase2] CongestionController support dynamic config [celeborn]
Posted by "RexXiong (via GitHub)" <gi...@apache.org>.
RexXiong commented on code in PR #2817:
URL: https://github.com/apache/celeborn/pull/2817#discussion_r1805698628
##########
worker/src/main/java/org/apache/celeborn/service/deploy/worker/congestcontrol/CongestionController.java:
##########
@@ -332,4 +340,17 @@ public UserCongestionControlContext getUserCongestionContext(UserIdentifier user
public BufferStatusHub getConsumedBufferStatusHub() {
return consumedBufferStatusHub;
}
+
+ private void updateQuota(ConfigService configService) {
Review Comment:
nit: configService is already a member variable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1487][Phase2] CongestionController support dynamic config [celeborn]
Posted by "RexXiong (via GitHub)" <gi...@apache.org>.
RexXiong commented on code in PR #2817:
URL: https://github.com/apache/celeborn/pull/2817#discussion_r1804343120
##########
service/src/main/java/org/apache/celeborn/server/common/service/config/ConfigService.java:
##########
@@ -108,6 +109,13 @@ default DynamicConfig getTenantUserConfigFromCache(String tenantId, String userI
*/
void refreshCache() throws IOException;
+ /**
+ * Registers a listener to be called when the configuration is updated.
+ *
+ * @param listener the listener to be registered
+ */
+ void registerListenerOnConfigUpdate(Consumer<ConfigService> listener);
Review Comment:
If we don't have a listener interface, perhaps using Runnable would be more suitable, as we wouldn't need to accept a ConfigService.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org