[kudu-CR] [test] Fix Unix socket tests on macOS
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16458 ) Change subject: [test] Fix Unix socket tests on macOS .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16458/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16458/1//COMMIT_MSG@9 PS1, Line 9: macOS does not support abstract namespaces for sockets. > Since that code isn't going to work on macOS, I guess that code should be f That impacts the tests that use ClientTestUnixSocket. client-test.cc ClientTestUnixSocket tests are an example of that. I could change the server_base.cc to use a patch in the metadata directory by default, or something like that. -- To view, visit http://gerrit.cloudera.org:8080/16458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb Gerrit-Change-Number: 16458 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 16 Sep 2020 20:29:54 + Gerrit-HasComments: Yes
[kudu-CR] [test] Fix Unix socket tests on macOS
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16458 ) Change subject: [test] Fix Unix socket tests on macOS .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16458/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16458/1//COMMIT_MSG@9 PS1, Line 9: macOS does not support abstract namespaces for sockets. > You mean we should just default to using a standard path in sever_base.cc i Since that code isn't going to work on macOS, I guess that code should be fixed as well. I think it's better to fix that code eventually, but in a separate patch. Let's keep this patch focused on test-related things for Unix sockets? -- To view, visit http://gerrit.cloudera.org:8080/16458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb Gerrit-Change-Number: 16458 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 16 Sep 2020 17:53:17 + Gerrit-HasComments: Yes
[kudu-CR] [test] Fix Unix socket tests on macOS
Greg Solovyev has posted comments on this change. ( http://gerrit.cloudera.org:8080/16458 ) Change subject: [test] Fix Unix socket tests on macOS .. Patch Set 2: Code-Review+1 client-test, master_authz-itest, location_cache-test, rpc-test, and socket-test pass on my Mac (10.14.4) with this change. -- To view, visit http://gerrit.cloudera.org:8080/16458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb Gerrit-Change-Number: 16458 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 16 Sep 2020 17:38:22 + Gerrit-HasComments: No
[kudu-CR] [test] Fix Unix socket tests on macOS
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16458 ) Change subject: [test] Fix Unix socket tests on macOS .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/util/net/socket-test.cc File src/kudu/util/net/socket-test.cc: http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/util/net/socket-test.cc@117 PS1, Line 117: unix: > How does it look like on macOS? unix:@ followed by a bunch of null characters. -- To view, visit http://gerrit.cloudera.org:8080/16458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb Gerrit-Change-Number: 16458 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 16 Sep 2020 17:16:37 + Gerrit-HasComments: Yes
[kudu-CR] [test] Fix Unix socket tests on macOS
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16458 ) Change subject: [test] Fix Unix socket tests on macOS .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16458/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16458/1//COMMIT_MSG@9 PS1, Line 9: macOS does not support abstract namespaces for sockets. > While you are on it, maybe it's worth updating the code to make it as porta You mean we should just default to using a standard path in sever_base.cc instead of only on macOS? -- To view, visit http://gerrit.cloudera.org:8080/16458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb Gerrit-Change-Number: 16458 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 16 Sep 2020 15:48:16 + Gerrit-HasComments: Yes
[kudu-CR] [test] Fix Unix socket tests on macOS
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16458 )
Change subject: [test] Fix Unix socket tests on macOS
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16458/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16458/1//COMMIT_MSG@9
PS1, Line 9: macOS does not support abstract namespaces for sockets.
While you are on it, maybe it's worth updating the code to make it as portable
as possible? I guess we can keep the Linux-specific extension for the abstract
namespace in Unix domain sockets only in
SocketTest.TestUnixSocketAbstractNamespace scenario, using the standard path
elsewhere.
http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:
http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/rpc/rpc-test.cc@123
PS1, Line 123: string path = Substitute("/tmp/kudu-$0-$1", uuid, getpid());
> Todd actually commented a reason why not to use the test directory in Socke
Ah, I see. I remember I had to deal with that when implementing mini_chronyd.
The solution (which is portable, btw) is here
https://github.com/apache/kudu/blob/4cbee427475396c9cf1e05a402c3f952d3fafde7/src/kudu/clock/test/mini_chronyd.cc#L421-L437
I used micros instead of uuid gen there :)
I think keeping those files not in /tmp but rather under Kudu test directory
(not exactly test _data_ directory) might be a bit cleaner unless the socket
files removed automatically.
--
To view, visit http://gerrit.cloudera.org:8080/16458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb
Gerrit-Change-Number: 16458
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 15:45:49 +
Gerrit-HasComments: Yes
[kudu-CR] [test] Fix Unix socket tests on macOS
Grant Henke has posted comments on this change. (
http://gerrit.cloudera.org:8080/16458 )
Change subject: [test] Fix Unix socket tests on macOS
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:
http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/rpc/rpc-test.cc@123
PS1, Line 123: string path = Substitute("/tmp/kudu-$0-$1", uuid, getpid());
> GetTestDataDirectory() in src/kudu/util/test_util.h returns the path to the
Todd actually commented a reason why not to use the test directory in
SocketTest.TestUnixSocketFilesystemPath:
// Use a path in /tmp/ instead of the normal GetTestPath approach because
// unix domain socket paths are limited in length. The test directory
// may be too long.
It uses SCOPED_CLEANUP to unlink the path, I could do something similar.
--
To view, visit http://gerrit.cloudera.org:8080/16458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb
Gerrit-Change-Number: 16458
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 15:26:49 +
Gerrit-HasComments: Yes
[kudu-CR] [test] Fix Unix socket tests on macOS
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16458 )
Change subject: [test] Fix Unix socket tests on macOS
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:
http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/rpc/rpc-test.cc@123
PS1, Line 123: string path = Substitute("/tmp/kudu-$0-$1", uuid, getpid());
> Here and elsewhere: the file isn't automatically removed when socket is clo
GetTestDataDirectory() in src/kudu/util/test_util.h returns the path to the
test directory.
--
To view, visit http://gerrit.cloudera.org:8080/16458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb
Gerrit-Change-Number: 16458
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 15:23:58 +
Gerrit-HasComments: Yes
[kudu-CR] [test] Fix Unix socket tests on macOS
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16458 )
Change subject: [test] Fix Unix socket tests on macOS
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:
http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/rpc/rpc-test.cc@123
PS1, Line 123: string path = Substitute("/tmp/kudu-$0-$1", uuid, getpid());
Here and elsewhere: the file isn't automatically removed when socket is closed
(see man 4 unix on macOS). So, it's necessary to remove it manually. I think
it's better to put it under the test's data directory, so the file is
automatically removed when test successfully passes.
I guess if putting the socket file under the test directory, then there isn't a
need to add an extra UUID along with PID into the path.
http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/util/net/socket-test.cc
File src/kudu/util/net/socket-test.cc:
http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/util/net/socket-test.cc@117
PS1, Line 117: unix:
How does it look like on macOS?
--
To view, visit http://gerrit.cloudera.org:8080/16458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb
Gerrit-Change-Number: 16458
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 15:20:59 +
Gerrit-HasComments: Yes
[kudu-CR] [test] Fix Unix socket tests on macOS
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16458 ) Change subject: [test] Fix Unix socket tests on macOS .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/rpc/rpc-test.cc File src/kudu/rpc/rpc-test.cc: http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/rpc/rpc-test.cc@65 PS1, Line 65: #include "kudu/util/oid_generator.h" > Looks like this needs to be #ifdef 'ed as well for MacOS. Done http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/util/net/socket-test.cc File src/kudu/util/net/socket-test.cc: http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/util/net/socket-test.cc@34 PS1, Line 34: #include > Double quotes instead of <> brackets. Done -- To view, visit http://gerrit.cloudera.org:8080/16458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb Gerrit-Change-Number: 16458 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 16 Sep 2020 15:19:26 + Gerrit-HasComments: Yes
[kudu-CR] [test] Fix Unix socket tests on macOS
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Bankim Bhavsar, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16458 to look at the new patch set (#2). Change subject: [test] Fix Unix socket tests on macOS .. [test] Fix Unix socket tests on macOS macOS does not support abstract namespaces for sockets. This patch fixes the socket usage by using a real path when on macOS. Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb --- M src/kudu/rpc/rpc-test.cc M src/kudu/server/server_base.cc M src/kudu/util/net/socket-test.cc 3 files changed, 25 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/16458/2 -- To view, visit http://gerrit.cloudera.org:8080/16458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb Gerrit-Change-Number: 16458 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [test] Fix Unix socket tests on macOS
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16458 ) Change subject: [test] Fix Unix socket tests on macOS .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/rpc/rpc-test.cc File src/kudu/rpc/rpc-test.cc: http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/rpc/rpc-test.cc@65 PS1, Line 65: #include "kudu/util/oid_generator.h" Looks like this needs to be #ifdef 'ed as well for MacOS. http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/util/net/socket-test.cc File src/kudu/util/net/socket-test.cc: http://gerrit.cloudera.org:8080/#/c/16458/1/src/kudu/util/net/socket-test.cc@34 PS1, Line 34: #include Double quotes instead of <> brackets. -- To view, visit http://gerrit.cloudera.org:8080/16458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb Gerrit-Change-Number: 16458 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 16 Sep 2020 14:59:28 + Gerrit-HasComments: Yes
[kudu-CR] [test] Fix Unix socket tests on macOS
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16458 Change subject: [test] Fix Unix socket tests on macOS .. [test] Fix Unix socket tests on macOS macOS does not support abstract namespaces for sockets. This patch fixes the socket usage by using a real path when on macOS. Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb --- M src/kudu/rpc/rpc-test.cc M src/kudu/server/server_base.cc M src/kudu/util/net/socket-test.cc 3 files changed, 23 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/16458/1 -- To view, visit http://gerrit.cloudera.org:8080/16458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb Gerrit-Change-Number: 16458 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
