SteNicholas commented on code in PR #2471:
URL: https://github.com/apache/celeborn/pull/2471#discussion_r1573838324


##########
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala:
##########
@@ -87,13 +87,22 @@ class CelebornConf(loadDefaults: Boolean) extends Cloneable 
with Logging with Se
     this
   }
 
+  private def warnIfInternalTransportModule(module: String, key: String): Unit 
= {
+    // for now, assert false to test

Review Comment:
   Could this add the TODO comment for this?



##########
common/src/main/java/org/apache/celeborn/common/protocol/TransportModuleConstants.java:
##########
@@ -24,11 +24,21 @@ public class TransportModuleConstants {
 
   // RPC module used by the application components to communicate with each 
other
   // This is used only at the application side.
+  // This is interally further split into RPC_LIFECYCLEMANAGER_MODULE and
+  // RPC_APP_CLIENT_MODULE - both of which inherit from RPC_APP_MODULE
+  // So for users, there is only RPC_APP_MODULE
   public static final String RPC_APP_MODULE = "rpc_app";
   // RPC module used to communicate with/between server components
   // This is used both at server (master/worker) and application side.
   public static final String RPC_SERVICE_MODULE = "rpc_service";
 
+  // See RPC_APP_MODULE for details - both RPC_LIFECYCLEMANAGER_MODULE and 
RPC_APP_CLIENT_MODULE
+  // are internal modules, and transport configs are not expected for these.
+  // For example, auto-ssl requires both RPC_LIFECYCLEMANAGER_MODULE and 
RPC_APP_CLIENT_MODULE
+  // to be in sync, though it is enabled only in RPC_LIFECYCLEMANAGER_MODULE
+  public static final String RPC_LIFECYCLEMANAGER_MODULE = 
"rpc_app_lifecyclemanager";
+  public static final String RPC_APP_CLIENT_MODULE = "rpc_app_client";

Review Comment:
   What's the difference between `rpc_app_client` and `rpc_app` module? From a 
user perspective, this can easily be confusing.



##########
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala:
##########
@@ -1371,12 +1397,24 @@ class CelebornConf(loadDefaults: Boolean) extends 
Cloneable with Logging with Se
 
 object CelebornConf extends Logging {
 
-  val TRANSPORT_MODULE_FALLBACKS = Map(
-    "rpc_service" -> "rpc",
-    "rpc_app" -> "rpc",
+  val TRANSPORT_MODULE_FALLBACKS: Map[String, String] = Map(

Review Comment:
   Would this fallback document later?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to