Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15244 )
Change subject: IMPALA-5904: (part 2) Fix more TSAN bugs ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/15244/2/be/src/runtime/krpc-data-stream-sender.cc File be/src/runtime/krpc-data-stream-sender.cc: http://gerrit.cloudera.org:8080/#/c/15244/2/be/src/runtime/krpc-data-stream-sender.cc@583 PS2, Line 583: #ifndef NDEBUG I'd prefer to avoid changes to locking in debug builds - this could make it harder to reproduce issues on debug builds. Maybe we can annotate this so that TSAN ignores the racy read, or remove the dcheck if it's not critical. http://gerrit.cloudera.org:8080/#/c/15244/2/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/15244/2/be/src/service/impala-server.cc@820 PS2, Line 820: query->fetch_rows_lock()->lock(); Why not use a lock_guard? I think this might also be fixing IMPALA-8442 -- To view, visit http://gerrit.cloudera.org:8080/15244 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I01feb40417dc5ea64ccb0c1044cfc3eed8508476 Gerrit-Change-Number: 15244 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Wed, 19 Feb 2020 19:51:47 +0000 Gerrit-HasComments: Yes
