[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. IMPALA-4111: backend death tests should not produce minidumps Move the existing core dump disabler into a shared utility header and also disable minidumps too. Add a macro to simplify use of the disabler. Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Reviewed-on: http://gerrit.cloudera.org:8080/4353 Reviewed-by: Tim Armstrong Tested-by: Internal Jenkins --- A be/src/testutil/death-test-util.h M be/src/util/minidump.cc M be/src/util/minidump.h M be/src/util/promise-test.cc 4 files changed, 83 insertions(+), 24 deletions(-) Approvals: Internal Jenkins: Verified Tim Armstrong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. Patch Set 6: Code-Review+2 rebase -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. Patch Set 5: Verified-1 Build failed: http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/210/ -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Hello Matthew Jacobs, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4353 to look at the new patch set (#6). Change subject: IMPALA-4111: backend death tests should not produce minidumps .. IMPALA-4111: backend death tests should not produce minidumps Move the existing core dump disabler into a shared utility header and also disable minidumps too. Add a macro to simplify use of the disabler. Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 --- A be/src/testutil/death-test-util.h M be/src/util/minidump.cc M be/src/util/minidump.h M be/src/util/promise-test.cc 4 files changed, 83 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/4353/6 -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Hello Matthew Jacobs, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4353 to look at the new patch set (#5). Change subject: IMPALA-4111: backend death tests should not produce minidumps .. IMPALA-4111: backend death tests should not produce minidumps Move the existing core dump disabler into a shared utility header and also disable minidumps too. Add a macro to simplify use of the disabler. Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 --- A be/src/testutil/death-test-util.h M be/src/util/minidump.cc M be/src/util/minidump.h M be/src/util/promise-test.cc 4 files changed, 83 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/4353/5 -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Dan Hecht has posted comments on this change. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. Patch Set 4: Somehow Lars' earlier comments got deleted. Reproducing from my email: Patch Set 3: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/4353/3/be/src/util/minidump.h File be/src/util/minidump.h: Lars Volker has posted comments on this change. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. Patch Set 3: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/4353/3/be/src/util/minidump.h File be/src/util/minidump.h: PS3, Line 29: for death test nit: maybe remove or reword like "for example during death test" or "during tests" -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4353/2/be/src/testutil/death-test-util.h File be/src/testutil/death-test-util.h: PS2, Line 41: setrlimit(RLIMIT_CORE, &limit); : : minidumps_enabled_before_ = EnableMinidumpsForTest(false); > Core dumps and minidumps / breakpad should be orthogonal, so enabling/disab Thanks. Also I had chatted with Tim in person. I was trying to understand the relationship between enabling/disabling these two. In the past I had seen cases where I was getting minidumps but not cores, and thought we had disabled them. Anyway, this makes sense, thanks. -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Lars Volker has posted comments on this change. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4353/2/be/src/testutil/death-test-util.h File be/src/testutil/death-test-util.h: PS2, Line 41: setrlimit(RLIMIT_CORE, &limit); : : minidumps_enabled_before_ = EnableMinidumpsForTest(false); > Ok, then a different question: why do we need to disable coredumps? Shouldn Core dumps and minidumps / breakpad should be orthogonal, so enabling/disabling one won't interfere with the other. Policy-wise I don't think we made any effort to disable core-dumps in dev environments. I would assume that's mostly because they contain more information and these systems are well connected, so collecting and transferring them isn't that much of a concern. -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4353/2/be/src/testutil/death-test-util.h File be/src/testutil/death-test-util.h: PS2, Line 41: setrlimit(RLIMIT_CORE, &limit); : : minidumps_enabled_before_ = EnableMinidumpsForTest(false); > I'm not sure I understand the comment. The class is meant to disable both i Ok, then a different question: why do we need to disable coredumps? Shouldn't they have been disabled elsewhere in favor of using breakpad? -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Hello Lars Volker, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4353 to look at the new patch set (#4). Change subject: IMPALA-4111: backend death tests should not produce minidumps .. IMPALA-4111: backend death tests should not produce minidumps Move the existing core dump disabler into a shared utility header and also disable minidumps too. Add a macro to simplify use of the disabler. Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 --- A be/src/testutil/death-test-util.h M be/src/util/minidump.cc M be/src/util/minidump.h M be/src/util/promise-test.cc 4 files changed, 83 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/4353/4 -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/4353/2/be/src/testutil/death-test-util.h File be/src/testutil/death-test-util.h: PS2, Line 41: setrlimit(RLIMIT_CORE, &limit); : : minidumps_enabled_before_ = EnableMinidumpsForTest(false); > I assume it's necessary to disable core dumps when we've disabled minidumps I'm not sure I understand the comment. The class is meant to disable both in tests, but I don't think there's a dependency between the two. The previous version of the class in promise-test.cc only disabled coredumps. http://gerrit.cloudera.org:8080/#/c/4353/3/be/src/util/minidump.h File be/src/util/minidump.h: PS3, Line 29: for death test > nit: maybe remove or reword like "for example during death test" or "during Done -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. Patch Set 4: Code-Review+1 Carry +1 -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4353/2/be/src/testutil/death-test-util.h File be/src/testutil/death-test-util.h: PS2, Line 41: setrlimit(RLIMIT_CORE, &limit); : : minidumps_enabled_before_ = EnableMinidumpsForTest(false); I assume it's necessary to disable core dumps when we've disabled minidumps because cores will get written when we've disabled minidumps? If that's the case, can you just mention that briefly? -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: Yes