[
https://issues.apache.org/jira/browse/IMPALA-15115?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18110038#comment-18110038
]
ASF subversion and git services commented on IMPALA-15115:
----------------------------------------------------------
Commit 6db79c84381b0311e56b0555fd182cd4c7c71b6e in impala's branch
refs/heads/master from Joe McDonnell
[ https://gitbox.apache.org/repos/asf?p=impala.git;h=6db79c843 ]
IMPALA-15115: Fix race condition in GetOperationStatus with query retries
GetOperationStatus() (and get_state() for beeswax) gets the
active query handle, waits for execution to complete, then
returns the status for that query handle. While this is
happening, the query could hit an error and get retried.
In that case, GetOperationStatus is still looking at the old
query handle that failed. It would return the failure even
though there is a new query handle for the retry.
Long polling waits for an extended time for execution to
complete, so it deterministically hits this issue.
This changes the logic so that it detects a retry, looks up the
new active handle, and returns the status from the new handle.
This applies to both HS2 and Beeswax.
Testing:
- Added tests to test_query_retries.py with long polling for both
HS2 and Beeswax
Change-Id: I9934a797650eec57b8280e6a6d2ef252dac00c69
Reviewed-on: http://gerrit.cloudera.org:8080/24702
Reviewed-by: Csaba Ringhofer <[email protected]>
Tested-by: Michael Smith <[email protected]>
> Race condition where GetOperationStatus() may return error when query is
> retried
> --------------------------------------------------------------------------------
>
> Key: IMPALA-15115
> URL: https://issues.apache.org/jira/browse/IMPALA-15115
> Project: IMPALA
> Issue Type: Bug
> Components: Backend
> Affects Versions: Impala 5.0.0
> Reporter: Joe McDonnell
> Assignee: Joe McDonnell
> Priority: Critical
> Fix For: Impala 5.0.0
>
>
> In HS2's GetOperationStatus() and Beeswax's get_state(), the code looks like
> this:
> {code:java}
> void ImpalaServer::GetOperationStatus(TGetOperationStatusResp& return_val,
> const TGetOperationStatusReq& request) {
> ...
> status = GetActiveQueryHandle(query_id, &query_handle); <---- #1
> ...
> // When using long polling, this waits up to long_polling_time_ms
> milliseconds for
> // query completion.polling
> query_handle->WaitForCompletionExecState(); <---- #2
> ...
> {
> lock_guard<mutex> l(*query_handle->lock());
> TOperationState::type operation_state = query_handle->TOperationState();
> return_val.__set_operationState(operation_state); <---- #3
> if (operation_state == TOperationState::ERROR_STATE) {
> DCHECK(!query_handle->query_status().ok());
> return_val.__set_errorMessage(Substitute(QUERY_ERROR_FORMAT,
> PrintId(query_id), query_handle->query_status().GetDetail()));
> return_val.__set_sqlState(SQLSTATE_GENERAL_ERROR);
> } else {
> ClientRequestState::RetryState retry_state =
> query_handle->retry_state();
> if (retry_state != ClientRequestState::RetryState::RETRYING
> && retry_state != ClientRequestState::RetryState::RETRIED) {
> DCHECK(query_handle->query_status().ok());
> }
> }{code}
> If we get the active query handle in #1, then it fails and gets retried
> before #3, GetOperationStatus() will return an error even though the query is
> being retried. This is deterministic with long polling, because it waits for
> significant time at #2 and gets posted out when the query hits an error.
> However, this can happen without long polling, and it might be causing some
> flakiness for some of our retry tests.
> When we have the lock, if the query hit an error, we should check to see if
> it was retried. If it was retried, we can call GetActiveQueryHandle() to get
> the new handle and then return its status.
> The same basic issue exists for Beeswax as well. This is a blocker for
> enabling long polling by default.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
