Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201611032259.uA3MxVdt010169@ip-10-146-233-104.ec2.internal>
Date: Thu, 3 Nov 2016 22:59:31 +0000
From: "Henry Robinson (Code Review)" <gerrit@cloudera.org>
To: impala-cr@cloudera.com, reviews@impala.incubator.apache.org
Reply-To: henry@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-3882=3A_Simplify_some_query_exec_state_locking=0A?=
In-Reply-To: <gerrit.1478199771000.I516357d2b5e9eb83e8209872cbe4c078c778a629@gerrit.cloudera.org>
References: <gerrit.1478199771000.I516357d2b5e9eb83e8209872cbe4c078c778a629@gerrit.cloudera.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.12.2
archived-at: Thu, 03 Nov 2016 22:59:38 -0000

Henry Robinson has uploaded a new patch set (#4).

Change subject: IMPALA-3882: Simplify some query exec state locking
......................................................................

IMPALA-3882: Simplify some query exec state locking

query_exec_state_map_lock_ is implicated in many deadlock paths, and is
heavily contended. This patch simplifies the usage of
query_exec_state_map_lock_ by requiring that it is a 'terminal' lock -
i.e. that it is incorrect to take any other locks while holding the map
lock.

The most significant change is to remove the code path from
GetQueryExecState() that could take QueryExecState::lock() *before*
yielding the query_exec_state_map_lock_. Instead, callers have to take
the exec state lock themselves.

Also change registration path so that QueryExecState::lock is not
held during planning. This prevents blocking e.g. web pages that need to
read QES data structures to produce their output.

A subtlety of the previous registration code path is that it forced
callers of get_metadata() rpcs to wait until registration and planning
was complete so that the query metadata was always available. This patch
replaces that logic with a promise that the RPC can block on, which is
triggered when metadata is available, rather than implicitly relying on
the locking protocol.

Also rename some QES methods to make it clearer what they do.

Change-Id: I516357d2b5e9eb83e8209872cbe4c078c778a629
---
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-exec-state.cc
M be/src/service/query-exec-state.h
7 files changed, 145 insertions(+), 151 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/4935/4
-- 
To view, visit http://gerrit.cloudera.org:8080/4935
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I516357d2b5e9eb83e8209872cbe4c078c778a629
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <henry@cloudera.com>