Hello Bharath Vissapragada, Matthew Jacobs, Tim Armstrong, Dan Hecht, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8070 to look at the new patch set (#10). Change subject: IMPALA-5908: Allow SET to unset modified query options. ...................................................................... IMPALA-5908: Allow SET to unset modified query options. The query 'SET <option>=""' will now unset an option within the session, reverting it to its default state. This change became necessary when "SET" started returning an empty string for unset options which don't have a default. The test infrastructure (impala_test_suite.py) resets options to what it thinks is its defaults, and, when this broke, some ASAN builds started to fail, presumably due to a timing issue with how we re-use connections between tests. Previously, SessionState copied over the default options from the server when the session was created and then mutated that. To support unsetting options at the session layer, this change keeps a pointer to the default server settings, keeps separately the mutations, and overlays the options each time they're requested. Similarly, for configuration overlays that happen per-query, the overlay is now done explicitly, because empty per-query overlay values (key=..., value="") now have no effect. Because "set key=''" is ambiguous between "set to the empty string" and "unset", it's now impossible to set to the empty string, at the session layer, an option that is configured at a previous layer. In practice, this is just debug_action and request_pool. debug_action is essentially an internal tool. For request_pool, this means that setting the default request_pool via impalad command line is now a bad idea, as it can't be cleared at a per-session level. For request_pool, the correct course of action for users is to use placement rules, and to have a default placement rule. Testing: * Added a simple test that triggered this side-effect without this code. Specifically, "impala-python infra/python/env/bin/py.test tests/metadata/test_set.py -s" with the modified set.test triggers. * Amended tests/custom_cluster/test_admission_controller.py; it was useful for testing these code paths. * Added cases to query-options-test to check behavior for both defaulted and non-defaulted values. * Added a custom cluster test that checks that overlays are working against * Ran an ASAN build where this was triggering previously. Change-Id: Ia8c383e68064f839cb5000118901dff77b4e5cb9 --- M be/src/service/client-request-state.cc M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-options-test.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M testdata/workloads/functional-query/queries/QueryTest/set.test M tests/common/impala_test_suite.py M tests/custom_cluster/test_admission_controller.py A tests/custom_cluster/test_set_and_unset.py M tests/hs2/hs2_test_suite.py 14 files changed, 266 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/8070/10 -- To view, visit http://gerrit.cloudera.org:8080/8070 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia8c383e68064f839cb5000118901dff77b4e5cb9 Gerrit-Change-Number: 8070 Gerrit-PatchSet: 10 Gerrit-Owner: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Jacobs <mjac...@apache.org> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>