Mihaly Szjatinya has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/22734 )
Change subject: WIP IMPALA-10268: Validate the debug actions when they are set ...................................................................... WIP 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 is sparsed between DebugOptions() constructor and 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 --- M be/src/exec/exec-node.cc M be/src/exec/exec-node.h M be/src/runtime/debug-options.cc M be/src/runtime/debug-options.h M be/src/service/query-options.cc M be/src/util/debug-util.cc M be/src/util/debug-util.h A tests/query_test/test_debug_action.py 8 files changed, 208 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/22734/3 -- To view, visit http://gerrit.cloudera.org:8080/22734 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I53816aba2c79b556688d3b916883fee7476fdbb5 Gerrit-Change-Number: 22734 Gerrit-PatchSet: 3 Gerrit-Owner: Mihaly Szjatinya <msz...@pm.me> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
