Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12740 )

Change subject: [subprocess] added test to repro deadlock on logging
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/12740/1//COMMIT_MSG
Commit Message:

PS1:
> How often does it deadlock? I wonder if it'd deadlock more often if the Sub
With the code as is, it deadlocked every time I ran it at ve0518 (ran about 100 
times or so in DEBUG configuration).  I can get a broader stats using 
dist-test, if needed.

Running Subprocess in a loop is a good idea -- I'll do that.


http://gerrit.cloudera.org:8080/#/c/12740/1/src/kudu/util/subprocess-test.cc
File src/kudu/util/subprocess-test.cc:

http://gerrit.cloudera.org:8080/#/c/12740/1/src/kudu/util/subprocess-test.cc@373
PS1, Line 373:     threads.emplace_back([&barrier, thread_idx] () {
> Should explain why each thread logs and sleeps.
Done


http://gerrit.cloudera.org:8080/#/c/12740/1/src/kudu/util/subprocess-test.cc@385
PS1, Line 385:       }
> Nit: any particular reason to use an absolute path here? Not that I would e
Good idea: I switched to using the file in the CWD.


http://gerrit.cloudera.org:8080/#/c/12740/1/src/kudu/util/subprocess-test.cc@387
PS1, Line 387:
> Could maybe simplify with Subprocess::Call? The barrier point would come "e
Done


http://gerrit.cloudera.org:8080/#/c/12740/1/src/kudu/util/subprocess-test.cc@388
PS1, Line 388:   barrier.Wait();
> Nit: does it need to be initialized?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie88073bb7fcccf24b4451fe3c4e4cd8bd471ca67
Gerrit-Change-Number: 12740
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <[email protected]>
Gerrit-Comment-Date: Wed, 13 Mar 2019 05:51:43 +0000
Gerrit-HasComments: Yes

Reply via email to