Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10813 )
Change subject: IMPALA-7163: Implement a state machine for the QueryState class ...................................................................... Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/10813/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10813/2//COMMIT_MSG@26 PS2, Line 26: We : use a combination of these atomic variables and promises to allow the : fragment instance threads to communicate their completion of their : respective states or errors with the QueryState thread. An alternative design is to have the fragment instance state threads themselves update the BackendExecState by calling QueryState::UpdateQueryState(). This would require more synchronization, and would also split the responsibility of sending reports between threads, i.e. good reports will be sent by the QueryState thread and the code path on an error (which includes sending a final error report) will execute on a FIS thread. With how it's implemented in this patch, all FIS threads have no other responsibility other than doing their bit of the query and notifying the QS of successful completion or an error via AtomicInts and Promises, and the QS thread will be able to handle all the logic of what to do on an error, or during successful completion, or cancellation. That's one of the main reasons why I went with the design as implemented in this patch. I'm open to suggestions however. -- To view, visit http://gerrit.cloudera.org:8080/10813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe Gerrit-Change-Number: 10813 Gerrit-PatchSet: 2 Gerrit-Owner: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Comment-Date: Mon, 25 Jun 2018 22:31:53 +0000 Gerrit-HasComments: Yes
