Dan Hecht has posted comments on this change. Change subject: IMPALA-5427: Fix order of status handling in ClientRequestState ......................................................................
Patch Set 1: (1 comment) This is really subtle. In order to be correct, you'd need at least a compiler barrier to ensure the writes stay in the intended order. However, the HS2 version of the RPCs is already taking the request state lock: GetOperationStatus(). I think we should just do the same inside of get_state() and get_log(). Then we can assert, like the HS2 code, that an error state implies a !ok() status. Is there a reason we don't want to take the lock? http://gerrit.cloudera.org:8080/#/c/7155/1//COMMIT_MSG Commit Message: PS1, Line 9: ClientRequestState::UpdateQueryStatus() the race isn't really in that function. It's between that function and the pair of beeswax get_state()/get_log() RPCs. Please clarify. -- To view, visit http://gerrit.cloudera.org:8080/7155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib4494fe3f933cc23841db0e7da407eec5650f2b5 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-HasComments: Yes
