[CELEBORN-1356] Split rpc module into rpc_app and rpc_service#2460
[CELEBORN-1356] Split rpc module into rpc_app and rpc_service#2460mridulm wants to merge 5 commits intoapache:mainfrom
Conversation
|
If it makes it easier to review, I can split it into two - updates to |
|
The test failure seems to be some timeout issue ? this is ready for review. +CC @otterc , @waitinfuture, @FMX , @pan3793 |
FMX
left a comment
There was a problem hiding this comment.
I think split rpc module looks fine to me, but there is no need to introduce something not used currently.
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala
Outdated
Show resolved
Hide resolved
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala
Outdated
Show resolved
Hide resolved
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala
Outdated
Show resolved
Hide resolved
common/src/main/java/org/apache/celeborn/common/protocol/TransportModuleConstants.java
Outdated
Show resolved
Hide resolved
|
code change lgtm, could you pls add an item to the migration guide? |
I was planning to add that as part of https://issues.apache.org/jira/browse/CELEBORN-1353 .. should I be adding it as part of this PR instead ? Edit: added a note in CELEBORN-1353 on migration guide |
|
@mridulm thanks for opening a dedicated ticket for docs, merged to main for 0.5 |
What changes were proposed in this pull request?
Split the
rpctransport module intorpc_appandrpc_serviceto allow for them to be independently configured.Why are the changes needed?
We need the ability to independently configure communication between application components (driver/executors in spark applications) and those to/from Celeborn service (master/workers) components.
This is particularly relevant for TLS support where applications might be running with TLS disabled for their rpc services or using self-signed certificates (see CELEBORN-1354 for an example), while services would have signed certs.
Does this PR introduce any user-facing change?
Yes, it allows users to independently configure rpc env within the application and those to/from services.
Backward compatibility is maintained - and so existing
rpcis the fallback in caserpc_apporrpc_serviceconfig is not found.How was this patch tested?
Unit tests were enhanced, existing tests pass.