[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-08 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11886 )

Change subject: run-test: don't report leaks that don't fail test
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11886/4/src/kudu/server/generic_service.cc
File src/kudu/server/generic_service.cc:

http://gerrit.cloudera.org:8080/#/c/11886/4/src/kudu/server/generic_service.cc@183
PS4, Line 183: LOG(INFO) << "LeakSanitizer found false positives. 
Ignoring them.";
May want to make this bigger so it stands out from the report. And maybe 
WARNING instead of INFO.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 07 Nov 2018 23:50:37 +
Gerrit-HasComments: Yes


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-08 Thread Andrew Wong (Code Review)
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/11886

to look at the new patch set (#5).

Change subject: run-test: don't report leaks that don't fail test
..

run-test: don't report leaks that don't fail test

LeakSanitizer will report a leak when allocating a string in
SuperviseThread.  It's unclear why this is the case, but upon inspecting
the code, it seems like a false positive. The stack trace is as follows:

=
==93677==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 58 byte(s) in 1 object(s) allocated from:
#0 0x5318c8 in operator new(unsigned long) 
/data/8/awong/kudu/thirdparty/src/llvm-6.0.0.src/projects/compiler-rt/lib/asan/asan_new_delete.cc:92
#1 0x3ae3a9c3c8 in std::string::_Rep::_S_create(unsigned long, unsigned 
long, std::allocator const&) (/usr/lib64/libstdc++.so.6+0x3ae3a9c3c8)
#2 0x3ae3a9d19a in std::string::_Rep::_M_clone(std::allocator const&, 
unsigned long) (/usr/lib64/libstdc++.so.6+0x3ae3a9d19a)
#3 0x3ae3a9d5eb in std::string::reserve(unsigned long) 
(/usr/lib64/libstdc++.so.6+0x3ae3a9d5eb)
#4 0x3ae3a9d770 in std::string::append(unsigned long, char) 
(/usr/lib64/libstdc++.so.6+0x3ae3a9d770)
#5 0x7f518f799c12 in strings::SubstituteAndAppend(std::string*, 
StringPiece, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&) 
../src/kudu/gutil/strings/substitute.cc:110:3
#6 0x536e76 in strings::Substitute(StringPiece, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&) 
../src/kudu/gutil/strings/substitute.h:188:3
#7 0x7f5190590860 in kudu::Thread::SuperviseThread(void*) 
../src/kudu/util/thread.cc:607:17
#8 0x3ae0e079d0 in start_thread (/lib64/libpthread.so.0+0x3ae0e079d0)
#9 0x3ae0ae88fc in clone (/lib64/libc.so.6+0x3ae0ae88fc)

This appears to be affecting a number tests, but generally only lines #0
and #1 are present in the logs, making them difficult to debug (a
developer would have to rerun the test with specific ASAN_OPTIONS to
unwind the stacktrace more). Namely, exactly_once_writes-itest
(KUDU-2517), kudu-admin-test (KUDU-2583), and rebalancer-tool-test
(untracked via Jira) all show the top of the above stack trace, and
based on the full stack trace, it seems these are all false positives.

The presence of issues like
https://github.com/google/sanitizers/issues/757 confirms that
LeakSanitizer can report false positives in workloads with high
concurrency. Generally, the test binary will return an error in the face
of real leaks, but in tests like the ones mentioned, the test may log
messages reporting leaks, but not actually return an error because the
"leak" was transient (e.g. see GenericServiceImpl::CheckLeaks).

We currently inject errors into JUnit XML report if any leaks are
reported, even for false positives, since the leak messages still find
their way into the logs. This patch updates this to only inject these
errors if the test also returned an error.

For clarity, I also threw in a log statement to
GenericServiceImpl::CheckLeaks denoting false positives.

Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
---
M build-support/run-test.sh
M src/kudu/server/generic_service.cc
2 files changed, 15 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/11886/5
--
To view, visit http://gerrit.cloudera.org:8080/11886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-08 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11886 )

Change subject: run-test: don't report leaks that don't fail test
..


Patch Set 7:

Turns out IWYU was unhappy too.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 08 Nov 2018 02:34:14 +
Gerrit-HasComments: No


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-08 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11886 )

Change subject: run-test: don't report leaks that don't fail test
..


Patch Set 7: Code-Review+2

Carrying forward Adar's +2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 08 Nov 2018 03:10:20 +
Gerrit-HasComments: No


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-08 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11886 )

Change subject: run-test: don't report leaks that don't fail test
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11886/4/src/kudu/server/generic_service.cc
File src/kudu/server/generic_service.cc:

http://gerrit.cloudera.org:8080/#/c/11886/4/src/kudu/server/generic_service.cc@183
PS4, Line 183: if (!has_leaks) {
> May want to make this bigger so it stands out from the report. And maybe WA
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 08 Nov 2018 00:48:19 +
Gerrit-HasComments: Yes


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-08 Thread Andrew Wong (Code Review)
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/11886

to look at the new patch set (#7).

Change subject: run-test: don't report leaks that don't fail test
..

run-test: don't report leaks that don't fail test

LeakSanitizer will report a leak when allocating a string in
SuperviseThread.  It's unclear why this is the case, but upon inspecting
the code, it seems like a false positive. The stack trace is as follows:

=
==93677==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 58 byte(s) in 1 object(s) allocated from:
#0 0x5318c8 in operator new(unsigned long) 
/data/8/awong/kudu/thirdparty/src/llvm-6.0.0.src/projects/compiler-rt/lib/asan/asan_new_delete.cc:92
#1 0x3ae3a9c3c8 in std::string::_Rep::_S_create(unsigned long, unsigned 
long, std::allocator const&) (/usr/lib64/libstdc++.so.6+0x3ae3a9c3c8)
#2 0x3ae3a9d19a in std::string::_Rep::_M_clone(std::allocator const&, 
unsigned long) (/usr/lib64/libstdc++.so.6+0x3ae3a9d19a)
#3 0x3ae3a9d5eb in std::string::reserve(unsigned long) 
(/usr/lib64/libstdc++.so.6+0x3ae3a9d5eb)
#4 0x3ae3a9d770 in std::string::append(unsigned long, char) 
(/usr/lib64/libstdc++.so.6+0x3ae3a9d770)
#5 0x7f518f799c12 in strings::SubstituteAndAppend(std::string*, 
StringPiece, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&) 
../src/kudu/gutil/strings/substitute.cc:110:3
#6 0x536e76 in strings::Substitute(StringPiece, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&) 
../src/kudu/gutil/strings/substitute.h:188:3
#7 0x7f5190590860 in kudu::Thread::SuperviseThread(void*) 
../src/kudu/util/thread.cc:607:17
#8 0x3ae0e079d0 in start_thread (/lib64/libpthread.so.0+0x3ae0e079d0)
#9 0x3ae0ae88fc in clone (/lib64/libc.so.6+0x3ae0ae88fc)

This appears to be affecting a number tests, but generally only lines #0
and #1 are present in the logs, making them difficult to debug (a
developer would have to rerun the test with specific ASAN_OPTIONS to
unwind the stacktrace more). Namely, exactly_once_writes-itest
(KUDU-2517), kudu-admin-test (KUDU-2583), and rebalancer-tool-test
(untracked via Jira) all show the top of the above stack trace, and
based on the full stack trace, it seems these are all false positives.

The presence of issues like
https://github.com/google/sanitizers/issues/757 confirms that
LeakSanitizer can report false positives in workloads with high
concurrency. Generally, the test binary will return an error in the face
of real leaks, but in tests like the ones mentioned, the test may log
messages reporting leaks, but not actually return an error because the
"leak" was transient (e.g. see GenericServiceImpl::CheckLeaks).

We currently inject errors into JUnit XML report if any leaks are
reported, even for false positives, since the leak messages still find
their way into the logs. This patch updates this to only inject these
errors if the test also returned an error.

For clarity, I also threw in a log statement to
GenericServiceImpl::CheckLeaks denoting false positives.

Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
---
M build-support/run-test.sh
M src/kudu/server/generic_service.cc
2 files changed, 15 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/11886/7
--
To view, visit http://gerrit.cloudera.org:8080/11886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-08 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11886 )

Change subject: run-test: don't report leaks that don't fail test
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11886/5/src/kudu/server/generic_service.cc
File src/kudu/server/generic_service.cc:

http://gerrit.cloudera.org:8080/#/c/11886/5/src/kudu/server/generic_service.cc@52
PS5, Line 52:
> warning: using decl 'Substitute' is unused [misc-unused-using-decls]
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 08 Nov 2018 01:10:17 +
Gerrit-HasComments: Yes


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-08 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11886 )

Change subject: run-test: don't report leaks that don't fail test
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 08 Nov 2018 00:48:38 +
Gerrit-HasComments: No


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-08 Thread Andrew Wong (Code Review)
Andrew Wong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11886 )

Change subject: run-test: don't report leaks that don't fail test
..

run-test: don't report leaks that don't fail test

LeakSanitizer will report a leak when allocating a string in
SuperviseThread.  It's unclear why this is the case, but upon inspecting
the code, it seems like a false positive. The stack trace is as follows:

=
==93677==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 58 byte(s) in 1 object(s) allocated from:
#0 0x5318c8 in operator new(unsigned long) 
/data/8/awong/kudu/thirdparty/src/llvm-6.0.0.src/projects/compiler-rt/lib/asan/asan_new_delete.cc:92
#1 0x3ae3a9c3c8 in std::string::_Rep::_S_create(unsigned long, unsigned 
long, std::allocator const&) (/usr/lib64/libstdc++.so.6+0x3ae3a9c3c8)
#2 0x3ae3a9d19a in std::string::_Rep::_M_clone(std::allocator const&, 
unsigned long) (/usr/lib64/libstdc++.so.6+0x3ae3a9d19a)
#3 0x3ae3a9d5eb in std::string::reserve(unsigned long) 
(/usr/lib64/libstdc++.so.6+0x3ae3a9d5eb)
#4 0x3ae3a9d770 in std::string::append(unsigned long, char) 
(/usr/lib64/libstdc++.so.6+0x3ae3a9d770)
#5 0x7f518f799c12 in strings::SubstituteAndAppend(std::string*, 
StringPiece, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&) 
../src/kudu/gutil/strings/substitute.cc:110:3
#6 0x536e76 in strings::Substitute(StringPiece, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&) 
../src/kudu/gutil/strings/substitute.h:188:3
#7 0x7f5190590860 in kudu::Thread::SuperviseThread(void*) 
../src/kudu/util/thread.cc:607:17
#8 0x3ae0e079d0 in start_thread (/lib64/libpthread.so.0+0x3ae0e079d0)
#9 0x3ae0ae88fc in clone (/lib64/libc.so.6+0x3ae0ae88fc)

This appears to be affecting a number tests, but generally only lines #0
and #1 are present in the logs, making them difficult to debug (a
developer would have to rerun the test with specific ASAN_OPTIONS to
unwind the stacktrace more). Namely, exactly_once_writes-itest
(KUDU-2517), kudu-admin-test (KUDU-2583), and rebalancer-tool-test
(untracked via Jira) all show the top of the above stack trace, and
based on the full stack trace, it seems these are all false positives.

The presence of issues like
https://github.com/google/sanitizers/issues/757 confirms that
LeakSanitizer can report false positives in workloads with high
concurrency. Generally, the test binary will return an error in the face
of real leaks, but in tests like the ones mentioned, the test may log
messages reporting leaks, but not actually return an error because the
"leak" was transient (e.g. see GenericServiceImpl::CheckLeaks).

We currently inject errors into JUnit XML report if any leaks are
reported, even for false positives, since the leak messages still find
their way into the logs. This patch updates this to only inject these
errors if the test also returned an error.

For clarity, I also threw in a log statement to
GenericServiceImpl::CheckLeaks denoting false positives.

Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Reviewed-on: http://gerrit.cloudera.org:8080/11886
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong 
---
M build-support/run-test.sh
M src/kudu/server/generic_service.cc
2 files changed, 15 insertions(+), 5 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Andrew Wong: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-08 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11886 )

Change subject: run-test: don't report leaks that don't fail test
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11886/5/src/kudu/server/generic_service.cc
File src/kudu/server/generic_service.cc:

http://gerrit.cloudera.org:8080/#/c/11886/5/src/kudu/server/generic_service.cc@52
PS5, Line 52:
> Odd suggestion, seeing as you were clearly using it.
I think it's mad because the `Substitute` is in a `ifndef` block.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 08 Nov 2018 01:18:16 +
Gerrit-HasComments: Yes


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-08 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11886 )

Change subject: run-test: don't report leaks that don't fail test
..


Patch Set 6: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11886/5/src/kudu/server/generic_service.cc
File src/kudu/server/generic_service.cc:

http://gerrit.cloudera.org:8080/#/c/11886/5/src/kudu/server/generic_service.cc@52
PS5, Line 52:
> Done
Odd suggestion, seeing as you were clearly using it.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 08 Nov 2018 01:17:08 +
Gerrit-HasComments: Yes


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-08 Thread Andrew Wong (Code Review)
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/11886

to look at the new patch set (#6).

Change subject: run-test: don't report leaks that don't fail test
..

run-test: don't report leaks that don't fail test

LeakSanitizer will report a leak when allocating a string in
SuperviseThread.  It's unclear why this is the case, but upon inspecting
the code, it seems like a false positive. The stack trace is as follows:

=
==93677==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 58 byte(s) in 1 object(s) allocated from:
#0 0x5318c8 in operator new(unsigned long) 
/data/8/awong/kudu/thirdparty/src/llvm-6.0.0.src/projects/compiler-rt/lib/asan/asan_new_delete.cc:92
#1 0x3ae3a9c3c8 in std::string::_Rep::_S_create(unsigned long, unsigned 
long, std::allocator const&) (/usr/lib64/libstdc++.so.6+0x3ae3a9c3c8)
#2 0x3ae3a9d19a in std::string::_Rep::_M_clone(std::allocator const&, 
unsigned long) (/usr/lib64/libstdc++.so.6+0x3ae3a9d19a)
#3 0x3ae3a9d5eb in std::string::reserve(unsigned long) 
(/usr/lib64/libstdc++.so.6+0x3ae3a9d5eb)
#4 0x3ae3a9d770 in std::string::append(unsigned long, char) 
(/usr/lib64/libstdc++.so.6+0x3ae3a9d770)
#5 0x7f518f799c12 in strings::SubstituteAndAppend(std::string*, 
StringPiece, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&) 
../src/kudu/gutil/strings/substitute.cc:110:3
#6 0x536e76 in strings::Substitute(StringPiece, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&) 
../src/kudu/gutil/strings/substitute.h:188:3
#7 0x7f5190590860 in kudu::Thread::SuperviseThread(void*) 
../src/kudu/util/thread.cc:607:17
#8 0x3ae0e079d0 in start_thread (/lib64/libpthread.so.0+0x3ae0e079d0)
#9 0x3ae0ae88fc in clone (/lib64/libc.so.6+0x3ae0ae88fc)

This appears to be affecting a number tests, but generally only lines #0
and #1 are present in the logs, making them difficult to debug (a
developer would have to rerun the test with specific ASAN_OPTIONS to
unwind the stacktrace more). Namely, exactly_once_writes-itest
(KUDU-2517), kudu-admin-test (KUDU-2583), and rebalancer-tool-test
(untracked via Jira) all show the top of the above stack trace, and
based on the full stack trace, it seems these are all false positives.

The presence of issues like
https://github.com/google/sanitizers/issues/757 confirms that
LeakSanitizer can report false positives in workloads with high
concurrency. Generally, the test binary will return an error in the face
of real leaks, but in tests like the ones mentioned, the test may log
messages reporting leaks, but not actually return an error because the
"leak" was transient (e.g. see GenericServiceImpl::CheckLeaks).

We currently inject errors into JUnit XML report if any leaks are
reported, even for false positives, since the leak messages still find
their way into the logs. This patch updates this to only inject these
errors if the test also returned an error.

For clarity, I also threw in a log statement to
GenericServiceImpl::CheckLeaks denoting false positives.

Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
---
M build-support/run-test.sh
M src/kudu/server/generic_service.cc
2 files changed, 15 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/11886/6
--
To view, visit http://gerrit.cloudera.org:8080/11886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-07 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11886 )

Change subject: run-test: don't report leaks that don't fail test
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11886/3/src/kudu/server/generic_service.cc
File src/kudu/server/generic_service.cc:

http://gerrit.cloudera.org:8080/#/c/11886/3/src/kudu/server/generic_service.cc@182
PS3, Line 182:   if (i > 0) {
> Won't this print in the normal case where there are no leaks at all? Seems
Agh, you are right.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 07 Nov 2018 23:03:36 +
Gerrit-HasComments: Yes


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-07 Thread Andrew Wong (Code Review)
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/11886

to look at the new patch set (#4).

Change subject: run-test: don't report leaks that don't fail test
..

run-test: don't report leaks that don't fail test

LeakSanitizer will report a leak when allocating a string in
SuperviseThread.  It's unclear why this is the case, but upon inspecting
the code, it seems like a false positive. The stack trace is as follows:

=
==93677==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 58 byte(s) in 1 object(s) allocated from:
#0 0x5318c8 in operator new(unsigned long) 
/data/8/awong/kudu/thirdparty/src/llvm-6.0.0.src/projects/compiler-rt/lib/asan/asan_new_delete.cc:92
#1 0x3ae3a9c3c8 in std::string::_Rep::_S_create(unsigned long, unsigned 
long, std::allocator const&) (/usr/lib64/libstdc++.so.6+0x3ae3a9c3c8)
#2 0x3ae3a9d19a in std::string::_Rep::_M_clone(std::allocator const&, 
unsigned long) (/usr/lib64/libstdc++.so.6+0x3ae3a9d19a)
#3 0x3ae3a9d5eb in std::string::reserve(unsigned long) 
(/usr/lib64/libstdc++.so.6+0x3ae3a9d5eb)
#4 0x3ae3a9d770 in std::string::append(unsigned long, char) 
(/usr/lib64/libstdc++.so.6+0x3ae3a9d770)
#5 0x7f518f799c12 in strings::SubstituteAndAppend(std::string*, 
StringPiece, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&) 
../src/kudu/gutil/strings/substitute.cc:110:3
#6 0x536e76 in strings::Substitute(StringPiece, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&) 
../src/kudu/gutil/strings/substitute.h:188:3
#7 0x7f5190590860 in kudu::Thread::SuperviseThread(void*) 
../src/kudu/util/thread.cc:607:17
#8 0x3ae0e079d0 in start_thread (/lib64/libpthread.so.0+0x3ae0e079d0)
#9 0x3ae0ae88fc in clone (/lib64/libc.so.6+0x3ae0ae88fc)

This appears to be affecting a number tests, but generally only lines #0
and #1 are present in the logs, making them difficult to debug (a
developer would have to rerun the test with specific ASAN_OPTIONS to
unwind the stacktrace more). Namely, exactly_once_writes-itest
(KUDU-2517), kudu-admin-test (KUDU-2583), and rebalancer-tool-test
(untracked via Jira) all show the top of the above stack trace, and
based on the full stack trace, it seems these are all false positives.

The presence of issues like
https://github.com/google/sanitizers/issues/757 confirms that
LeakSanitizer can report false positives in workloads with high
concurrency. Generally, the test binary will return an error in the face
of real leaks, but in tests like the ones mentioned, the test may log
messages reporting leaks, but not actually return an error because the
"leak" was transient (e.g. see GenericServiceImpl::CheckLeaks).

We currently inject errors into JUnit XML report if any leaks are
reported, even for false positives, since the leak messages still find
their way into the logs. This patch updates this to only inject these
errors if the test also returned an error.

For clarity, I also threw in a log statement to
GenericServiceImpl::CheckLeaks denoting false positives.

Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
---
M build-support/run-test.sh
M src/kudu/server/generic_service.cc
2 files changed, 12 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/11886/4
--
To view, visit http://gerrit.cloudera.org:8080/11886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-07 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11886 )

Change subject: run-test: don't report leaks that don't fail test
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11886/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11886/2//COMMIT_MSG@16
PS2, Line 16: Direct leak of 58 byte(s) in 1 object(s) allocated from:
: #0 0x5318c8 in operator new(unsigned long) 
/data/8/awong/kudu/thirdparty/src/llvm-6.0.0.src/projects/compiler-rt/lib/asan/asan_new_delete.cc:92
: #1
> Oh, so this is squarely the "false positive leak that goes away if __lsan_d
Right. Todd suggested maybe patching LSAN so that it logs into a buffer instead 
of stdout/stderr during __lsan_do_recoverable_leak_check(), but looking into 
that, it seems there'd be a decent amount of plumbing to get that.

I'm somewhat conflicted because it's still uncertain whether this is the only 
class of leak that will not fail a test. That said, I don't think the injected 
JUnit errors are particularly helpful; presumably if the leak were real and 
worth developer attention, it would be surfaced as a test failure anyway.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 07 Nov 2018 22:23:01 +
Gerrit-HasComments: Yes


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-07 Thread Andrew Wong (Code Review)
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/11886

to look at the new patch set (#3).

Change subject: run-test: don't report leaks that don't fail test
..

run-test: don't report leaks that don't fail test

LeakSanitizer will report a leak when allocating a string in
SuperviseThread.  It's unclear why this is the case, but upon inspecting
the code, it seems like a false positive. The stack trace is as follows:

=
==93677==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 58 byte(s) in 1 object(s) allocated from:
#0 0x5318c8 in operator new(unsigned long) 
/data/8/awong/kudu/thirdparty/src/llvm-6.0.0.src/projects/compiler-rt/lib/asan/asan_new_delete.cc:92
#1 0x3ae3a9c3c8 in std::string::_Rep::_S_create(unsigned long, unsigned 
long, std::allocator const&) (/usr/lib64/libstdc++.so.6+0x3ae3a9c3c8)
#2 0x3ae3a9d19a in std::string::_Rep::_M_clone(std::allocator const&, 
unsigned long) (/usr/lib64/libstdc++.so.6+0x3ae3a9d19a)
#3 0x3ae3a9d5eb in std::string::reserve(unsigned long) 
(/usr/lib64/libstdc++.so.6+0x3ae3a9d5eb)
#4 0x3ae3a9d770 in std::string::append(unsigned long, char) 
(/usr/lib64/libstdc++.so.6+0x3ae3a9d770)
#5 0x7f518f799c12 in strings::SubstituteAndAppend(std::string*, 
StringPiece, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&) 
../src/kudu/gutil/strings/substitute.cc:110:3
#6 0x536e76 in strings::Substitute(StringPiece, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&, strings::internal::SubstituteArg 
const&, strings::internal::SubstituteArg const&, 
strings::internal::SubstituteArg const&) 
../src/kudu/gutil/strings/substitute.h:188:3
#7 0x7f5190590860 in kudu::Thread::SuperviseThread(void*) 
../src/kudu/util/thread.cc:607:17
#8 0x3ae0e079d0 in start_thread (/lib64/libpthread.so.0+0x3ae0e079d0)
#9 0x3ae0ae88fc in clone (/lib64/libc.so.6+0x3ae0ae88fc)

This appears to be affecting a number tests, but generally only lines #0
and #1 are present in the logs, making them difficult to debug (a
developer would have to rerun the test with specific ASAN_OPTIONS to
unwind the stacktrace more). Namely, exactly_once_writes-itest
(KUDU-2517), kudu-admin-test (KUDU-2583), and rebalancer-tool-test
(untracked via Jira) all show the top of the above stack trace, and
based on the full stack trace, it seems these are all false positives.

The presence of issues like
https://github.com/google/sanitizers/issues/757 confirms that
LeakSanitizer can report false positives in workloads with high
concurrency. Generally, the test binary will return an error in the face
of real leaks, but in tests like the ones mentioned, the test may log
messages reporting leaks, but not actually return an error because the
"leak" was transient (e.g. see GenericServiceImpl::CheckLeaks).

We currently inject errors into JUnit XML report if any leaks are
reported, even for false positives, since the leak messages still find
their way into the logs. This patch updates this to only inject these
errors if the test also returned an error.

For clarity, I also threw in a log statement to
GenericServiceImpl::CheckLeaks denoting false positives.

Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
---
M build-support/run-test.sh
M src/kudu/server/generic_service.cc
2 files changed, 10 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/11886/3
--
To view, visit http://gerrit.cloudera.org:8080/11886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-07 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11886 )

Change subject: run-test: don't report leaks that don't fail test
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11886/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11886/2//COMMIT_MSG@16
PS2, Line 16: We currently inject errors into JUnit XML report if any leaks are 
found.
: This patch updates this to only inject these errors if the test 
also
: failed.
> Ah, I like that approach much better. You're right that it isn't clear why
Oh, so this is squarely the "false positive leak that goes away if 
__lsan_do_recoverable_leak_check() is called again" issue? I didn't realize 
LSAN still prints a leak report in that case, but I guess that's to be 
expected. Unfortunately I don't see a way to control the printing of the report 
at runtime within LSAN.

In that case, maybe your original solution is the correct one. Maybe we should 
also print something in GenericServiceImpl::CheckLeaks to the effect of "LSAN 
found false positives, ignore them".



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 07 Nov 2018 17:29:41 +
Gerrit-HasComments: Yes


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-06 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11886 )

Change subject: run-test: don't report leaks that don't fail test
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11886/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11886/2//COMMIT_MSG@12
PS2, Line 12: test
Nit: tests


http://gerrit.cloudera.org:8080/#/c/11886/2//COMMIT_MSG@16
PS2, Line 16: We currently inject errors into JUnit XML report if any leaks are 
found.
: This patch updates this to only inject these errors if the test 
also
: failed.
Hmm, I don't really understand why this is safe. Are you 100% sure that a leak 
without a corresponding test failure constitutes a false positive? What 
evidence supports this?

Have you tried something different, such as adding a ScopedLeakCheckDisabler to 
the string allocation within Thread::SuperviseThread? Or, in 
GenericServiceImpl::CheckLeaks, either looping further or sleeping for longer 
in between loop cycles?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 07 Nov 2018 00:23:49 +
Gerrit-HasComments: Yes


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-06 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11886 )

Change subject: run-test: don't report leaks that don't fail test
..


Patch Set 2: Verified+1

The test failure seems unrelated:
  TabletCopyClientSessionITest.TestStopCopyOnClientDiskFailure


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Nov 2018 20:56:09 +
Gerrit-HasComments: No


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-06 Thread Andrew Wong (Code Review)
Andrew Wong has removed a vote on this change.

Change subject: run-test: don't report leaks that don't fail test
..


Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/11886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-06 Thread Andrew Wong (Code Review)
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/11886

to look at the new patch set (#2).

Change subject: run-test: don't report leaks that don't fail test
..

run-test: don't report leaks that don't fail test

The presence of issues like
https://github.com/google/sanitizers/issues/757 means that LeakSanitizer
can report false positives in workloads with high concurrency.
Generally, tests will fail in the face of real leaks, but in some test,
particularly those that use an ExternalMiniCluster, the test may report
leaks, but not actually crash (e.g. see GenericServiceImpl::CheckLeaks).

We currently inject errors into JUnit XML report if any leaks are found.
This patch updates this to only inject these errors if the test also
failed.

Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
---
M build-support/run-test.sh
1 file changed, 6 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/11886/2
--
To view, visit http://gerrit.cloudera.org:8080/11886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] run-test: don't report leaks that don't fail test

2018-11-06 Thread Andrew Wong (Code Review)
Andrew Wong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11886


Change subject: run-test: don't report leaks that don't fail test
..

run-test: don't report leaks that don't fail test

The presence of issues like
https://github.com/google/sanitizers/issues/757 means that LeakSanitizer
can report false positives in workloads with high concurrency.
Generally, tests will fail in the face of real leaks, but in some
scenarios, particularly those that use an ExternalMiniCluster, may
report leaks, but not actually crash (e.g. see
GenericServiceImpl::CheckLeaks).

We currently inject errors into JUnit XML report if leaks are found.
This patch updates this to only inject these errors if the test also
failed.

Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
---
M build-support/run-test.sh
1 file changed, 6 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/11886/1
--
To view, visit http://gerrit.cloudera.org:8080/11886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1f199795c48bd9b6106110aae132ec165eb0f647
Gerrit-Change-Number: 11886
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong