Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/22882 )
Change subject: [socket] Fix test failure caused due to unsupported socket option ...................................................................... Patch Set 3: (1 comment) > Patch Set 3: > > (1 comment) http://gerrit.cloudera.org:8080/#/c/22882/3/src/kudu/master/mini_master.cc File src/kudu/master/mini_master.cc: http://gerrit.cloudera.org:8080/#/c/22882/3/src/kudu/master/mini_master.cc@61 PS3, Line 61: opts_.rpc_opts.rpc_reuseport = FLAGS_rpc_reuseport; > If you are just looking for next steps in this context and want to introduc There is one more reason that flag makes sense. This setting is applied per messenger object which remains same for the rpc server object. So when a new master get initialized, the setting will part of its corresponding messenger. Following check inside Messenger::AddAcceptorPool won't make sense because we will return config error from here and test will fail because it knowingly set the port reuse to true (because of "opts_.rpc_opts.rpc_reuseport = true" statement here) for a Unix domain socket: if (reuseport_) { if (PREDICT_FALSE(accept_addr.is_unix())) { return Status::ConfigurationError(...); } RETURN_NOT_OK(sock.SetReusePort(true)); } That raises questions on why MiniMaster object is being used for Unix domain socket. Maybe client test that work on Unix domain socket should not be using InternalMiniCluster because that uses MiniMaster internally and it has port reuse always set to true. Also, there is just one test (TestConnectViaUnixSocket which is also failing) inside client-test.cc that is making use of rpc server created from MiniMaster object and expects it to work for Unix domain sockets. I am not sure but the commit message from the patch that introduced this test might provide some background on why the class was used: ++++ commit be68ce81beeb708edfc0545695e72282506f3845 Author: Todd Lipcon <[email protected]> Date: Thu Apr 9 16:42:44 2020 -0700 client/tserver: add support for connecting over unix domain sockets This adds new experiental flags -rpc_listen_on_unix_domain_socket and -client_use_unix_domain_sockets. The former makes the RPC server bind to a unix socket and advertise this to the kudu master as part of the TS registration. The latter makes the client attempt to connect via a domain socket when it sees such a socket path advertised. Note that this makes one behavioral change even when those flags are not enabled: we now consider any tablet server with a loopback IP to be "local" (and thus a candidate for unix domain socket connection). This mostly affects the MiniCluster where tablet servers register using various IPs in the loopback range 127.0.0.0/8, and was necessary in order to test unix socket connections from the client. ++++ Maybe, as a limited and targeted fix for test specific issue, don't set reuseport for messenger builder during server init so that while adding new acceptor pool, port reuse socket option is not set on Unix domain socket, thereby, proceeding with successful bind call. Something like this: src/kudu/server/server_base.cc - if (options_.rpc_opts.rpc_reuseport) { -- + if (!FLAGS_rpc_listen_on_unix_domain_socket && options_.rpc_opts.rpc_reuseport) { + builder.set_reuseport(); -- To view, visit http://gerrit.cloudera.org:8080/22882 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ice00b1c4fd1df78fa84c3dd2a79c968a4a91cc21 Gerrit-Change-Number: 22882 Gerrit-PatchSet: 3 Gerrit-Owner: Ashwani Raina <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 29 May 2025 16:00:03 +0000 Gerrit-HasComments: Yes
