You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by GitBox <gi...@apache.org> on 2022/08/22 03:30:24 UTC

[GitHub] [incubator-pegasus] GehaFearless opened a new pull request, #1132: chore(server): disable UDP service by default

GehaFearless opened a new pull request, #1132:
URL: https://github.com/apache/incubator-pegasus/pull/1132

   issue: https://github.com/apache/incubator-pegasus/issues/1131
   
   ### What problem does this PR solve? <!--add issue link with summary if exists-->
   Disable UDP service when not use it.
   
   ### What is changed and how does it work?
   Add a configuration 'enable_udp' with a default value of 'false' in the 'network' section. It gives the server control over whether to start the UDP service.


-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1132: feat(server): disable UDP service by default

Posted by GitBox <gi...@apache.org>.
acelyc111 commented on code in PR #1132:
URL: https://github.com/apache/incubator-pegasus/pull/1132#discussion_r952119615


##########
src/server/config.ini:
##########
@@ -94,6 +94,7 @@
   io_service_worker_count = 4
   ; how many connections can be established from one ip address to a server(both replica and meta), 0 means no threshold
   conn_threshold_per_ip = 0
+  enable_udp = false

Review Comment:
   UDP is enabled before, the default value would better to be `true` to keep compatiblity, you can define it what ever you like in your own envs.



##########
src/server/config.ini:
##########
@@ -708,12 +709,10 @@
 
 [task.RPC_FD_FAILURE_DETECTOR_PING]
   rpc_call_header_format = NET_HDR_DSN
-  rpc_call_channel = RPC_CHANNEL_UDP

Review Comment:
   same, not remove it.



##########
src/rdsn/src/runtime/nativerun.cpp:
##########
@@ -59,19 +60,22 @@ void nativerun::install(service_spec &spec)
         cs2.message_buffer_block_size = 1024 * 64;
         spec.network_default_server_cfs[cs2] = cs2;
     }
-    {
-        network_client_config cs;
-        cs.factory_name = "dsn::tools::asio_udp_provider";
-        cs.message_buffer_block_size = 1024 * 64;
-        spec.network_default_client_cfs[RPC_CHANNEL_UDP] = cs;
-    }
-    {
-        network_server_config cs2;
-        cs2.port = 0;
-        cs2.channel = RPC_CHANNEL_UDP;
-        cs2.factory_name = "dsn::tools::asio_udp_provider";
-        cs2.message_buffer_block_size = 1024 * 64;
-        spec.network_default_server_cfs[cs2] = cs2;
+    if (dsn_config_get_value_bool(

Review Comment:
   use the recommend new style DSN_DECLARE_bool, FLAGS_enable_udp



##########
src/rdsn/src/runtime/providers.common.cpp:
##########
@@ -64,8 +64,11 @@ void register_common_providers()
 
     register_std_lock_providers();
 
+    if (dsn_config_get_value_bool(

Review Comment:
   use the new config framework `DSN_DEFINE_bool(...)`



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] empiredan merged pull request #1132: feat(server): disable UDP service by config

Posted by GitBox <gi...@apache.org>.
empiredan merged PR #1132:
URL: https://github.com/apache/incubator-pegasus/pull/1132


-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1132: feat(server): disable UDP service by config

Posted by GitBox <gi...@apache.org>.
acelyc111 commented on code in PR #1132:
URL: https://github.com/apache/incubator-pegasus/pull/1132#discussion_r963214249


##########
src/rdsn/src/runtime/task/task_spec.cpp:
##########
@@ -232,6 +236,12 @@ bool task_spec::init()
                 return false;
             }
         }
+
+        if (spec->rpc_call_channel == RPC_CHANNEL_UDP && !dsn::tools::FLAGS_enable_udp) {
+            derror("task rpc_call_channel RPC_CHANNEL_UCP need udp service, set network "
+                   "configuration enable_udp true");

Review Comment:
   Better to clarify what config it is, that is `[network].enable_udp`



##########
src/rdsn/src/runtime/nativerun.cpp:
##########
@@ -59,19 +62,21 @@ void nativerun::install(service_spec &spec)
         cs2.message_buffer_block_size = 1024 * 64;
         spec.network_default_server_cfs[cs2] = cs2;
     }
-    {
-        network_client_config cs;
-        cs.factory_name = "dsn::tools::asio_udp_provider";
-        cs.message_buffer_block_size = 1024 * 64;
-        spec.network_default_client_cfs[RPC_CHANNEL_UDP] = cs;
-    }
-    {
-        network_server_config cs2;
-        cs2.port = 0;
-        cs2.channel = RPC_CHANNEL_UDP;
-        cs2.factory_name = "dsn::tools::asio_udp_provider";
-        cs2.message_buffer_block_size = 1024 * 64;
-        spec.network_default_server_cfs[cs2] = cs2;
+    if (FLAGS_enable_udp) {
+        {
+            network_client_config cs;
+            cs.factory_name = "dsn::tools::asio_udp_provider";
+            cs.message_buffer_block_size = 1024 * 64;
+            spec.network_default_client_cfs[RPC_CHANNEL_UDP] = cs;
+        }
+        {
+            network_server_config cs2;

Review Comment:
   Now that they are in different scope, it's not needed to use different name, `cs2` is not a very good name.



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org