Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 )
Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC ...................................................................... Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/10855/10/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/10/be/src/runtime/query-state.cc@287 PS10, Line 287: > To answer the question about your second race, I think it's impossible toda Ah, I missed that we join on the reporter thread first. Given that, does the report sequence number generation need to be atomic? It seems you expect only a single thread to be reporting at once, in which case non-atomic int would be fine. On a similar note, the interaction between 'done' and the sequence numbers is a bit strange to me. It seems like, if we trust our sequence number generation is free of the above race, then we dont need the special handling where "done" always wins out over the sequence-number checks, right? i.e we already guarantee that the "done" message has a higher sequence than any prior report? I also wonder whether it is feasible to add an assertion for non-concurrency of report-sending by using a DFAKE_MUTEX inside the ReportExecStatusAux function. Then if someone accidentally breaks these assumptions we'll get a clear failure in debug builds instead of some more rare hard-to-debug stuck query. -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 11 Gerrit-Owner: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Wed, 22 Aug 2018 18:47:07 +0000 Gerrit-HasComments: Yes
