Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12739 )

Change subject: [subprocess] use RAW_LOG() in child process
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12739/6/src/kudu/util/signal.cc
File src/kudu/util/signal.cc:

http://gerrit.cloudera.org:8080/#/c/12739/6/src/kudu/util/signal.cc@40
PS6, Line 40: %i]
nit: was surprised to see %i here. Apparently %i and %d are synonymous but %d 
is a lot more commonly used in my experience. Mind changing it just so it looks 
more "standard"?  (same below)


http://gerrit.cloudera.org:8080/#/c/12739/6/src/kudu/util/subprocess.cc
File src/kudu/util/subprocess.cc:

http://gerrit.cloudera.org:8080/#/c/12739/6/src/kudu/util/subprocess.cc@384
PS6, Line 384:     // strictly speaking (might call malloc() and getenv() in 
some cases which
at least in tcmalloc, I'm pretty sure malloc-after-fork is safe due to it 
installing an atfork handler to lock the mutex prior to fork and unlock it 
after fork:

  perftools_pthread_atfork(
    CentralCacheLockAll,    // parent calls before fork
    CentralCacheUnlockAll,  // parent calls after fork
    CentralCacheUnlockAll); // child calls after fork



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9dca4ca8b1a6d72c9fc818ea41109c80ace3e39
Gerrit-Change-Number: 12739
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Will Berkeley <[email protected]>
Gerrit-Comment-Date: Wed, 13 Mar 2019 16:14:36 +0000
Gerrit-HasComments: Yes

Reply via email to