[ 
https://issues.apache.org/jira/browse/IMPALA-10268?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17947461#comment-17947461
 ] 

ASF subversion and git services commented on IMPALA-10268:
----------------------------------------------------------

Commit 5e39afc2d90a606ce05de376af5cd54035f7f99a in impala's branch 
refs/heads/master from Mihaly Szjatinya
[ https://gitbox.apache.org/repos/asf?p=impala.git;h=5e39afc2d ]

IMPALA-10268: Validate the debug actions when they are set

This patch aims to extract existing verifications on DEBUG_ACTION query
option format onto pre-planner stage SetQueryOption(), in order to
prevent failures on execution stage. Also, it localizes verification
code for two existing types of debug actions.

There are two types of debug actions, global e.g. 'RECVR_ADD_BATCH:FAIL'
and ExecNode debug actions, e.g. '0:GETNEXT:FAIL'. Two types are
implemented independently in source code, both having verification code
intertwined with execution. In addition, global debug actions subdivide
into C++ and Java, the two being more or less synchronized though.

In case of global debug actions, most of the code inside existing
DebugActionImpl() consists of verification, therefore it makes sense to
make a wrapper around it for separating out the execution code.

Things are worse for ExecNode debug actions, where verification code
consists of two parts, one in DebugOptions() constructor and another one
in ExecNode::ExecDebugActionImpl(). Additionally, some verification in
constructor produces warnings, while ExecDebugActionImpl() verification
either fails on DCHECK() or (in a single case) returns an error. For
this case, a reasonable solution seems to be simply calling the
constructor for a temporary object and extracting verification code from
ExecNode::ExecDebugActionImpl(). This has the drawback of having the
same warning being produced two times.

Finally, having extracted verification code for both types, logic in
impala::SetQueryOption() combines the two verification mechanisms.

Note: In the long run, it is better to write a single verification
routine for both Global and ExecNode debug actions, ideally as part of a
general unification of the two existing debug_action mechanisms. With
this in mind, the current patch intends to preserve current behavior,
while avoiding complex refactoring.

Change-Id: I53816aba2c79b556688d3b916883fee7476fdbb5
Reviewed-on: http://gerrit.cloudera.org:8080/22734
Reviewed-by: Impala Public Jenkins <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>


> Validate the debug actions when they are set
> --------------------------------------------
>
>                 Key: IMPALA-10268
>                 URL: https://issues.apache.org/jira/browse/IMPALA-10268
>             Project: IMPALA
>          Issue Type: Improvement
>          Components: Backend
>            Reporter: Vihang Karajgaonkar
>            Assignee: Mihaly Szjatinya
>            Priority: Minor
>              Labels: newbie
>
> Impala has a query option called {{debug_action}} which can be set to invoke 
> some debug code like adding a sleep statement to simulate delay. Currently 
> the implementation of this debug action also validates whether the action is 
> a correctly formatted as seen in 
> https://github.com/apache/impala/blob/198bbe280cb991f653eba18c3d49b35582608520/be/src/util/debug-util.cc#L355.
>  However, if the debug_action is not correctly set up, this code can throw 
> exceptions at places where it is invoked. It would be good to validate the 
> debug action when the query option is set so that a invalid string is not 
> passed to the debug action execution.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to