Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/11650 )
Change subject: IMPALA-7272: Fix crash in StringMinMaxFilter ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/11650/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11650/2//COMMIT_MSG@26 PS2, Line 26: I have been unable to repro the actual crash despite trying a large : variety of different things. However, with additional logging added : its clear that the MemPool is being used concurrently, which is : incorrect. > Can you reproduce the problem now with DFAKE_MUTEX ? I looped for awhile and it didn't hit, but I can generate a thread collision by adding an appropriate sleep() to Allocate to increase the period the lock is held for. http://gerrit.cloudera.org:8080/#/c/11650/2/be/CMakeLists.txt File be/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/11650/2/be/CMakeLists.txt@234 PS2, Line 234: SET(CLANG_IR_CXX_FLAGS "${CLANG_IR_CXX_FLAGS}" "-DNDEBUG") > I think this makes sense. I wonder what the historical context for not doin Yeah, it seems that its always been this way since we first introduced CLANG_IR_CXX_FLAGS, which unfortunately was back before the Impala team used good commit messages. The perf difference seems very minor - I think exhaustive DEBUG usually takes about 7-8 hours and the exhaustive DEBUG build I ran with this patch took 7h49m. http://gerrit.cloudera.org:8080/#/c/11650/2/be/src/runtime/mem-pool.h File be/src/runtime/mem-pool.h: http://gerrit.cloudera.org:8080/#/c/11650/2/be/src/runtime/mem-pool.h@135 PS2, Line 135: DFAKE_SCOPED_LOCK(mutex_); > nit: Feel free to ignore but it seems easier to just add DFAKE_SCOPED_LOCK( Yeah, I guess it doesn't make much difference, but I just wanted to maximize the amount of time the lock is held for since actually hitting the bug is quite difficult. -- To view, visit http://gerrit.cloudera.org:8080/11650 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I751cad7e6b75c9d95d7ad029bbd1e52fe09e8a29 Gerrit-Change-Number: 11650 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Comment-Date: Wed, 17 Oct 2018 19:18:07 +0000 Gerrit-HasComments: Yes
