[Impala-ASF-CR] IMPALA-7179: allow multiple scratch dirs per device=true by default
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10736 ) Change subject: IMPALA-7179: allow_multiple_scratch_dirs_per_device=true by default .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2688/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10736 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I23394c9949ae4cd0a21d7bb25551371b3198e76c Gerrit-Change-Number: 10736 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Sat, 16 Jun 2018 04:31:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7056: [DOCS] ALTER TABLE only affects the future inserts
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/10743 ) Change subject: IMPALA-7056: [DOCS] ALTER TABLE only affects the future inserts .. Patch Set 1: Hi Tim, Could you assign a dev for this review? Thank you! -- To view, visit http://gerrit.cloudera.org:8080/10743 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib07457a5e2a42f0d83f4da513f2aade779c07ffb Gerrit-Change-Number: 10743 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 16 Jun 2018 01:27:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7056: [DOCS] ALTER TABLE only affects the future inserts
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10743 Change subject: IMPALA-7056: [DOCS] ALTER TABLE only affects the future inserts .. IMPALA-7056: [DOCS] ALTER TABLE only affects the future inserts Change-Id: Ib07457a5e2a42f0d83f4da513f2aade779c07ffb --- M docs/topics/impala_alter_table.xml 1 file changed, 13 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/10743/1 -- To view, visit http://gerrit.cloudera.org:8080/10743 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib07457a5e2a42f0d83f4da513f2aade779c07ffb Gerrit-Change-Number: 10743 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] IMPALA-7169: Prevent HDFS from checkpointing trash until 3000 AD
Tianyi Wang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10742 Change subject: IMPALA-7169: Prevent HDFS from checkpointing trash until 3000 AD .. IMPALA-7169: Prevent HDFS from checkpointing trash until 3000 AD HDFS trash checkpointing renames files in the trash folder and breaks impala tests. Impala set the trash checkpointing interval to 1440 to try to postpone it for 24 hours. Unfortunately that told HDFS to do it when the UNIX time is a multiple of 1440 * 60 and it broke trash-related tests run around midnight in GMT. This patch sets the interval to 541728000 so that HDFS won't do the checkpointing until Jan 1st 3000, and HDFS will checkpoint every 1030 years after that. Change-Id: I9452f7e44c7679f86a947cd20115c078757223d8 --- M testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.tmpl 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/10742/1 -- To view, visit http://gerrit.cloudera.org:8080/10742 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I9452f7e44c7679f86a947cd20115c078757223d8 Gerrit-Change-Number: 10742 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang
[Impala-ASF-CR] IMPALA-6034: Add Cpu and scanned bytes limits per query
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/10415 )
Change subject: IMPALA-6034: Add Cpu and scanned bytes limits per query
..
Patch Set 6:
Following diff prevents it from crashing immediately
-
query_state->GetCoordinator()->AggregateBackendsResourceUsage(_resource_usage);
+ Coordinator* coord = query_state->GetCoordinator();
+ if (coord == nullptr) {
+queries_by_timestamp_.emplace(ExpirationEvent{
+now + 1000L, expiration_event->query_id,
ExpirationKind::RESOURCE_LIMIT});
+expiration_event = queries_by_timestamp_.erase(expiration_event);
+continue;
+ }
+ coord->AggregateBackendsResourceUsage(_resource_usage);
--
To view, visit http://gerrit.cloudera.org:8080/10415
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20
Gerrit-Change-Number: 10415
Gerrit-PatchSet: 6
Gerrit-Owner: Mostafa Mokhtar
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Mostafa Mokhtar
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Sat, 16 Jun 2018 00:37:14 +
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6034: Add Cpu and scanned bytes limits per query
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/10415 )
Change subject: IMPALA-6034: Add Cpu and scanned bytes limits per query
..
Patch Set 6:
(2 comments)
I ran a local stress test with avro/snappy and it crashed almost immediately:
(gdb) down
#9 0x030e0378 in impala::Coordinator::AggregateBackendsResourceUsage
(this=0x0,
query_resource_utilization=0x7f2f1ddc1270) at
be/src/runtime/coordinator.cc:823
823 for (BackendState* backend_state: backend_states_) {
(gdb) p this
$4 = (impala::Coordinator * const) 0x0
(gdb) bt
#0 0x7f2f8befb428 in __GI_raise (sig=sig@entry=6) at
../sysdeps/unix/sysv/linux/raise.c:54
#1 0x7f2f8befd02a in __GI_abort () at abort.c:89
#2 0x7f2f8ee372c9 in ?? () from
/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server/libjvm.so
#3 0x7f2f8efe7c77 in ?? () from
/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server/libjvm.so
#4 0x7f2f8ee405af in JVM_handle_linux_signal ()
from /usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server/libjvm.so
#5 0x7f2f8ee34408 in ?? () from
/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server/libjvm.so
#6
#7 0x030e5912 in
__gnu_cxx::__normal_iterator > >::__normal_iterator
(this=0x7f2f1ddc11c0,
__i=@0x10: )
at
/home/tarmstrong/Impala/incubator-impala/toolchain/gcc-4.9.2/include/c++/4.9.2/bits/stl_iterator.h:729
#8 0x030e3e59 in std::vector >::begin (this=0x10)
at
/home/tarmstrong/Impala/incubator-impala/toolchain/gcc-4.9.2/include/c++/4.9.2/bits/stl_vector.h:548
#9 0x030e0378 in impala::Coordinator::AggregateBackendsResourceUsage
(this=0x0,
query_resource_utilization=0x7f2f1ddc1270) at
be/src/runtime/coordinator.cc:823
#10 0x01e0210f in impala::ImpalaServer::ExpireQueries (this=0xc611800)
at be/src/service/impala-server.cc:2001
#11 0x01e322c5 in boost::_mfi::mf0::operator() (this=0x7f2f1ddc1ce8, p=0xc611800)
at
/opt/Impala-Toolchain/boost-1.57.0-p3/include/boost/bind/mem_fn_template.hpp:49
#12 0x01e3095c in
boost::_bi::list1
>::operator(), boost::_bi::list0>
(this=0x7f2f1ddc1cf8, f=..., a=...)
at /opt/Impala-Toolchain/boost-1.57.0-p3/include/boost/bind/bind.hpp:253
#13 0x01e2de65 in boost::_bi::bind_t,
boost::_bi::list1 > >::operator()
(this=0x7f2f1ddc1ce8)
at
/opt/Impala-Toolchain/boost-1.57.0-p3/include/boost/bind/bind_template.hpp:20
#14 0x01e2a6e7 in
boost::detail::function::void_function_obj_invoker0,
boost::_bi::list1 > >, void>::invoke (
function_obj_ptr=...) at
/opt/Impala-Toolchain/boost-1.57.0-p3/include/boost/function/function_template.hpp:153
#15 0x01b92aca in boost::function0::operator()
(this=0x7f2f1ddc1ce0)
at
/opt/Impala-Toolchain/boost-1.57.0-p3/include/boost/function/function_template.hpp:767
#16 0x01f865e9 in impala::Thread::SuperviseThread(std::string const&,
std::string const&, boost::function, impala::ThreadDebugInfo const*,
impala::Promise*) (name="query-expirer",
category="impala-server", functor=..., parent_thread_info=0x0,
thread_started=0x7fff1e696170)
at be/src/util/thread.cc:356
#17 0x01f8e6c1 in boost::_bi::list5,
boost::_bi::value, boost::_bi::value >,
boost::_bi::value,
boost::_bi::value*>
>::operator(), impala::ThreadDebugInfo const*, impala::Promise*), boost::_bi::list0>(boost::_bi::type, void
(*&)(std::string const&, std::string const&, boost::function,
impala::ThreadDebugInfo const*, impala::Promise*), boost::_bi::list0&, int) (this=0xe6561c0,
f=@0xe6561b8: 0x1f86282 , impala::ThreadDebugInfo
const*, impala::Promise*)>, a=...)
at /opt/Impala-Toolchain/boost-1.57.0-p3/include/boost/bind/bind.hpp:525
#18 0x01f8e5e5 in boost::_bi::bind_t, impala::ThreadDebugInfo const*,
impala::Promise*),
boost::_bi::list5,
boost::_bi::value, boost::_bi::value >,
boost::_bi::value,
boost::_bi::value*> >
>::operator()() (
this=0xe6561b8) at
/opt/Impala-Toolchain/boost-1.57.0-p3/include/boost/bind/bind_template.hpp:20
#19 0x01f8e5a8 in boost::detail::thread_data,
impala::ThreadDebugInfo const*, impala::Promise*), boost::_bi::list5,
boost::_bi::value, boost::_bi::value >,
boost::_bi::value,
boost::_bi::value*> > > >::run()
(this=0xe656000) at
/opt/Impala-Toolchain/boost-1.57.0-p3/include/boost/thread/detail/thread.hpp:116
#20 0x031db81a in thread_proxy ()
#21 0x7f2f8c2976ba in start_thread (arg=0x7f2f1ddc2700) at
pthread_create.c:333
#22 0x7f2f8bfcd41d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:109
http://gerrit.cloudera.org:8080/#/c/10415/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10415/6//COMMIT_MSG@18
PS6, Line 18: Added tests for various permutations for MAX_CPU_TIME_S and
MAX_SCAN_BYTES
Something I noticed after playing around: How about we rename this to have a
_limit suffix for consistency with other
[Impala-ASF-CR] IMPALA-7171: [DOCS] Hints for Kudu insert and upsert
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10737 Change subject: IMPALA-7171: [DOCS] Hints for Kudu insert and upsert .. IMPALA-7171: [DOCS] Hints for Kudu insert and upsert Change-Id: I04378e6f2b17d4d6e844192807d946b9045e2927 --- M docs/shared/impala_common.xml M docs/topics/impala_hints.xml M docs/topics/impala_kudu.xml 3 files changed, 31 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/10737/1 -- To view, visit http://gerrit.cloudera.org:8080/10737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I04378e6f2b17d4d6e844192807d946b9045e2927 Gerrit-Change-Number: 10737 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] IMPALA-7046: introduce "global" debug actions
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10690 ) Change subject: IMPALA-7046: introduce "global" debug_actions .. Patch Set 6: (1 comment) This is a rebase but required fix up to remove the NDEBUG and do the empty() check like Tim added to the old code. http://gerrit.cloudera.org:8080/#/c/10690/6/be/src/util/debug-util.h File be/src/util/debug-util.h: http://gerrit.cloudera.org:8080/#/c/10690/6/be/src/util/debug-util.h@129 PS6, Line 129: WARN_UNUSED_RESULT gcc complains if this is at the end for a function definition, for some reason. So I put it here. -- To view, visit http://gerrit.cloudera.org:8080/10690 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I77663a539be18711a4f12c470ffd7474e3d69388 Gerrit-Change-Number: 10690 Gerrit-PatchSet: 6 Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Comment-Date: Sat, 16 Jun 2018 00:04:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7046: introduce "global" debug actions
Hello Bikramjeet Vig, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10690 to look at the new patch set (#6). Change subject: IMPALA-7046: introduce "global" debug_actions .. IMPALA-7046: introduce "global" debug_actions The motivation is to add jitter to backend startup in test_failpoints. The race in IMPALA-7033 can be reproduced by adding jitter to the exec rpcs when some backends fail. Let's add jitter to test_failpoints to get better coverage of exec startup races. This builds on top of the debug action extensions added in the async admission control patch by allowing the new "global" debug actions (i.e. actions that can be used in points outside of the ExecNodes). See the code comments for details. For now, we're only using the SLEEP and JITTER commands, but I've included a FAIL command as well since I'll want to use that to write a test for IMPALA-6788 to simulate exec rpc failure. Note that I don't bother resolving the actions ahead of time (like we do for ExecNode actions). It doesn't seem worth it since the resolution only needs to occur after we've matched the label and I don't expect the same label to be hit many times within a single thread. We can always optimize this later if needed. Instead, just continue compiling it out of Release builds. Testing: - Verified that test_failpoints can reproduce the race in IMPALA-7033 by reverting that fix and testing. - Ran the modified tests and grepped the impalad log to see that the sleeps are still occuring. - Manually verify global FAIL command (in a build with another patch). Change-Id: I77663a539be18711a4f12c470ffd7474e3d69388 --- M be/src/runtime/coordinator.cc M be/src/runtime/debug-options.cc M be/src/scheduling/admission-controller.cc M be/src/service/client-request-state.cc M be/src/util/debug-util.cc M be/src/util/debug-util.h M common/thrift/ImpalaService.thrift M tests/custom_cluster/test_admission_controller.py M tests/failure/test_failpoints.py M tests/query_test/test_observability.py 10 files changed, 194 insertions(+), 69 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/10690/6 -- To view, visit http://gerrit.cloudera.org:8080/10690 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I77663a539be18711a4f12c470ffd7474e3d69388 Gerrit-Change-Number: 10690 Gerrit-PatchSet: 6 Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Bikramjeet Vig
[Impala-ASF-CR] IMPALA-110 (part 1): Refactor ExecNode::buffer pool client
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10493 ) Change subject: IMPALA-110 (part 1): Refactor ExecNode::buffer_pool_client_ .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/10493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I75f92c3f4f05adeef11a70f59e0c8ff2d19bc17a Gerrit-Change-Number: 10493 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 15 Jun 2018 23:58:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-110 (part 2): Refactor PartitionedAggregationNode
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10394 ) Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode .. Patch Set 2: > (14 comments) > > While developing this patch, I maintained a branch with a series of > commits separating the work out into several mostly mechanical > steps to keep everything straight. > Does each step function on it's own? it might be easier to just review and commit each step individually (or at least some grouping of the steps) so that big mechanical changes are easier to review. -- To view, visit http://gerrit.cloudera.org:8080/10394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e Gerrit-Change-Number: 10394 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 23:56:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6034: Add Cpu and scanned bytes limits per query
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/10415 )
Change subject: IMPALA-6034: Add Cpu and scanned bytes limits per query
..
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10415/6/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/10415/6/be/src/service/impala-server.cc@1978
PS6, Line 1978: // Query didn't cross the resource limits, move to
the end of the queue
We still need to erase the event - now we have two copies of it so we've end up
with exponential growth.
http://gerrit.cloudera.org:8080/#/c/10415/6/be/src/service/impala-server.cc@1979
PS6, Line 1979: queries_by_timestamp_.emplace(ExpirationEvent{
nit: tabs
--
To view, visit http://gerrit.cloudera.org:8080/10415
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20
Gerrit-Change-Number: 10415
Gerrit-PatchSet: 6
Gerrit-Owner: Mostafa Mokhtar
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Mostafa Mokhtar
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 15 Jun 2018 23:53:33 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7179: allow multiple scratch dirs per device=true by default
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10736 ) Change subject: IMPALA-7179: allow_multiple_scratch_dirs_per_device=true by default .. Patch Set 2: ok -- To view, visit http://gerrit.cloudera.org:8080/10736 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I23394c9949ae4cd0a21d7bb25551371b3198e76c Gerrit-Change-Number: 10736 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Fri, 15 Jun 2018 23:47:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-110 (part 1): Refactor ExecNode::buffer pool client
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10493 ) Change subject: IMPALA-110 (part 1): Refactor ExecNode::buffer_pool_client_ .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/10493/2/be/src/runtime/reservation-manager.h File be/src/runtime/reservation-manager.h: http://gerrit.cloudera.org:8080/#/c/10493/2/be/src/runtime/reservation-manager.h@33 PS2, Line 33: which with? http://gerrit.cloudera.org:8080/#/c/10493/2/be/src/runtime/reservation-manager.h@37 PS2, Line 37: static void Register(ReservationManager* rm, std::string name, please document that http://gerrit.cloudera.org:8080/#/c/10493/2/be/src/runtime/reservation-manager.h@39 PS2, Line 39: TBackendResourceProfile resource_profile, const TDebugOptions you probably meant to use references here. http://gerrit.cloudera.org:8080/#/c/10493/2/be/src/runtime/reservation-manager.cc File be/src/runtime/reservation-manager.cc: http://gerrit.cloudera.org:8080/#/c/10493/2/be/src/runtime/reservation-manager.cc@31 PS2, Line 31: Register this looks more like an Init() function (and doesn't have to be static given the first parameters is effectively 'this'). -- To view, visit http://gerrit.cloudera.org:8080/10493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I75f92c3f4f05adeef11a70f59e0c8ff2d19bc17a Gerrit-Change-Number: 10493 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 15 Jun 2018 23:47:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7179: allow multiple scratch dirs per device=true by default
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10736 ) Change subject: IMPALA-7179: allow_multiple_scratch_dirs_per_device=true by default .. Patch Set 2: I couldn't think of anything that surprising - worst case performance of spilling might get worse because we don't load-balance across disks as well. -- To view, visit http://gerrit.cloudera.org:8080/10736 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I23394c9949ae4cd0a21d7bb25551371b3198e76c Gerrit-Change-Number: 10736 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Fri, 15 Jun 2018 23:41:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7179: allow multiple scratch dirs per device=true by default
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10736 ) Change subject: IMPALA-7179: allow_multiple_scratch_dirs_per_device=true by default .. Patch Set 2: Code-Review+2 Seems better. Is there any weirdness/surprise that can happen with this change in default that we should document? -- To view, visit http://gerrit.cloudera.org:8080/10736 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I23394c9949ae4cd0a21d7bb25551371b3198e76c Gerrit-Change-Number: 10736 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Fri, 15 Jun 2018 23:38:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-110 (part 1): Refactor ExecNode::buffer pool client
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10493 ) Change subject: IMPALA-110 (part 1): Refactor ExecNode::buffer_pool_client_ .. Patch Set 2: Code-Review+2 Thanks. I think this makes more sense than having them all as members directly in ExecNode. -- To view, visit http://gerrit.cloudera.org:8080/10493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I75f92c3f4f05adeef11a70f59e0c8ff2d19bc17a Gerrit-Change-Number: 10493 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 15 Jun 2018 23:29:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7179: allow multiple scratch dirs per device=true by default
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10736 ) Change subject: IMPALA-7179: allow_multiple_scratch_dirs_per_device=true by default .. Patch Set 2: Anuj pointed out that I'd forgot to include the critical part of the change (some git add mistake) - thanks Anuj! -- To view, visit http://gerrit.cloudera.org:8080/10736 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I23394c9949ae4cd0a21d7bb25551371b3198e76c Gerrit-Change-Number: 10736 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Fri, 15 Jun 2018 23:26:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7179: allow multiple scratch dirs per device=true by default
anujphadke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10736 ) Change subject: IMPALA-7179: allow_multiple_scratch_dirs_per_device=true by default .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/10736 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I23394c9949ae4cd0a21d7bb25551371b3198e76c Gerrit-Change-Number: 10736 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Fri, 15 Jun 2018 23:25:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7179: allow multiple scratch dirs per device=true by default
Tim Armstrong has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/10736 ) Change subject: IMPALA-7179: allow_multiple_scratch_dirs_per_device=true by default .. IMPALA-7179: allow_multiple_scratch_dirs_per_device=true by default The previous default was often confusing to users of Impala. It is simpler to do exactly what is asked instead of trying to fix bad configurations automatically. Testing: Ran core tests. Change-Id: I23394c9949ae4cd0a21d7bb25551371b3198e76c --- M be/src/runtime/tmp-file-mgr.cc M tests/custom_cluster/test_scratch_disk.py 2 files changed, 3 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/10736/2 -- To view, visit http://gerrit.cloudera.org:8080/10736 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I23394c9949ae4cd0a21d7bb25551371b3198e76c Gerrit-Change-Number: 10736 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-2746: part 1: enable LSAN for many backend tests
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/10668 ) Change subject: IMPALA-2746: part 1: enable LSAN for many backend tests .. Patch Set 9: Code-Review+2 (1 comment) This all looks good. It's great to get this enabled. http://gerrit.cloudera.org:8080/#/c/10668/8/be/src/runtime/CMakeLists.txt File be/src/runtime/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/10668/8/be/src/runtime/CMakeLists.txt@83 PS8, Line 83: ADD_BE_TEST(data-stream-test) # TODO: this test leaks > I fixed a couple of easy ones; the hard ones are around the lifecycles of t Ok, cool -- To view, visit http://gerrit.cloudera.org:8080/10668 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibdda092a4eb4bc827c75a8c121e5428ec746b7f4 Gerrit-Change-Number: 10668 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 15 Jun 2018 23:20:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2746: part 1: enable LSAN for many backend tests
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10668 ) Change subject: IMPALA-2746: part 1: enable LSAN for many backend tests .. Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/10668/8/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/10668/8/be/src/exprs/expr-test.cc@7387 PS8, Line 7387: ObjectPool pool; > I think this is now unused (and I think clang-tidy might complain). Good catch. I think the constructor is non-trivial so it can't infer that this does nothing. http://gerrit.cloudera.org:8080/#/c/10668/8/be/src/runtime/CMakeLists.txt File be/src/runtime/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/10668/8/be/src/runtime/CMakeLists.txt@83 PS8, Line 83: ADD_BE_TEST(data-stream-test) # TODO: this test leaks > Is this intended? (i.e. this code change fixes some leaks in data-stream-te I fixed a couple of easy ones; the hard ones are around the lifecycles of the various servers. -- To view, visit http://gerrit.cloudera.org:8080/10668 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibdda092a4eb4bc827c75a8c121e5428ec746b7f4 Gerrit-Change-Number: 10668 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 15 Jun 2018 23:17:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-110 (part 2): Refactor PartitionedAggregationNode
Hello Tim Armstrong, Alex Behm, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10394 to look at the new patch set (#2). Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode .. IMPALA-110 (part 2): Refactor PartitionedAggregationNode This patch refactors PartitionedAggregationNode in preparation for supporting multiple distinct operators in a query. The primary goal of the refactor is to separate out the core aggregation functionality into a new type of object called an Aggregator. For now, each aggregation ExecNode will contain a single Aggregator. Then, future patches will extend the aggregation ExecNode to support taking a single input and processing it with multiple Aggregators, allowing us to support more exotic combinations of aggregate functions and groupings. Specifically, this patch splits PartitionedAggregationNode into five new classes: - Aggregator: a superclass containing the functionality that's shared between GroupingAggregator and NonGroupingAggregator. - GroupingAggregator: this class contains the bulk of the interesting aggregation code, including everything related to creating and updating partitions and hash tables, spilling, etc. - NonGroupingAggregator: this class handles the case of aggregations that don't have grouping exprs. Since these aggregations always result in just a single output row, the functionality here is relatively simple (eg. no spilling or streaming). - StreamingAggregationNode: this node performs a streaming preaggregation, where the input is retrieved from the child during GetNext() and passed to the GroupingAggregator (non-grouping do not support streaming) Eventually, we'll support a list of GroupingAggregators. - AggregationNode: this node performs a final aggregation, where the input is retrieved from the child during Open() and passed to the Aggregator. Currently the Aggregator can be either grouping or non-grouping. Eventually we'll support a list of GroupingAggregator and/or a single NonGroupingAggregator. Testing: - Ran the existing aggregation tests. Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e --- M be/src/codegen/gen_ir_descriptions.py M be/src/codegen/impala-ir.cc M be/src/common/global-flags.cc M be/src/exec/CMakeLists.txt A be/src/exec/aggregation-node.cc A be/src/exec/aggregation-node.h A be/src/exec/aggregator.cc A be/src/exec/aggregator.h M be/src/exec/exec-node.cc M be/src/exec/exec-node.h A be/src/exec/grouping-aggregator-ir.cc A be/src/exec/grouping-aggregator-partition.cc A be/src/exec/grouping-aggregator.cc A be/src/exec/grouping-aggregator.h A be/src/exec/non-grouping-aggregator-ir.cc A be/src/exec/non-grouping-aggregator.cc A be/src/exec/non-grouping-aggregator.h A be/src/exec/streaming-aggregation-node.cc A be/src/exec/streaming-aggregation-node.h 19 files changed, 3,763 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/10394/2 -- To view, visit http://gerrit.cloudera.org:8080/10394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e Gerrit-Change-Number: 10394 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-110 (part 1): Refactor ExecNode::buffer pool client
Hello Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10493 to look at the new patch set (#2). Change subject: IMPALA-110 (part 1): Refactor ExecNode::buffer_pool_client_ .. IMPALA-110 (part 1): Refactor ExecNode::buffer_pool_client_ IMPALA-110 will involve refactoring PartitionedAggregationNode by separating out the aggregation logic into a new type called Aggregator, and then supporting multiple Aggregators per node to allow for multiple aggregation classes to be evaluated at the same time. Each Aggregator will need to have its own memory reservation to operate, and we can do this by giving each Aggregator its own BufferPool::ClientHandle instead of using the usual ExecNode::buffer_pool_client_. To facilitate this, this patch refactors all of the buffer_pool_client_ related logic into a new class, ReservationManager, so that eventually each Aggregator can have its own ReservationManager and the logic in ClaimBufferReservation(), ReleaseUnusedReservation(), etc. won't be duplicated. Change-Id: I75f92c3f4f05adeef11a70f59e0c8ff2d19bc17a --- M be/src/exec/analytic-eval-node.cc M be/src/exec/exec-node.cc M be/src/exec/exec-node.h M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/hdfs-scan-node-mt.cc M be/src/exec/hdfs-scan-node.cc M be/src/exec/partial-sort-node.cc M be/src/exec/partitioned-aggregation-node.cc M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/sort-node.cc M be/src/runtime/CMakeLists.txt A be/src/runtime/reservation-manager.cc A be/src/runtime/reservation-manager.h 13 files changed, 284 insertions(+), 164 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/10493/2 -- To view, visit http://gerrit.cloudera.org:8080/10493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I75f92c3f4f05adeef11a70f59e0c8ff2d19bc17a Gerrit-Change-Number: 10493 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7161: Fix impala-config.sh's handling of JAVA HOME
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10702 ) Change subject: IMPALA-7161: Fix impala-config.sh's handling of JAVA_HOME .. Patch Set 3: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2687/ -- To view, visit http://gerrit.cloudera.org:8080/10702 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf3521b4f44fdbdc841a90fd00c477c9423a75bb Gerrit-Change-Number: 10702 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Comment-Date: Fri, 15 Jun 2018 21:59:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7179: allow multiple scratch dirs per device=true by default
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10736 Change subject: IMPALA-7179: allow_multiple_scratch_dirs_per_device=true by default .. IMPALA-7179: allow_multiple_scratch_dirs_per_device=true by default The previous default was often confusing to users of Impala. It is simpler to do exactly what is asked instead of trying to fix bad configurations automatically. Testing: Ran core tests. Change-Id: I23394c9949ae4cd0a21d7bb25551371b3198e76c --- M tests/custom_cluster/test_scratch_disk.py 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/10736/1 -- To view, visit http://gerrit.cloudera.org:8080/10736 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I23394c9949ae4cd0a21d7bb25551371b3198e76c Gerrit-Change-Number: 10736 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 21:37:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 4): support creating descriptors for FS tables
Hello Tianyi Wang, Vuk Ercegovac, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/10735 to review the following change. Change subject: IMPALA-7140 (part 4): support creating descriptors for FS tables .. IMPALA-7140 (part 4): support creating descriptors for FS tables This adds the relevant methods to convert LocalFsTable and LocalFsPartition to thrift descriptors for consumption by the backend. Unfortunately we cannot yet enable the planner tests, since they are checking file counts and sizes as part of the explain output, and we haven't yet implemented file info fetching in the LocalCatalog. However, I was able to manually test this change by starting an impalad with --use_local_catalog, connecting to it from the shell, and running various EXPLAIN SELECT queries against tpch and functional tables. The explain output is more or less as expected with the exception of missing file info. Change-Id: I4550612eb6d1e3a324f49a9c4d24b048e45d3738 --- M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java 10 files changed, 223 insertions(+), 105 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/10735/1 -- To view, visit http://gerrit.cloudera.org:8080/10735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4550612eb6d1e3a324f49a9c4d24b048e45d3738 Gerrit-Change-Number: 10735 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7140 (part 3): load partitions for FS tables
Hello Tianyi Wang, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10713 to look at the new patch set (#3). Change subject: IMPALA-7140 (part 3): load partitions for FS tables .. IMPALA-7140 (part 3): load partitions for FS tables This implements the loading of partitions from the HMS for FS-based tables. The LocalPartitionSpec class implements PrunablePartition based on parsing the partition names returned from the HMS. After partitions are pruned, the remaining partitions can be loaded by name. With this patch, I am able to connect to an impalad running with --use_local_catalog and issue 'show partitions functional.alltypes' with the expected results. A new unit test is added which shares some code with CatalogTest to ensure that partitions can be loaded and parsed properly. Testing partition pruning via end-to-end planning is not quite supported yet: we need to implement 'toThriftDescriptor()' before we can run those end-to-end tests. Change-Id: Iddf2edbd6bdc0684560b2ecca9c4c6b6819ef1d3 --- M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/PartitionStatsUtil.java M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java A fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java A fe/src/main/java/org/apache/impala/catalog/local/LocalPartitionSpec.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java 14 files changed, 706 insertions(+), 108 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/10713/3 -- To view, visit http://gerrit.cloudera.org:8080/10713 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddf2edbd6bdc0684560b2ecca9c4c6b6819ef1d3 Gerrit-Change-Number: 10713 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7140 (part 3): load partitions for FS tables
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10713 ) Change subject: IMPALA-7140 (part 3): load partitions for FS tables .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@132 PS2, Line 132: Warehouse.makeSpecFromName > Makes sense to push down the column names in this api and agree that keepin Done http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@148 PS2, Line 148: name > yes, would be an annoying thing to track down if differences leak in with h Done. Also checked that HMS doesn't ever return the same partition twice. Added a unit test which checks one of the tables that uses the __HIVE_DEFAULT_PARTITION__ thing, too. http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java: http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@183 PS2, Line 183: List names = Lists.newArrayList(); > yup, but its a public method, so good to be explicit. Done http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalPartitionSpec.java File fe/src/main/java/org/apache/impala/catalog/local/LocalPartitionSpec.java: http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalPartitionSpec.java@51 PS2, Line 51: getPartValuesFromPartName > Will check this out. OK, I was able to make use of that. -- To view, visit http://gerrit.cloudera.org:8080/10713 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iddf2edbd6bdc0684560b2ecca9c4c6b6819ef1d3 Gerrit-Change-Number: 10713 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 20:39:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6812: Fix flaky Kudu scan tests
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/10503 ) Change subject: IMPALA-6812: Fix flaky Kudu scan tests .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/10503/4/be/src/service/query-options.h File be/src/service/query-options.h: http://gerrit.cloudera.org:8080/#/c/10503/4/be/src/service/query-options.h@140 PS4, Line 140: ADVANCED > so this will be documented and expected to work in some reasonable way? if Yes, I'll make sure it gets documented. It should work exactly as expected, eg. allow users to set a scan consistency level on a per-query/session basis. In fact, even beyond addressing test flakiness I think this is a good feature to support since users may want different consistency levels for different workloads running in the same cluster, which we didn't previously support. -- To view, visit http://gerrit.cloudera.org:8080/10503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I70df84f2cbc663107f2ad029565d3c15bdfbd47c Gerrit-Change-Number: 10503 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 15 Jun 2018 20:28:01 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6812: Fix flaky Kudu scan tests
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10503 ) Change subject: IMPALA-6812: Fix flaky Kudu scan tests .. Patch Set 4: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/10503/4/be/src/service/query-options.h File be/src/service/query-options.h: http://gerrit.cloudera.org:8080/#/c/10503/4/be/src/service/query-options.h@140 PS4, Line 140: ADVANCED so this will be documented and expected to work in some reasonable way? if not, maybe we should make it DEVELOPMENT. -- To view, visit http://gerrit.cloudera.org:8080/10503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I70df84f2cbc663107f2ad029565d3c15bdfbd47c Gerrit-Change-Number: 10503 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 15 Jun 2018 20:22:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Optimize dependencies for Codegen
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/10688 ) Change subject: Optimize dependencies for Codegen .. Patch Set 1: > Looks like the current way was done as a result of IMPALA-1896. > Could you check that JIRA to see if removing this will cause that > problem? This has that problem. I am abandoning this change. -- To view, visit http://gerrit.cloudera.org:8080/10688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3294cd27c2d35388a04934440a1d2b0ba3a0dd9 Gerrit-Change-Number: 10688 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Fri, 15 Jun 2018 20:07:58 + Gerrit-HasComments: No
[Impala-ASF-CR] Optimize dependencies for Codegen
Joe McDonnell has abandoned this change. ( http://gerrit.cloudera.org:8080/10688 ) Change subject: Optimize dependencies for Codegen .. Abandoned This has a flawed dependency graph. -- To view, visit http://gerrit.cloudera.org:8080/10688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: Ie3294cd27c2d35388a04934440a1d2b0ba3a0dd9 Gerrit-Change-Number: 10688 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] [DOCS] Fixed the DITA concept ID for the new impala disable codegen rows threshold
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10733 ) Change subject: [DOCS] Fixed the DITA concept ID for the new impala_disable_codegen_rows_threshold .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8ecbf93749d5eb8da1d5c873fd7503f5bb2c7d0f Gerrit-Change-Number: 10733 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 15 Jun 2018 19:47:44 + Gerrit-HasComments: No
[Impala-ASF-CR] [DOCS] Fixed the DITA concept ID for the new impala disable codegen rows threshold
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10733 ) Change subject: [DOCS] Fixed the DITA concept ID for the new impala_disable_codegen_rows_threshold .. [DOCS] Fixed the DITA concept ID for the new impala_disable_codegen_rows_threshold Change-Id: I8ecbf93749d5eb8da1d5c873fd7503f5bb2c7d0f Reviewed-on: http://gerrit.cloudera.org:8080/10733 Reviewed-by: Alex Rodoni Tested-by: Impala Public Jenkins --- M docs/topics/impala_disable_codegen_rows_threshold.xml 1 file changed, 9 insertions(+), 11 deletions(-) Approvals: Alex Rodoni: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8ecbf93749d5eb8da1d5c873fd7503f5bb2c7d0f Gerrit-Change-Number: 10733 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-2746: part 1: enable LSAN for many backend tests
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/10668 ) Change subject: IMPALA-2746: part 1: enable LSAN for many backend tests .. Patch Set 8: (2 comments) I'm basically ready to +2 this. Just a couple things. http://gerrit.cloudera.org:8080/#/c/10668/8/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/10668/8/be/src/exprs/expr-test.cc@7387 PS8, Line 7387: ObjectPool pool; I think this is now unused (and I think clang-tidy might complain). http://gerrit.cloudera.org:8080/#/c/10668/8/be/src/runtime/CMakeLists.txt File be/src/runtime/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/10668/8/be/src/runtime/CMakeLists.txt@83 PS8, Line 83: ADD_BE_TEST(data-stream-test) # TODO: this test leaks Is this intended? (i.e. this code change fixes some leaks in data-stream-test.cc, are there more that aren't fixed?) -- To view, visit http://gerrit.cloudera.org:8080/10668 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibdda092a4eb4bc827c75a8c121e5428ec746b7f4 Gerrit-Change-Number: 10668 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 15 Jun 2018 19:46:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [DOCS] Fixed the DITA concept ID for the new impala disable codegen rows threshold
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10733 ) Change subject: [DOCS] Fixed the DITA concept ID for the new impala_disable_codegen_rows_threshold .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-docs-submit/320/ -- To view, visit http://gerrit.cloudera.org:8080/10733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8ecbf93749d5eb8da1d5c873fd7503f5bb2c7d0f Gerrit-Change-Number: 10733 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 15 Jun 2018 19:39:08 + Gerrit-HasComments: No
[Impala-ASF-CR] [DOCS] Fixed the DITA concept ID for the new impala disable codegen rows threshold
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/10733 ) Change subject: [DOCS] Fixed the DITA concept ID for the new impala_disable_codegen_rows_threshold .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8ecbf93749d5eb8da1d5c873fd7503f5bb2c7d0f Gerrit-Change-Number: 10733 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Comment-Date: Fri, 15 Jun 2018 19:38:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 3): load partitions for FS tables
Vuk Ercegovac has posted comments on this change. (
http://gerrit.cloudera.org:8080/10713 )
Change subject: IMPALA-7140 (part 3): load partitions for FS tables
..
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@132
PS2, Line 132: Warehouse.makeSpecFromName
> Good point.
Makes sense to push down the column names in this api and agree that keeping
these abstractions separate is preferable.
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@148
PS2, Line 148: name
> Yea, it should. Do you think it's worth adding an assertion here that ret.k
yes, would be an annoying thing to track down if differences leak in with how
these names are constructed.
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java:
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@183
PS2, Line 183: List names = Lists.newArrayList();
> ah, I'll add a Preconditions.checkState here that the partition specs are l
yup, but its a public method, so good to be explicit.
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@279
PS2, Line 279: if (getNumClusteringCols() == 0) {
> I can see the appeal but seems like almost all the rest of the code is the
that's fine
--
To view, visit http://gerrit.cloudera.org:8080/10713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iddf2edbd6bdc0684560b2ecca9c4c6b6819ef1d3
Gerrit-Change-Number: 10713
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Fri, 15 Jun 2018 19:38:48 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] [DOCS] Fixed the DITA concept ID for the new impala disable codegen rows threshold
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10733 Change subject: [DOCS] Fixed the DITA concept ID for the new impala_disable_codegen_rows_threshold .. [DOCS] Fixed the DITA concept ID for the new impala_disable_codegen_rows_threshold Change-Id: I8ecbf93749d5eb8da1d5c873fd7503f5bb2c7d0f --- M docs/topics/impala_disable_codegen_rows_threshold.xml 1 file changed, 9 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/10733/1 -- To view, visit http://gerrit.cloudera.org:8080/10733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8ecbf93749d5eb8da1d5c873fd7503f5bb2c7d0f Gerrit-Change-Number: 10733 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 5 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 18:43:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7161: Fix impala-config.sh's handling of JAVA HOME
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10702 ) Change subject: IMPALA-7161: Fix impala-config.sh's handling of JAVA_HOME .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2687/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/10702 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf3521b4f44fdbdc841a90fd00c477c9423a75bb Gerrit-Change-Number: 10702 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Comment-Date: Fri, 15 Jun 2018 18:35:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2686/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 18:14:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 2). Create skeleton for LocalFsTable
Hello Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10712 to look at the new patch set (#3). Change subject: IMPALA-7140 (part 2). Create skeleton for LocalFsTable .. IMPALA-7140 (part 2). Create skeleton for LocalFsTable This adds a skeleton implementation of FeFsTable for the LocalCatalog. Most of the implementation in this patch is stubbed out -- only the bare bones necessary to instantiate the subclass when a table is loaded and the simplest table-level methods. Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8 --- M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalogException.java A fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java 10 files changed, 278 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/10712/3 -- To view, visit http://gerrit.cloudera.org:8080/10712 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8 Gerrit-Change-Number: 10712 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Hello Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10629 to look at the new patch set (#7). Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. IMPALA-7137. Support configuring Frontend to use LocalCatalog This adds a new flag -use_local_catalog which is passed through to the frontend and causes it to use LocalCatalog instead of ImpaladCatalog. Additionally, when this flag is configured, the impalad does not subscribe to catalog topic updates from the statestore. The patch is slightly more complex than simply picking which class to instantiate, because the lifecycle is designed a bit differently between the two implementations: - LocalCatalog is instantiated once per query/request. - ImpaladCatalog is instantiated once and stateful across queries, except when a full catalog update is received. This maintains the current behavior for this implementation. In order to abstract this difference in lifecycle from the frontend, I introduced a new FeCatalogManager class with different implementations for the two lifecycles. I also had to add a simple test implementation since some tests rely on directly injecting a Catalog implementation into the Frontend. This patch also includes a few small changes to the local catalog implementation objects to enable the impalad to start and accept connections. With this patch, I was able to manually test as follows: I started just the statestore and the impalad in the new mode: - ./bin/start-statestored.sh - ./bin/start-impalad.sh --use_local_catalog I connected with impala-shell as usual and was able to run the most simple queries: - SHOW DATABASES; - USE functional; - SHOW TABLES; All other functionality results in error messages due to the various TODOs in the current skeleton implementation. Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 --- M be/src/runtime/exec-env.cc M be/src/service/impala-server.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java A fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/main/java/org/apache/impala/service/MetadataOp.java 11 files changed, 241 insertions(+), 64 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/10629/7 -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7141 (part 1): clean up handling of default/dummy partition
Hello Gabor Kaszab, Zoltan Borok-Nagy, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10711 to look at the new patch set (#3). Change subject: IMPALA-7141 (part 1): clean up handling of default/dummy partition .. IMPALA-7141 (part 1): clean up handling of default/dummy partition Currently, HdfsTable inconsistently uses the term "default partition" to refer to two different concepts: 1) For unpartitioned tables, a single partition with ID 1 and no partition keys is created and added to the partition map. 2) All tables have an additional partition added with partition ID -1 which acts as a sort of prototype for partition creation: when new partitions are created during an INSERT operation, the file format and other related options are copied out of this special partition. This partition is inconsistently referred to as either the "default partition" or the "dummy partition". The handling of this second case (the partition with id -1) was somewhat messy: - the partition shows up in the partitionMap_ member, but does not show up in the partitionIds_ member. - almost all of the call sites that iterate through the partitions of an HdfsTable instance ended up skipping over the dummy partition. - several call sites called getPartitions().size() but then had to adjust the result by subtracting one in order to actually count the number of partitions in a table. - similarly, test assertions had to assert that tables with 24 partitions had an expected partition map size of 25. In order to address the above, this patch makes the following changes: - getPartitions() and getPartitionMap() no longer include the dummy partition. This removes a bunch of special case checks to skip over the dummy partition or to adjust partition counts based on it. - to clarify the purpose of this partition, references to it are renamed to "prototype partition" instead of "default partition". - when converting the HdfsTable to/from Thrift, the prototype partition is included in its own field in the struct, instead of being stuffed into the same map with the true partitions of the table. This reflects the fact that this partition is special (eg missing fields like 'location' which otherwise are required for real partitions) This change should should be entirely internal with no functional differences. As such, the only testing changes are some fixes for assertions on the Thrift serialized structures and other internals. Change-Id: I15e91b50eb7c2a5e0bac8c33d603d6cd8cbaca2e --- M be/src/exec/hdfs-table-sink.cc M be/src/exec/hdfs-table-sink.h M be/src/runtime/descriptors.cc M be/src/runtime/descriptors.h M common/thrift/CatalogObjects.thrift M common/thrift/ImpalaInternalService.thrift M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/testutil/BlockIdGenerator.java 18 files changed, 163 insertions(+), 205 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/10711/3 -- To view, visit http://gerrit.cloudera.org:8080/10711 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I15e91b50eb7c2a5e0bac8c33d603d6cd8cbaca2e Gerrit-Change-Number: 10711 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-7140 (part 3): load partitions for FS tables
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/10713 )
Change subject: IMPALA-7140 (part 3): load partitions for FS tables
..
Patch Set 2:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java
File fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java:
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java@171
PS2, Line 171: for (String partitionKey: hmsPartitionValues)
> this'll allow for a subset of partitioning columns to be returned for table
OK, I'll add a Preconditions check that the length of this list matches the
number of clustering columns for the table.
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@546
PS2, Line 546: // only to end up throwing away the result.
> yikes... yes, just ran across this today as well.
yea, luckily this only seems to be called from ComputeStatsStmt analysis, and
computing stats is already slow, so the overhead probably isn't disastrous
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@132
PS2, Line 132: Warehouse.makeSpecFromName
> something seems off here-- we're going back to hms for partitioning informa
Good point.
I think the most straight-forward would be to just make this API in
metaprovider take a List partitionColumnNames. It's a little odd
feeling from a clean API perspective, but maybe better than backing the
partition column names out from the first partition name?
I think that's preferable to extracting out SchemaInfo as more than an
implementation detail of the FsTable class hierarchy, since keeping the API
arguments here minimal makes it easier to test, and unit-testing at the level
of MetaProvider will probably make sense once we add more complex
implementations (eg a caching one).
That said, I could be convinced otherwise.
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@143
PS2, Line 143:
> nit: consistent spacing
Done
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@148
PS2, Line 148: name
> how does name correspond to the partition name that's passed in? I assume i
Yea, it should. Do you think it's worth adding an assertion here that
ret.keySet() is a subset of ImmutableSet.of(partitionNames)?
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java:
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java@122
PS2, Line 122: // TODO Auto-generated catch block
> remove
Done
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java@144
PS2, Line 144: // TODO(todd): copy-paste from HdfsPartition
> mark it as expensive (like in the other place)
Done
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java:
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@63
PS2, Line 63: /**
> nit: newline
Done
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@183
PS2, Line 183: List names = Lists.newArrayList();
> is loadPartitionSpecs() missing?
ah, I'll add a Preconditions.checkState here that the partition specs are
loaded. Given that ids are identifiers local to a given FsTable instance, it
wouldn't really make sense to call this without having already retrieved
partition specs to get their IDs.
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@198
PS2, Line 198:
> nit: remove empty line
Done
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@214
PS2, Line 214: if (spec.getName().isEmpty()) continue;
> when does this happen?
ah, I think never :) removed.
http://gerrit.cloudera.org:8080/#/c/10713/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@279
PS2, Line 279: if (getNumClusteringCols() == 0) {
> perhaps this is modeled better as
[Impala-ASF-CR] IMPALA-7161: Fix impala-config.sh's handling of JAVA HOME
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/10702 ) Change subject: IMPALA-7161: Fix impala-config.sh's handling of JAVA_HOME .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10702 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf3521b4f44fdbdc841a90fd00c477c9423a75bb Gerrit-Change-Number: 10702 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Comment-Date: Fri, 15 Jun 2018 17:44:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7161: Fix impala-config.sh's handling of JAVA HOME
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/10702 ) Change subject: IMPALA-7161: Fix impala-config.sh's handling of JAVA_HOME .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/10702/2/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/10702/2/bin/impala-config.sh@230 PS2, Line 230: # bin/impala-config-local.sh to set JAVA_HOME, so it is important to pick up that > We could remove this from bootstrap_development as part of this change. I think I'm going to punt on this. I can think of arguments in both directions. http://gerrit.cloudera.org:8080/#/c/10702/2/bin/impala-config.sh@238 PS2, Line 238: SYSTEM_JAVA_HOME=$(readlink -f /usr/bin/javac | sed "s:bin/javac::") > I think Tianyi is right here in that we should honor the javac that's on th Makes sense, done. I'm keeping the assumption that javac is under $JAVA_HOME/bin/. That should be safe. http://gerrit.cloudera.org:8080/#/c/10702/2/bin/impala-config.sh@238 PS2, Line 238: SYSTEM_JAVA_HOME=$(readlink -f /usr/bin/javac | sed "s:bin/javac::") > How about using "which javac" and "JAVA_HOME=$(which javac|xargs readlink - Switched to use "which javac" http://gerrit.cloudera.org:8080/#/c/10702/2/bin/impala-config.sh@241 PS2, Line 241: # Prefer the JAVA_HOME set in the environment, but use the system's JAVA_HOME otherwise > If you don't mind, could you remove the following line in this change. Done -- To view, visit http://gerrit.cloudera.org:8080/10702 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf3521b4f44fdbdc841a90fd00c477c9423a75bb Gerrit-Change-Number: 10702 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Comment-Date: Fri, 15 Jun 2018 17:42:34 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7161: Fix impala-config.sh's handling of JAVA HOME
Hello Tianyi Wang, Philip Zeyliger, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10702 to look at the new patch set (#3). Change subject: IMPALA-7161: Fix impala-config.sh's handling of JAVA_HOME .. IMPALA-7161: Fix impala-config.sh's handling of JAVA_HOME It is common for developers to specify JAVA_HOME in bin/impala-config-local.sh, so wait until after it is sourced to validate JAVA_HOME. Also, try harder to auto-detect the system's JAVA_HOME in case it has not been specified in the environment. Here is a run through of different scenarios: 1. Not set in environment, not set in impala-config-local.sh: Didn't work before, now tries to autodetect by looking for javac on the PATH 2. Set in environment, not set in impala-config-local.sh: No change 3. Not set in environment, set in impala-config-local.sh: Didn't work before, now works 4. Set in environment and set in impala-config-local.sh: This used to be potentially inconsistent (i.e. JAVA comes from the environment's JAVA_HOME, but JAVA_HOME is overwritten by impala-config-local.sh), now it always uses the value from impala-config-local.sh. Change-Id: Idf3521b4f44fdbdc841a90fd00c477c9423a75bb --- M bin/impala-config.sh M docker/entrypoint.sh 2 files changed, 26 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/10702/3 -- To view, visit http://gerrit.cloudera.org:8080/10702 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idf3521b4f44fdbdc841a90fd00c477c9423a75bb Gerrit-Change-Number: 10702 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang
[Impala-ASF-CR] IMPALA-7140 (part 2). Create skeleton for LocalFsTable
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10712 ) Change subject: IMPALA-7140 (part 2). Create skeleton for LocalFsTable .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java File fe/src/main/java/org/apache/impala/catalog/FeFsTable.java: http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@82 PS2, Line 82: Hdfs > sorry about not catching this before, but pls add a todo (assign it to me) Added a top-level comment on this class with a TODO for you - there are several methods needing rename so figured one TODO was better than one per method. http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java: http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@164 PS2, Line 164: public Map> getFilesSample(Collection inputParts, > nit: too long line Done http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java File fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java: http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@95 PS2, Line 95: "__HIVE_DEFAULT_PARTITION__" > make this a public member so the test can use it as well. Done -- To view, visit http://gerrit.cloudera.org:8080/10712 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8 Gerrit-Change-Number: 10712 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 17:19:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7141 (part 1): clean up handling of default/dummy partition
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10711 ) Change subject: IMPALA-7141 (part 1): clean up handling of default/dummy partition .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/10711/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/10711/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@755 PS2, Line 755: creates the default partition > nit: this comment needs updating Done http://gerrit.cloudera.org:8080/#/c/10711/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@763 PS2, Line 763: setPrototypePartition(msTbl.getSd()); > I'm just wondering if we can call the setPrototypePartition() in the constr I don't think so, because this is also called from HdfsTable.load and a few of the "alter" code paths, and changing that seems like it would bleed into a much more complex patch. -- To view, visit http://gerrit.cloudera.org:8080/10711 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I15e91b50eb7c2a5e0bac8c33d603d6cd8cbaca2e Gerrit-Change-Number: 10711 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 15 Jun 2018 17:11:46 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6812: Fix flaky Kudu scan tests
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/10503 ) Change subject: IMPALA-6812: Fix flaky Kudu scan tests .. Patch Set 4: Code-Review+1 (1 comment) carrying forward http://gerrit.cloudera.org:8080/#/c/10503/3/be/src/exec/kudu-util.h File be/src/exec/kudu-util.h: http://gerrit.cloudera.org:8080/#/c/10503/3/be/src/exec/kudu-util.h@112 PS3, Line 112: i > nit extra space Done -- To view, visit http://gerrit.cloudera.org:8080/10503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I70df84f2cbc663107f2ad029565d3c15bdfbd47c Gerrit-Change-Number: 10503 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 15 Jun 2018 17:08:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6812: Fix flaky Kudu scan tests
Hello David Ribeiro Alves, Todd Lipcon, Tim Armstrong, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10503 to look at the new patch set (#4). Change subject: IMPALA-6812: Fix flaky Kudu scan tests .. IMPALA-6812: Fix flaky Kudu scan tests Many of our Kudu related tests have been flaky with the symptom that scans appear to not return rows that were just inserted. This occurs because our default Kudu scan level of READ_LATEST doesn't make any consistency guarantees. This patch adds a query option 'kudu_read_mode', which overrides the startup flag of the same name, and then set that option to READ_AT_SNAPSHOT for all tests with Kudu inserts and scans, which should give us more consistent test results. Testing: - Passed a full exhaustive run. Does not appear to increase time to run by any significant amount. Change-Id: I70df84f2cbc663107f2ad029565d3c15bdfbd47c --- M be/src/exec/kudu-scanner.cc M be/src/exec/kudu-util.cc M be/src/exec/kudu-util.h M be/src/service/query-options.cc M be/src/service/query-options.h M be/src/util/debug-util.cc M be/src/util/debug-util.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M tests/common/test_dimensions.py M tests/custom_cluster/test_kudu.py M tests/metadata/test_ddl.py M tests/query_test/test_kudu.py 13 files changed, 119 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/10503/4 -- To view, visit http://gerrit.cloudera.org:8080/10503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I70df84f2cbc663107f2ad029565d3c15bdfbd47c Gerrit-Change-Number: 10503 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-2746: part 1: enable LSAN for many backend tests
Hello Philip Zeyliger, Joe McDonnell, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10668 to look at the new patch set (#8). Change subject: IMPALA-2746: part 1: enable LSAN for many backend tests .. IMPALA-2746: part 1: enable LSAN for many backend tests This turns on leak sanitizer for backend tests that required relatively small modifications to pass. We suppress a few leaks, mainly related to the embedded JVM. Testing: Ran core tests under ASAN. Change-Id: Ibdda092a4eb4bc827c75a8c121e5428ec746b7f4 --- M be/CMakeLists.txt M be/src/catalog/CMakeLists.txt M be/src/codegen/CMakeLists.txt M be/src/codegen/instruction-counter-test.cc M be/src/common/CMakeLists.txt M be/src/common/atomic-test.cc M be/src/common/atomic.h M be/src/exec/CMakeLists.txt M be/src/experiments/CMakeLists.txt M be/src/exprs/CMakeLists.txt M be/src/exprs/expr-test.cc M be/src/rpc/CMakeLists.txt M be/src/runtime/CMakeLists.txt M be/src/runtime/bufferpool/CMakeLists.txt M be/src/runtime/bufferpool/buffer-pool-test.cc M be/src/runtime/data-stream-test.cc M be/src/runtime/io/CMakeLists.txt M be/src/runtime/io/disk-io-mgr-stress.cc M be/src/runtime/io/disk-io-mgr-stress.h M be/src/runtime/io/disk-io-mgr-test.cc M be/src/runtime/mem-tracker.cc M be/src/scheduling/CMakeLists.txt M be/src/service/CMakeLists.txt M be/src/statestore/CMakeLists.txt M be/src/udf/udf.cc M be/src/util/CMakeLists.txt M be/src/util/decompress-test.cc A bin/lsan-suppressions.txt 28 files changed, 296 insertions(+), 240 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/10668/8 -- To view, visit http://gerrit.cloudera.org:8080/10668 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibdda092a4eb4bc827c75a8c121e5428ec746b7f4 Gerrit-Change-Number: 10668 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7140 (part 1). Support fetching schema info in LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10630 ) Change subject: IMPALA-7140 (part 1). Support fetching schema info in LocalCatalog .. Patch Set 7: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2685/ -- To view, visit http://gerrit.cloudera.org:8080/10630 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I640f27e36198955e057da62a3ce25a858406e496 Gerrit-Change-Number: 10630 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 16:39:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 1). Support fetching schema info in LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10630 ) Change subject: IMPALA-7140 (part 1). Support fetching schema info in LocalCatalog .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2685/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10630 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I640f27e36198955e057da62a3ce25a858406e496 Gerrit-Change-Number: 10630 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 16:36:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7130: impala-shell -b / --kerberos host fqdn flag overrides value passed in via -i / --impalad
Philip Zeyliger has posted comments on this change. (
http://gerrit.cloudera.org:8080/10580 )
Change subject: IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag
overrides value passed in via -i / --impalad
..
Patch Set 3:
(5 comments)
I think this is close. I could be swayed on the test.
http://gerrit.cloudera.org:8080/#/c/10580/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10580/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag overrides
subjects shouldn't be wrapped, in theory.
http://gerrit.cloudera.org:8080/#/c/10580/3//COMMIT_MSG@10
PS3, Line 10: After additional testing around IMPALA-2782, it was discovered
Let's find a way to add a test for this scenario here.
We can't fully test it because we don't have a kerberos environment, but we can
at least figure out if the right socket is opened.
Use Python or netcat to start listening on two ports, A (the load balancer) and
B (the actual impalad).
With one set of impala-shell options, it should try to connect to A and with
another set it should try to connect to B. We can see if A and B have anything
to read.
Let me know if you think that's unreasonable effort-wise.
http://gerrit.cloudera.org:8080/#/c/10580/3/shell/impala_client.py
File shell/impala_client.py:
http://gerrit.cloudera.org:8080/#/c/10580/3/shell/impala_client.py@66
PS3, Line 66: self.impalad = impalad
It would be reasonable to introduce:
self.impalad_host = impalad[0].encode(...)
self.impalad_port = int(impalad[1])
here
I don't insist.
http://gerrit.cloudera.org:8080/#/c/10580/3/shell/impala_client.py@281
PS3, Line 281: # principal. So in the presence of a load balancer, the its
hostname is expected by
"the its hostname" -> this doesn't parse to me. Can you clear it up?
http://gerrit.cloudera.org:8080/#/c/10580/3/shell/impala_client.py@284
PS3, Line 284: if self.kerberos_host_fqdn is not None:
: host, port =
(self.kerberos_host_fqdn.split(':')[0].encode('ascii', 'ignore'),
: int(self.impalad[1]))
: else:
: host, port = self.impalad[0].encode('ascii', 'ignore'),
int(self.impalad[1])
Please rename this to "sasl_host", since the only use is line 306.
Please move the "port = int(self.impalad[1])" part of this to line 292 and call
it "sock_port". I had to think a little bit too hard about why you didn't need
a port number for the load balancer, and, in part, I think that helped obscure
the bug.
--
To view, visit http://gerrit.cloudera.org:8080/10580
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a
Gerrit-Change-Number: 10580
Gerrit-PatchSet: 3
Gerrit-Owner: Vincent Tran
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: a...@phdata.io
Gerrit-Comment-Date: Fri, 15 Jun 2018 15:55:10 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6969: add AC last queued cause to profile
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10731 Change subject: IMPALA-6969: add AC last queued cause to profile .. IMPALA-6969: add AC last queued cause to profile The reason is updated during initial admission and when the query is at the head of the queue but can't be admitted. It is not updated while the query is in the middle of the queue. Together with the async admission change, this makes it possible to determine from the profile why the query has not been admitted yet. Testing: Added admission control tests that check that the string is set for queries queued based both on the query count and the max memory. Looped the tests overnight to confirm non-flakiness. Change-Id: Ida9b75dc50dfb7a27f59deda91bad6ac838130a1 --- M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M tests/custom_cluster/test_admission_controller.py 3 files changed, 119 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/10731/1 -- To view, visit http://gerrit.cloudera.org:8080/10731 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ida9b75dc50dfb7a27f59deda91bad6ac838130a1 Gerrit-Change-Number: 10731 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2684/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 5 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 15:20:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Adam Holley has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. IMPALA-6802 (part 4): Clean up authorization tests The fourth part of this patch is to rewrite the following authorization tests: - describe Testing: - Added new authorization tests - Ran all front-end tests Cherry-picks: not for 2.x Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 --- M fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java 1 file changed, 279 insertions(+), 57 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/10442/5 -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 5 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7121: Clean up partitionIds from HdfsTable
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/10654 ) Change subject: IMPALA-7121: Clean up partitionIds_ from HdfsTable .. Patch Set 3: -Code-Review Todd, I see you have a +2 on https://gerrit.cloudera.org/#/c/10711/ I'll wait for it to get submitted so that I can make some simplifications on this review. getPartitionIds() would be much simpler, and I'd keep the changes on the callsites. Thanks! -- To view, visit http://gerrit.cloudera.org:8080/10654 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8b5a480e570aeae565fafd4f3e2b279e7a98c7da Gerrit-Change-Number: 10654 Gerrit-PatchSet: 3 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 15 Jun 2018 12:27:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7141 (part 1): clean up handling of default/dummy partition
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/10711 ) Change subject: IMPALA-7141 (part 1): clean up handling of default/dummy partition .. Patch Set 2: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/10711/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/10711/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@755 PS2, Line 755: creates the default partition nit: this comment needs updating http://gerrit.cloudera.org:8080/#/c/10711/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@763 PS2, Line 763: setPrototypePartition(msTbl.getSd()); I'm just wondering if we can call the setPrototypePartition() in the constructor for HdfsTable to have the nwly introduced member initialized from the very beginning. -- To view, visit http://gerrit.cloudera.org:8080/10711 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I15e91b50eb7c2a5e0bac8c33d603d6cd8cbaca2e Gerrit-Change-Number: 10711 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 15 Jun 2018 12:19:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Fail cleanly when in process server can't bind
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/10726 ) Change subject: Fail cleanly when in process server can't bind .. Patch Set 1: Code-Review+1 I thought a bit about not too intrusive ways to reduce the window where a port number is already decided but the port itself is unbound. A static/global port number->socket fd map could be created, and FindUnusedEphemeralPort() could add the fd to this map instead of closing it. Another static function like "ClosePortIfReserved()" could be called as close as possible to the place where the port is bound to socket by the actual server. This function would be a noop if the port is not in the map, so caller would not have to know if the port was "reserved" or not. -- To view, visit http://gerrit.cloudera.org:8080/10726 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I376a2aa559f4b5cf3b96fa3465520e9983ecec4b Gerrit-Change-Number: 10726 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Comment-Date: Fri, 15 Jun 2018 12:11:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6119: Fix issue with multiple partitions sharing same location
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/10543 ) Change subject: IMPALA-6119: Fix issue with multiple partitions sharing same location .. Patch Set 12: Patch set 12 is adjusting comment and also does a rebase. Sorry for having these in one patch. -- To view, visit http://gerrit.cloudera.org:8080/10543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a54bc8224bcefe65b83de2df58bb84629f2aa4a Gerrit-Change-Number: 10543 Gerrit-PatchSet: 12 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Vuk Ercegovac Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 15 Jun 2018 11:38:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6119: Fix issue with multiple partitions sharing same location
Hello Bharath Vissapragada, Zoltan Borok-Nagy, Sailesh Mukil, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10543 to look at the new patch set (#12). Change subject: IMPALA-6119: Fix issue with multiple partitions sharing same location .. IMPALA-6119: Fix issue with multiple partitions sharing same location When multiple partitions point to the same location and a new data file is added to any of them then the expected behaviour is that this new file is added to the other partitions pointing to the same location as well. Apparently, this is not the case and right after the insertion the new file is only visible in the partition where it was inserted to and an invalidate metadata is needed to resolve this inconsistency. This fix addresses this issue with keeping track of a mapping between locations and the HdfsPartitions pointing to it. When new files are inserted into a partition then all the other partition's metadata are reloaded that point to the same location as the one where the files are inserted. It's no longer allowed to drop a partition that shares it's location with other partitions. In this case an error is displayed to the user. Testing: There was an existing test that covered partitions pointing to the same location. However, after each insert it executed a refresh to reload the metadata for the entire table. This reload was removed to cover the changes of this fix. Another test is introduced to cover the case when the location of a partition is altered or a partition is removed. One more test is created to cover when Impala reloads some of it's partitions after Hive had dropped a partition that shares it's location with other partitions. Change-Id: I2a54bc8224bcefe65b83de2df58bb84629f2aa4a --- M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M tests/metadata/test_partition_metadata.py 3 files changed, 177 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/10543/12 -- To view, visit http://gerrit.cloudera.org:8080/10543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2a54bc8224bcefe65b83de2df58bb84629f2aa4a Gerrit-Change-Number: 10543 Gerrit-PatchSet: 12 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Vuk Ercegovac Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 10:03:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2683/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 06:31:24 + Gerrit-HasComments: No
