You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2020/09/16 14:39:30 UTC

[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 <gr...@apache.org>

[kudu-CR] [test] Fix Unix socket tests on macOS

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
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 <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 15:26:49 +0000
Gerrit-HasComments: Yes

[kudu-CR] [test] Fix Unix socket tests on macOS

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
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 <kudu/util/oid_generator.h>
> 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 <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 15:19:26 +0000
Gerrit-HasComments: Yes

[kudu-CR] [test] Fix Unix socket tests on macOS

Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
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 <kudu/util/oid_generator.h>
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 <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 14:59:28 +0000
Gerrit-HasComments: Yes

[kudu-CR] [test] Fix Unix socket tests on macOS

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 15:45:49 +0000
Gerrit-HasComments: Yes

[kudu-CR] [test] Fix Unix socket tests on macOS

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
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 <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 17:16:37 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix Unix socket usage on macOS

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16458 )

Change subject: Fix Unix socket usage on macOS
......................................................................


Patch Set 6: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:

http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/rpc/rpc-test.cc@117
PS5, Line 117: unlink(socket_path_.c_str());
> I am not sure. I think I prefer ulink given it the defacto way to handle so
As an additional point for advocating for the change, I looked through tests and found that we primarily use Env::X() for filesystem operations instead of direct system calls.

But I guess it's OK to use unlink() here since it's not critical in this context of a clean-up for a test scenario.  So, feel free to ignore if my attempt to sell this change isn't appealing :)



-- 
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: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 21 Sep 2020 23:20:28 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix Unix socket usage on macOS

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Greg Solovyev, 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 (#3).

Change subject: Fix Unix socket usage on macOS
......................................................................

Fix Unix socket usage on macOS

macOS does not support abstract namespaces for sockets.
This patch fixes the socket usage by using a real path instead
of abstract namespaces.

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
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
5 files changed, 29 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/16458/3
-- 
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: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [test] Fix Unix socket tests on macOS

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
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 <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 15:48:16 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix Unix socket usage on macOS

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.

Change subject: Fix Unix socket usage on macOS
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
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: deleteVote
Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb
Gerrit-Change-Number: 16458
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] Fix Unix socket usage on macOS

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Greg Solovyev, 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 (#6).

Change subject: Fix Unix socket usage on macOS
......................................................................

Fix Unix socket usage on macOS

macOS does not support abstract namespaces for sockets.
This patch fixes the socket usage by using a real path instead
of abstract namespaces.

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
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
5 files changed, 41 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/16458/6
-- 
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: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] Fix Unix socket usage on macOS

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16458 )

Change subject: Fix Unix socket usage on macOS
......................................................................


Patch Set 5:

(3 comments)

Looks good overall, just a couple nits.

http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/rpc/rpc-test-base.h
File src/kudu/rpc/rpc-test-base.h:

http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/rpc/rpc-test-base.h@426
PS5, Line 426: virtual 
nit: 'virtual' isn't necessary here -- this class overrides TearDown() declared somewhere in the base


http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:

http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/rpc/rpc-test.cc@117
PS5, Line 117: unlink(socket_path_.c_str());
here and below: would env_->DeleteFile(socket_path_) be a better fit?  At least, it provides a common path for logging (like WARN_IF_NOT_ON()) and error handling.


http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/util/net/socket-test.cc
File src/kudu/util/net/socket-test.cc:

http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/util/net/socket-test.cc@162
PS5, Line 162: unlink(path.c_str());
nit: maybe, replace with
  WARN_NOT_OK(env_->DeleteFile(path));

?



-- 
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: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 18 Sep 2020 23:08:48 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix Unix socket usage on macOS

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Greg Solovyev, 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 (#4).

Change subject: Fix Unix socket usage on macOS
......................................................................

Fix Unix socket usage on macOS

macOS does not support abstract namespaces for sockets.
This patch fixes the socket usage by using a real path instead
of abstract namespaces.

Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb
---
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc-test.cc
M src/kudu/server/server_base.cc
M src/kudu/util/net/socket-test.cc
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
6 files changed, 42 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/16458/4
-- 
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: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] Fix Unix socket usage on macOS

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16458 )

Change subject: Fix Unix socket usage on macOS
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:

http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/rpc/rpc-test.cc@117
PS5, Line 117: unlink(socket_path_.c_str());
> As an additional point for advocating for the change, I looked through test
I agree thats true, but this is slightly different given this is specifically for socket usage. The precedent for this in the codebase is in the one place where unlink was already used in socket-test.cc



-- 
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: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 22 Sep 2020 16:07:05 +0000
Gerrit-HasComments: Yes

[kudu-CR] [test] Fix Unix socket tests on macOS

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 15:20:59 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix Unix socket usage on macOS

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16458 )

Change subject: Fix Unix socket usage on macOS
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16458/3/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:

http://gerrit.cloudera.org:8080/#/c/16458/3/src/kudu/rpc/rpc-test.cc@116
PS3, Line 116: 
> That's cool!
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@117
PS1, Line 117: unix:
> nit: does it make sense to add a CHECK() based on string match in case of m
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 <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 18 Sep 2020 19:22:33 +0000
Gerrit-HasComments: Yes

[kudu-CR] [test] Fix Unix socket tests on macOS

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 15:23:58 +0000
Gerrit-HasComments: Yes

[kudu-CR] [test] Fix Unix socket tests on macOS

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 17:53:17 +0000
Gerrit-HasComments: Yes

[kudu-CR] [test] Fix Unix socket tests on macOS

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
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 <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [test] Fix Unix socket tests on macOS

Posted by "Greg Solovyev (Code Review)" <ge...@cloudera.org>.
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 <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 17:38:22 +0000
Gerrit-HasComments: No

[kudu-CR] [test] Fix Unix socket tests on macOS

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
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 <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 20:29:54 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix Unix socket usage on macOS

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16458 )

Change subject: Fix Unix socket usage on macOS
......................................................................


Patch Set 6: Verified+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: comment
Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb
Gerrit-Change-Number: 16458
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 21 Sep 2020 02:44:11 +0000
Gerrit-HasComments: No

[kudu-CR] Fix Unix socket usage on macOS

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16458 )

Change subject: Fix Unix socket usage on macOS
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/rpc/rpc-test-base.h
File src/kudu/rpc/rpc-test-base.h:

http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/rpc/rpc-test-base.h@426
PS5, Line 426: virtual 
> nit: 'virtual' isn't necessary here -- this class overrides TearDown() decl
Done


http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:

http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/rpc/rpc-test.cc@117
PS5, Line 117: unlink(socket_path_.c_str());
> here and below: would env_->DeleteFile(socket_path_) be a better fit?  At l
I am not sure. I think I prefer ulink given it the defacto way to handle socket files and we don't check or care much about error handling.


http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/util/net/socket-test.cc
File src/kudu/util/net/socket-test.cc:

http://gerrit.cloudera.org:8080/#/c/16458/5/src/kudu/util/net/socket-test.cc@162
PS5, Line 162: unlink(path.c_str());
> nit: maybe, replace with
See my other response.



-- 
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: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 21 Sep 2020 01:47:34 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix Unix socket usage on macOS

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16458 )

Change subject: Fix Unix socket usage on macOS
......................................................................

Fix Unix socket usage on macOS

macOS does not support abstract namespaces for sockets.
This patch fixes the socket usage by using a real path instead
of abstract namespaces.

Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb
Reviewed-on: http://gerrit.cloudera.org:8080/16458
Tested-by: Grant Henke <gr...@apache.org>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/rpc/rpc-test.cc
M src/kudu/server/server_base.cc
M src/kudu/util/net/socket-test.cc
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
5 files changed, 41 insertions(+), 10 deletions(-)

Approvals:
  Grant Henke: Verified
  Alexey Serbin: Looks good to me, approved

-- 
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: merged
Gerrit-Change-Id: I434e92e86fb87d1085c35b5d707d1238295192fb
Gerrit-Change-Number: 16458
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] Fix Unix socket usage on macOS

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16458 )

Change subject: Fix Unix socket usage on macOS
......................................................................


Patch Set 3:

(3 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.
> That impacts the tests that use ClientTestUnixSocket. client-test.cc Client
Ah, it for some reason I was under impression that changes in server_base.cc are not necessary to make those tests in question to pass on macOS.

I think the way how it's now in server_base.cc is totally fine.  I was just advocating for having _tests_ using POSIX-compatible way of creating Unix domain sockets, unless they specifically target to test the anonymous namespace Unix sockets on Linux, like SocketTest.TestUnixSocketAbstractNamespace scenario does.


http://gerrit.cloudera.org:8080/#/c/16458/3/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:

http://gerrit.cloudera.org:8080/#/c/16458/3/src/kudu/rpc/rpc-test.cc@116
PS3, Line 116: GetTestSocketPath
That's cool!

However, it seems the corresponding files will accumulat under the test directory (which is /tmp/kudutest-<os_uid> by default).  Is it possible to clean those files up automatically?


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:@ followed by a bunch of null characters.
nit: does it make sense to add a CHECK() based on string match in case of macOS as well?



-- 
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: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 21:25:27 +0000
Gerrit-HasComments: Yes