Attila Bukor has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15374 )

Change subject: KUDU-3079 Add MiniPostgres
......................................................................


Patch Set 10:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/15374/6/src/kudu/ranger/mini_postgres.cc
File src/kudu/ranger/mini_postgres.cc:

http://gerrit.cloudera.org:8080/#/c/15374/6/src/kudu/ranger/mini_postgres.cc@102
PS6, Line 102:
             :       JoinPathSegments(bin_dir_, "postgres/post
> I was and am worried that this would be flaky. If we run this from multiple
They should each get a new random port as the kernel won't assign the same 
ephemeral port until it's in TIME_WAIT (IIRC 120s by default). Only issue that 
I can see with this is that it's persisted to the config directory, so if we 
reuse the same conf dir (e.g. in a MiniCluster) it might be occupied. I added a 
todo to fix this.


http://gerrit.cloudera.org:8080/#/c/15374/8/src/kudu/ranger/mini_postgres.cc
File src/kudu/ranger/mini_postgres.cc:

http://gerrit.cloudera.org:8080/#/c/15374/8/src/kudu/ranger/mini_postgres.cc@81
PS8, Line 81:       JoinPathSegments(bin_dir_, "postgres/initdb"),
> Nit: capitalize and make a complete sentence?
Done


http://gerrit.cloudera.org:8080/#/c/15374/8/src/kudu/ranger/mini_postgres.cc@143
PS8, Line 143:     "-p", Substitute("$0", port_),
> Nit: would be nice to read this file and just add a new line for the port.
Done



--
To view, visit http://gerrit.cloudera.org:8080/15374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iba40126aa60e0ecbc5ae10cc1328493194c345bc
Gerrit-Change-Number: 15374
Gerrit-PatchSet: 10
Gerrit-Owner: Attila Bukor <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 19 Mar 2020 18:13:40 +0000
Gerrit-HasComments: Yes

Reply via email to