[Impala-ASF-CR] IMPALA-4557: Fix flakiness with FLAGS stress free pool alloc
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael HoGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4557: Fix flakiness with FLAGS stress free pool alloc
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc .. IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc FLAGS_stress_free_pool_alloc is a gflag for simulating malloc failure in debug builds. If set, FreePool::Allocate() and FreePool::Reallocate() will return NULL on every FLAGS_stress_free_pool_alloc allocations. The problem is that the counter for tracking number of allocations is updated racily by multiple threads, leading to non-determinism and flaky tests. This change fixes the problem by making the counter an AtomicInt32. Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 Reviewed-on: http://gerrit.cloudera.org:8080/5281 Reviewed-by: Jim AppleReviewed-by: Dan Hecht Tested-by: Internal Jenkins --- M be/src/common/atomic.h M be/src/runtime/CMakeLists.txt A be/src/runtime/free-pool.cc M be/src/runtime/free-pool.h 4 files changed, 41 insertions(+), 6 deletions(-) Approvals: Jim Apple: Looks good to me, but someone else must approve Internal Jenkins: Verified Dan Hecht: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/5281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4557: Fix flakiness with FLAGS stress free pool alloc
Dan Hecht has posted comments on this change. Change subject: IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael HoGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4557: Fix flakiness with FLAGS stress free pool alloc
Jim Apple has posted comments on this change. Change subject: IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/5281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael HoGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4557: Fix flakiness with FLAGS stress free pool alloc
Michael Ho has posted comments on this change. Change subject: IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5281/1/be/src/runtime/free-pool.h File be/src/runtime/free-pool.h: Line 214: static AtomicInt32 alloc_counts_; > I think it's clearer to explicitly initialize the value. Done -- To view, visit http://gerrit.cloudera.org:8080/5281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael HoGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4557: Fix flakiness with FLAGS stress free pool alloc
Hello Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5281 to look at the new patch set (#3). Change subject: IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc .. IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc FLAGS_stress_free_pool_alloc is a gflag for simulating malloc failure in debug builds. If set, FreePool::Allocate() and FreePool::Reallocate() will return NULL on every FLAGS_stress_free_pool_alloc allocations. The problem is that the counter for tracking number of allocations is updated racily by multiple threads, leading to non-determinism and flaky tests. This change fixes the problem by making the counter an AtomicInt32. Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 --- M be/src/common/atomic.h M be/src/runtime/CMakeLists.txt A be/src/runtime/free-pool.cc M be/src/runtime/free-pool.h 4 files changed, 41 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/5281/3 -- To view, visit http://gerrit.cloudera.org:8080/5281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael HoGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4557: Fix flakiness with FLAGS stress free pool alloc
Jim Apple has posted comments on this change. Change subject: IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5281/1/be/src/runtime/free-pool.h File be/src/runtime/free-pool.h: Line 214: static AtomicInt32 alloc_counts_; > https://github.com/apache/incubator-impala/blob/master/be/src/common/atomic I think it's clearer to explicitly initialize the value. While you're doing this, I think it's also best to turn the DCHECK in atomic.h into a static_assert to make it impossible for there to be a Static Initialization Order Fiasco bug with DCHECK's logging facilities: https://isocpp.org/wiki/faq/ctors#static-init-order -- To view, visit http://gerrit.cloudera.org:8080/5281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael HoGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4557: Fix flakiness with FLAGS stress free pool alloc
Michael Ho has posted comments on this change. Change subject: IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5281/1/be/src/runtime/free-pool.h File be/src/runtime/free-pool.h: Line 214: static AtomicInt32 alloc_counts_; > I do not see that. https://github.com/apache/incubator-impala/blob/master/be/src/common/atomic.h#L73 -- To view, visit http://gerrit.cloudera.org:8080/5281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael HoGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4557: Fix flakiness with FLAGS stress free pool alloc
Jim Apple has posted comments on this change. Change subject: IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5281/1/be/src/runtime/free-pool.h File be/src/runtime/free-pool.h: Line 214: static AtomicInt32 alloc_counts_; > Also surround this with NDEBUG? And initialize it? -- To view, visit http://gerrit.cloudera.org:8080/5281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael HoGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4557: Fix flakiness with FLAGS stress free pool alloc
Hello Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5281 to look at the new patch set (#2). Change subject: IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc .. IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc FLAGS_stress_free_pool_alloc is a gflag for simulating malloc failure in debug builds. If set, FreePool::Allocate() and FreePool::Reallocate() will return NULL on every FLAGS_stress_free_pool_alloc allocations. The problem is that the counter for tracking number of allocations is updated racily by multiple threads, leading to non-determinism and flaky tests. This change fixes the problem by making the counter an AtomicInt32. Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 --- M be/src/runtime/CMakeLists.txt A be/src/runtime/free-pool.cc M be/src/runtime/free-pool.h 3 files changed, 38 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/5281/2 -- To view, visit http://gerrit.cloudera.org:8080/5281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael HoGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4557: Fix flakiness with FLAGS stress free pool alloc
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/5281/1/be/src/runtime/free-pool.h File be/src/runtime/free-pool.h: Line 214: static AtomicInt32 alloc_counts_; Also surround this with NDEBUG? -- To view, visit http://gerrit.cloudera.org:8080/5281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael HoGerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4557: Fix flakiness with FLAGS stress free pool alloc
Michael Ho has uploaded a new change for review. http://gerrit.cloudera.org:8080/5281 Change subject: IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc .. IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc FLAGS_stress_free_pool_alloc is a gflag for simulating malloc failure in debug builds. If set, FreePool::Allocate() and FreePool::Reallocate() will return NULL on every FLAGS_stress_free_pool_alloc allocations. The problem is that the counter for tracking number of allocations is updated racily by multiple threads, leading to non-determinism and flaky tests. This change fixes the problem by making the counter an AtomicInt32. Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 --- M be/src/runtime/CMakeLists.txt A be/src/runtime/free-pool.cc M be/src/runtime/free-pool.h 3 files changed, 32 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/5281/1 -- To view, visit http://gerrit.cloudera.org:8080/5281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho