Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14143 )
Change subject: IMPALA-8572: Log query events before unregister. ...................................................................... Patch Set 11: (5 comments) Still not clear on the test failures. Addressing the comments and kicking off another run while I look into it. http://gerrit.cloudera.org:8080/#/c/14143/10/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/14143/10/be/src/service/client-request-state.cc@749 PS10, Line 749: if (wait_thread_.get() != nullptr) { > Should we drop the lock while joining on the wait thread? I feel like there Done http://gerrit.cloudera.org:8080/#/c/14143/10/be/src/service/client-request-state.cc@754 PS10, Line 754: // flush log events to audit authorization errors, if any. > We could probably also drop the lock when calling this. Done http://gerrit.cloudera.org:8080/#/c/14143/10/be/src/service/client-request-state.cc@780 PS10, Line 780: if (wait_thread_.get() == nullptr) return; > I think we can actually just delete this line, the next line is sufficient. Done http://gerrit.cloudera.org:8080/#/c/14143/10/be/src/service/client-request-state.cc@1395 PS10, Line 1395: Status status; > Accessing query_status without acquiring a lock might not actually be safe Done http://gerrit.cloudera.org:8080/#/c/14143/10/be/src/service/client-request-state.cc@1426 PS10, Line 1426: } > Should be a const TExecRequest& to avoid copying. nice find. -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 11 Gerrit-Owner: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: radford nguyen <radford.ngu...@gmail.com> Gerrit-Comment-Date: Wed, 11 Sep 2019 00:51:25 +0000 Gerrit-HasComments: Yes
