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

Change subject: tsan: flush TSAN memory every 10 seconds
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8251/1/build-support/run-test.sh
File build-support/run-test.sh:

http://gerrit.cloudera.org:8080/#/c/8251/1/build-support/run-test.sh@106
PS1, Line 106: history_size=7
> Would it make sense to reduce it down to 5 (i.e. 1M accesses)?
I believe we set this to 7 because we were previously getting various errors 
that the stack was lost. This made debugging rather tricky.

I think the memory used by this "history" ends up being relatively low compared 
to the memory used by the shadow zone, which is what's getting flushed by the 
flush_memory_* flags discussed below.


http://gerrit.cloudera.org:8080/#/c/8251/1/build-support/run-test.sh@109
PS1, Line 109: flush_memory_ms
> Maybe, dynamically setting memory_limit_mb option to some value based on /p
One issue with that is that we typically fork out several processes from a 
minicluster, and those processes are also instrumented. So, we could set it to 
80% of system RAM or whatever, but then _each_ of the daemons would be 
restricted to that, and the sum would be way more than the total amount of RAM.

I considered changing ExternalMiniCluster to set this flag based on something 
conservative like 1/4 or 1/5 of system RAM but felt like it would be a bit 
messy to have it in two places (since we still need the setting on non-EMC 
tests too). Based on a thread on the thread-sanitizer mailing list, apparently 
Google sets flush_memory_ms=10000 and _in addition_ sets memory_limit_mb based 
on the available RAM. We could certainly do that as a belt-and-suspenders 
approach.

What do you think?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I721f126eb731c7f35afb732cebe48abd52c3ebd1
Gerrit-Change-Number: 8251
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Thu, 12 Oct 2017 23:09:48 +0000
Gerrit-HasComments: Yes

Reply via email to