FMX commented on code in PR #2232:
URL: 
https://github.com/apache/incubator-celeborn/pull/2232#discussion_r1454688859


##########
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala:
##########
@@ -1432,48 +1432,102 @@ object CelebornConf extends Logging {
   val NETWORK_IO_PREFER_DIRECT_BUFS: ConfigEntry[Boolean] =
     buildConf("celeborn.<module>.io.preferDirectBufs")
       .categories("network")
-      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty.")
+      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty. " +
+        s"If setting <module> to `${TransportModuleConstants.RPC_MODULE}`, " +
+        s"it works for shuffle client, master or worker server rpc and should 
be configured on client side. " +
+        s"If setting <module> to `${TransportModuleConstants.DATA_MODULE}`, " +
+        s"it works for shuffle client push or fetch data and should be 
configured on client side. " +

Review Comment:
   ```suggestion
           s"it works for shuffle client push and fetch data. " +
   ```



##########
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala:
##########
@@ -1432,48 +1432,102 @@ object CelebornConf extends Logging {
   val NETWORK_IO_PREFER_DIRECT_BUFS: ConfigEntry[Boolean] =
     buildConf("celeborn.<module>.io.preferDirectBufs")
       .categories("network")
-      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty.")
+      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty. " +
+        s"If setting <module> to `${TransportModuleConstants.RPC_MODULE}`, " +
+        s"it works for shuffle client, master or worker server rpc and should 
be configured on client side. " +
+        s"If setting <module> to `${TransportModuleConstants.DATA_MODULE}`, " +
+        s"it works for shuffle client push or fetch data and should be 
configured on client side. " +
+        s"If setting <module> to `${TransportModuleConstants.PUSH_MODULE}`, " +
+        s"it works for worker server push data to peer worker and should be 
configured on worker side. " +
+        s"If setting <module> to 
`${TransportModuleConstants.REPLICATE_MODULE}`, " +
+        s"it works for replicate server or client of worker server replicate 
data to peer worker and should be configured on worker side. " +

Review Comment:
   ```suggestion
           s"it works for replicate server or client of worker server 
replicating data to peer worker. " +
   ```



##########
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala:
##########
@@ -1432,48 +1432,102 @@ object CelebornConf extends Logging {
   val NETWORK_IO_PREFER_DIRECT_BUFS: ConfigEntry[Boolean] =
     buildConf("celeborn.<module>.io.preferDirectBufs")
       .categories("network")
-      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty.")
+      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty. " +
+        s"If setting <module> to `${TransportModuleConstants.RPC_MODULE}`, " +
+        s"it works for shuffle client, master or worker server rpc and should 
be configured on client side. " +

Review Comment:
   ```suggestion
           s"it works for shuffle client, master and worker. " +
   ```
   I think works for shuffle client is enough for a user to understand that 
this config needs to be added to client.



##########
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala:
##########
@@ -1432,48 +1432,102 @@ object CelebornConf extends Logging {
   val NETWORK_IO_PREFER_DIRECT_BUFS: ConfigEntry[Boolean] =
     buildConf("celeborn.<module>.io.preferDirectBufs")
       .categories("network")
-      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty.")
+      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty. " +
+        s"If setting <module> to `${TransportModuleConstants.RPC_MODULE}`, " +
+        s"it works for shuffle client, master or worker server rpc and should 
be configured on client side. " +
+        s"If setting <module> to `${TransportModuleConstants.DATA_MODULE}`, " +
+        s"it works for shuffle client push or fetch data and should be 
configured on client side. " +
+        s"If setting <module> to `${TransportModuleConstants.PUSH_MODULE}`, " +
+        s"it works for worker server push data to peer worker and should be 
configured on worker side. " +
+        s"If setting <module> to 
`${TransportModuleConstants.REPLICATE_MODULE}`, " +
+        s"it works for replicate server or client of worker server replicate 
data to peer worker and should be configured on worker side. " +
+        s"If setting <module> to `${TransportModuleConstants.FETCH_MODULE}`, " 
+
+        s"it works for worker server fetch data to peer worker and should be 
configured on worker side.")
       .booleanConf
       .createWithDefault(true)
 
   val NETWORK_IO_CONNECT_TIMEOUT: ConfigEntry[Long] =
     buildConf("celeborn.<module>.io.connectTimeout")
       .categories("network")
-      .doc("Socket connect timeout.")
+      .doc("Socket connect timeout. " +
+        s"If setting <module> to `${TransportModuleConstants.RPC_MODULE}`, " +
+        s"it works for shuffle client rpc and should be configured on client 
side. " +

Review Comment:
   ```suggestion
           s"it works for shuffle client rpc. " +
   ```



##########
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala:
##########
@@ -1432,48 +1432,102 @@ object CelebornConf extends Logging {
   val NETWORK_IO_PREFER_DIRECT_BUFS: ConfigEntry[Boolean] =
     buildConf("celeborn.<module>.io.preferDirectBufs")
       .categories("network")
-      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty.")
+      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty. " +
+        s"If setting <module> to `${TransportModuleConstants.RPC_MODULE}`, " +
+        s"it works for shuffle client, master or worker server rpc and should 
be configured on client side. " +
+        s"If setting <module> to `${TransportModuleConstants.DATA_MODULE}`, " +
+        s"it works for shuffle client push or fetch data and should be 
configured on client side. " +
+        s"If setting <module> to `${TransportModuleConstants.PUSH_MODULE}`, " +
+        s"it works for worker server push data to peer worker and should be 
configured on worker side. " +
+        s"If setting <module> to 
`${TransportModuleConstants.REPLICATE_MODULE}`, " +
+        s"it works for replicate server or client of worker server replicate 
data to peer worker and should be configured on worker side. " +
+        s"If setting <module> to `${TransportModuleConstants.FETCH_MODULE}`, " 
+
+        s"it works for worker server fetch data to peer worker and should be 
configured on worker side.")

Review Comment:
   ```suggestion
           s"it works for worker fetch server.")
   ```



##########
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala:
##########
@@ -1432,48 +1432,102 @@ object CelebornConf extends Logging {
   val NETWORK_IO_PREFER_DIRECT_BUFS: ConfigEntry[Boolean] =
     buildConf("celeborn.<module>.io.preferDirectBufs")
       .categories("network")
-      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty.")
+      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty. " +
+        s"If setting <module> to `${TransportModuleConstants.RPC_MODULE}`, " +
+        s"it works for shuffle client, master or worker server rpc and should 
be configured on client side. " +
+        s"If setting <module> to `${TransportModuleConstants.DATA_MODULE}`, " +
+        s"it works for shuffle client push or fetch data and should be 
configured on client side. " +
+        s"If setting <module> to `${TransportModuleConstants.PUSH_MODULE}`, " +
+        s"it works for worker server push data to peer worker and should be 
configured on worker side. " +
+        s"If setting <module> to 
`${TransportModuleConstants.REPLICATE_MODULE}`, " +
+        s"it works for replicate server or client of worker server replicate 
data to peer worker and should be configured on worker side. " +
+        s"If setting <module> to `${TransportModuleConstants.FETCH_MODULE}`, " 
+
+        s"it works for worker server fetch data to peer worker and should be 
configured on worker side.")
       .booleanConf
       .createWithDefault(true)
 
   val NETWORK_IO_CONNECT_TIMEOUT: ConfigEntry[Long] =
     buildConf("celeborn.<module>.io.connectTimeout")
       .categories("network")
-      .doc("Socket connect timeout.")
+      .doc("Socket connect timeout. " +
+        s"If setting <module> to `${TransportModuleConstants.RPC_MODULE}`, " +
+        s"it works for shuffle client rpc and should be configured on client 
side. " +
+        s"If setting <module> to `${TransportModuleConstants.DATA_MODULE}`, " +
+        s"it works for shuffle client push or fetch data and should be 
configured on client side. " +

Review Comment:
   ```suggestion
           s"it works for shuffle client push or fetch data. " +
   ```



##########
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala:
##########
@@ -1432,48 +1432,102 @@ object CelebornConf extends Logging {
   val NETWORK_IO_PREFER_DIRECT_BUFS: ConfigEntry[Boolean] =
     buildConf("celeborn.<module>.io.preferDirectBufs")
       .categories("network")
-      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty.")
+      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty. " +
+        s"If setting <module> to `${TransportModuleConstants.RPC_MODULE}`, " +
+        s"it works for shuffle client, master or worker server rpc and should 
be configured on client side. " +
+        s"If setting <module> to `${TransportModuleConstants.DATA_MODULE}`, " +
+        s"it works for shuffle client push or fetch data and should be 
configured on client side. " +
+        s"If setting <module> to `${TransportModuleConstants.PUSH_MODULE}`, " +
+        s"it works for worker server push data to peer worker and should be 
configured on worker side. " +

Review Comment:
   ```suggestion
           s"it works for worker server receiving push data. " +
   ```



##########
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala:
##########
@@ -1432,48 +1432,102 @@ object CelebornConf extends Logging {
   val NETWORK_IO_PREFER_DIRECT_BUFS: ConfigEntry[Boolean] =
     buildConf("celeborn.<module>.io.preferDirectBufs")
       .categories("network")
-      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty.")
+      .doc("If true, we will prefer allocating off-heap byte buffers within 
Netty. " +
+        s"If setting <module> to `${TransportModuleConstants.RPC_MODULE}`, " +
+        s"it works for shuffle client, master or worker server rpc and should 
be configured on client side. " +
+        s"If setting <module> to `${TransportModuleConstants.DATA_MODULE}`, " +
+        s"it works for shuffle client push or fetch data and should be 
configured on client side. " +
+        s"If setting <module> to `${TransportModuleConstants.PUSH_MODULE}`, " +
+        s"it works for worker server push data to peer worker and should be 
configured on worker side. " +
+        s"If setting <module> to 
`${TransportModuleConstants.REPLICATE_MODULE}`, " +
+        s"it works for replicate server or client of worker server replicate 
data to peer worker and should be configured on worker side. " +
+        s"If setting <module> to `${TransportModuleConstants.FETCH_MODULE}`, " 
+
+        s"it works for worker server fetch data to peer worker and should be 
configured on worker side.")
       .booleanConf
       .createWithDefault(true)
 
   val NETWORK_IO_CONNECT_TIMEOUT: ConfigEntry[Long] =
     buildConf("celeborn.<module>.io.connectTimeout")
       .categories("network")
-      .doc("Socket connect timeout.")
+      .doc("Socket connect timeout. " +
+        s"If setting <module> to `${TransportModuleConstants.RPC_MODULE}`, " +
+        s"it works for shuffle client rpc and should be configured on client 
side. " +
+        s"If setting <module> to `${TransportModuleConstants.DATA_MODULE}`, " +
+        s"it works for shuffle client push or fetch data and should be 
configured on client side. " +
+        s"If setting <module> to 
`${TransportModuleConstants.REPLICATE_MODULE}`, " +
+        s"it works for replicate client of worker server replicate data to 
peer worker and should be configured on worker side.")

Review Comment:
   ```suggestion
           s"it works for the replicate client of workers replicating data to 
peer worker.")
   ```



-- 
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