[Impala-ASF-CR] IMPALA-4091: Fix backend unit to log in logs/be tests.

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change.

Change subject: IMPALA-4091: Fix backend unit to log in logs/be_tests.
..


Patch Set 2:

(9 comments)

Made a first pass. I'm still building the change to validate it myself.

http://gerrit.cloudera.org:8080/#/c/4352/2/be/src/exec/old-hash-table-test.cc
File be/src/exec/old-hash-table-test.cc:

PS2, Line 23: #include "testutil/gtest-util.h"
Still needed?


PS2, Line 24: #include "common/init.h"
: #include "common/compiler-util.h"
nit: preserve alphabetical order (here and in other files)


http://gerrit.cloudera.org:8080/#/c/4352/2/be/src/exec/parquet-plain-test.cc
File be/src/exec/parquet-plain-test.cc:

PS2, Line 22: #include 
nit: not needed with the gtest-util import. We could remove this from all other 
tests, too.


http://gerrit.cloudera.org:8080/#/c/4352/2/be/src/runtime/buffered-tuple-stream-test.cc
File be/src/runtime/buffered-tuple-stream-test.cc:

PS2, Line 1171: BE_TEST
it was not clear to me why this was set to BE_TEST here, while we also 
initialize the FE in the next line. Maybe add an explanation to the comment in 
common/init.h, or even add an overload that removes the parameter, since none 
of the be tests seem to use it.


http://gerrit.cloudera.org:8080/#/c/4352/2/be/src/testutil/gtest-util.h
File be/src/testutil/gtest-util.h:

Line 32: // Basic main() function to be used in gtest unit tests. Does not 
start a JVM
nit: wrap at 90 characters


http://gerrit.cloudera.org:8080/#/c/4352/2/be/src/util/bloom-filter-test.cc
File be/src/util/bloom-filter-test.cc:

PS2, Line 27: #include "common/init.h"
still needed?


http://gerrit.cloudera.org:8080/#/c/4352/2/be/src/util/decompress-test.cc
File be/src/util/decompress-test.cc:

PS2, Line 22: #include "common/init.h"
It is needed? It's seems to be included in gtest-util.h anyways


http://gerrit.cloudera.org:8080/#/c/4352/2/be/src/util/metrics-test.cc
File be/src/util/metrics-test.cc:

PS2, Line 386: 
I don't think this gets executed anymore, since IMPALA_TEST_MAIN() will set the 
respective parameter in InitCommonRuntime to false. Is this change intended? I 
couldn't tell from the code in this file.


PS2, Line 26: #include "util/jni-util.h"
not needed anymore?


-- 
To view, visit http://gerrit.cloudera.org:8080/4352
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaff0acf09bf192d54baeb0bb347e895fce6ed23b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

[Jim Apple (Code Review)]

Jim Apple has submitted this change and it was merged.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
..


IMPALA-4096: Allow clean.sh to work from snapshots

buildall.sh calls bin/clean.sh, which fails when not in a git
directory. This skips the "git clean" steps when not in a git
checkout.

Todd Lipcon found this when testing a snapshot created using git
archive.

Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Reviewed-on: http://gerrit.cloudera.org:8080/4336
Tested-by: Internal Jenkins
Reviewed-by: Jim Apple 
---
M bin/clean.sh
1 file changed, 5 insertions(+), 3 deletions(-)

Approvals:
  Jim Apple: Looks good to me, approved
  Internal Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/4336
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 

[Impala-ASF-CR] IMPALA-3823: Add timer to measure Parquet footer reads

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3823: Add timer to measure Parquet footer reads
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4371/3/be/src/exec/hdfs-parquet-scanner.h
File be/src/exec/hdfs-parquet-scanner.h:

PS3, Line 430: avg_timer_
let's update the variable names too. e.g. something like 
process_footer_timer_stats_ or such


http://gerrit.cloudera.org:8080/#/c/4371/2/be/src/util/runtime-profile-counters.h
File be/src/util/runtime-profile-counters.h:

PS2, Line 240: t TSummaryStatsCount
> Yes that's right. The only reason I kept it as a Counter instead of a value
Thanks

I think it's more useful to remove the restriction. Even in 
hdfs-parquet-scanner where you're using this, there is a Counter constructed 
just for it's timer, it's not included in a profile. That code might as well 
have just used a timing function explicitly. (I'm not saying it's worth 
changing that code, just that there's no dependency on this taking Counters.)

Also, I don't think we get much from checking the Counter's unit. We don't 
really set counter units carefully in many cases (e.g. we use UNIT all over the 
place instead of anything useful), and ultimately it's up to the user of this 
interface to make sure what they're providing here makes sense.


http://gerrit.cloudera.org:8080/#/c/4371/3/be/src/util/runtime-profile-counters.h
File be/src/util/runtime-profile-counters.h:

PS3, Line 240: Overwrite
"SetStats()" to be more consistent with some of the other fns, e.g. 
SetSamples(). If I saw Overwrite() I'd wonder why there wasn't just a copy 
constructor or something like that.


http://gerrit.cloudera.org:8080/#/c/4371/2/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:

Line 661:   }
> Done
thanks!


http://gerrit.cloudera.org:8080/#/c/4371/3/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:

PS3, Line 668: - 
I don't see this dash in the example above? Is it intentional? I see the event 
markers under an event sequence make a list, though those are items of a 
specific timeline. I don't see why that'd be the case here.

Also value() is an average, right? I think it'd be good to have that specified.


It'd be nice to include the number of samples as well. Also, avg isn't really 
all that different from the other stats, so maybe just print them all the same 
way, e.g.:

" :  avg: , min: , max: , num_samples: "


-- 
To view, visit http://gerrit.cloudera.org:8080/4371
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf87bad90037dd0cea63b10c537382ec0f980cbf
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4026: Implement double-buffering for BlockingQueue.

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change.

Change subject: IMPALA-4026: Implement double-buffering for BlockingQueue.
..


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/4350/3/be/src/util/blocking-queue.h
File be/src/util/blocking-queue.h:

PS3, Line 188: put_list
> shouldn't these members have _ at the end?
I just converted this to a struct :-).


Line 197:   boost::mutex get_lock;
> Shouldn't we align get_lock?
It's implicitly aligned after I added the attribute to ConditionVariable as the 
compiler seems to pad that struct to 64-byte. I will add to DCHECK to verify 
that's the case.


http://gerrit.cloudera.org:8080/#/c/4350/3/be/src/util/condition-variable.h
File be/src/util/condition-variable.h:

Line 28: /// Simple wrapper around POSIX pthread condition variable.
> Explain briefly how it differs from the boost cv?
Sure.


PS3, Line 29: __attribute__ ((aligned(64)))
> What do you think about putting this in compiler-util.h as CACHE_ALIGN or s
Good idea.


PS3, Line 29: struct
> class?
Can you actually put __attribute__ ((aligned(64))) on class in C++ ?


PS3, Line 61: cv
> cv_
I think struct requires no "_" ?!


-- 
To view, visit http://gerrit.cloudera.org:8080/4350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Chen Huang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4091: Fix backend unit to log in logs/be tests.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-4091: Fix backend unit to log in logs/be_tests.
..


Patch Set 1:

I decided to add a macro for the common main() instead of going the library 
route. The library route seemed a little complicated because we'd need to muck 
with how we add tests in our CMakeLists.txt (add dependencies or add a new 
ADD_STANDARD_TEST macro). Seemed easier this way.

-- 
To view, visit http://gerrit.cloudera.org:8080/4352
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaff0acf09bf192d54baeb0bb347e895fce6ed23b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4026: Implement double-buffering for BlockingQueue.

[Michael Ho (Code Review)]

Michael Ho has uploaded a new patch set (#3).

Change subject: IMPALA-4026: Implement double-buffering for BlockingQueue.
..

IMPALA-4026: Implement double-buffering for BlockingQueue.

With recent changes to improve the parquet scanner's efficency,
row batches are produced more quickly, leading to higher contention
in the blocking queue shared between scanner threads and the scan
node. The contention happens between different producers (i.e.
the scanner threads) and also to a lesser extent, between the
scanner threads and the scan node.

This change addresses the contention between the scanner threads
and the scan node by splitting the queue into a 'read_list_' and
a 'write_list_'. The consumers will consume from 'read_list_' until
it's exhausted while the producers will enqueue into 'write_list_'
until it's full. When 'read_list_' is exhausted, the consumer will
atomically swap the 'read_list_' with 'write_list_'. This reduces
the contention/overhead in two ways: (1) 'read_list_' and 'write_list_'
are protected by two different locks so consumer only contends for the
write lock when 'read_list_' is empty. (2) the consumer only signals
the producer after an entire 'read_list_' is consumed instead of
signalling it per entry in the 'read_list_'.

With this change, the regression in primitive_filter_bigint_non_selective
went from 1.6s to 0.8s, improving by 50%.

Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54
---
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M be/src/util/blocking-queue.h
A be/src/util/condition-variable.h
4 files changed, 196 insertions(+), 70 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/4350/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Chen Huang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-4099: Fix the error message while loading UDFs with no JARs

[Marcel Kornacker (Code Review)]

Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-4099: Fix the error message while loading UDFs with no 
JARs
..


Patch Set 2: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4365
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I167426ea96b0a41374227ef238b6f60e4184a9d7
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3902: Scheduler improvements for running multiple fragment instances on a single backend

[Marcel Kornacker (Code Review)]

Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-3902: Scheduler improvements for running multiple 
fragment instances on a single backend
..


Patch Set 4:

(27 comments)

http://gerrit.cloudera.org:8080/#/c/4054/4/be/src/runtime/coordinator.h
File be/src/runtime/coordinator.h:

PS4, Line 218:   const TNetworkAddress local_addr_;
> Can you use ExecEnv::backend_address()? Or find some way to avoid the dupli
Done


PS4, Line 297: Wait
> nit: Wait()
Done


PS4, Line 388: fragment_instance_state_idxs
> Is this referencing the "fragment id (TPlanFragment.id)"? If so, please say
it's not, it's an index into fragment_instance_states_. i thought that would be 
clear from the name.

added comment.


http://gerrit.cloudera.org:8080/#/c/4054/4/be/src/scheduling/query-schedule.cc
File be/src/scheduling/query-schedule.cc:

PS4, Line 121:   std::sort(fragments.begin(), fragments.end(),
 :   [](const TPlanFragment* a, const TPlanFragment* b) { 
return a->id < b->id; });
> shouldn't these come in an expected order?
this adds an extra level of robustness, otherwise i have to make this ordering 
guarantee part of the interface.

i don't expect this to have any performance impact.


PS4, Line 346: NULL
> nullptr
Done


http://gerrit.cloudera.org:8080/#/c/4054/4/be/src/scheduling/query-schedule.h
File be/src/scheduling/query-schedule.h:

Line 97:   std::map per_exch_num_senders;
> any benefit from using map over unordered_map here?
it makes it easier to assign to thrift structs, and this isn't 
performance-critical.


PS4, Line 173: NULL
> nullptr
Done


Line 174:   const TPlanFragment* GetCoordFragment() const; 
> nit: trailing whitespace
Done


Line 213:   MtFragmentExecParams* GetFragmentExecParams(FragmentId id) {
> If you need non-const access, isn't it more idiomatic to return a non-const
we typically return pointers for mutable structures (and const refs for 
immutable structures).


PS4, Line 219: NULL
> nullptr
Done


Line 307: 
> comment, e.g. what state in MtFragmentExecParams gets initialized here vs. 
Done


http://gerrit.cloudera.org:8080/#/c/4054/4/be/src/scheduling/simple-scheduler.cc
File be/src/scheduling/simple-scheduler.cc:

PS4, Line 384: auto
> auto only for iterators
this is a map iterator


PS4, Line 414: depth-first traversal
> isn't this an implementation detail of the other MtComputeFragmentExecParam
Done


PS4, Line 425: f
> can you change f indexes here to match the code? it's explained as if it st
moved comment


PS4, Line 433:   const TDataStreamSink& sink = 
src_fragment.output_sink.stream_sink;
 :   DCHECK(
 :   sink.output_partition.type == 
TPartitionType::UNPARTITIONED
 : || sink.output_partition.type == 
TPartitionType::HASH_PARTITIONED
 : || sink.output_partition.type == 
TPartitionType::RANDOM);
> move this closer to where it's used on l453
Done


PS4, Line 470: MtComputeFragmentExecParams
> Can we be sure that recursion depth will not be a problem here?
yes


Line 524: // try to load-balance scan ranges by assigning just beyond the
> nit: wrap at 90 characters
Done


PS4, Line 541:   while (params_idx < params_list.size()
 :   && total_assigned_bytes < threshold_total_bytes) {
> It's not obvious to me why this will always assign all params. If the last 
i'm not sure what to add.

for the last fragment, threshold_total_bytes == total_size, so this won't get 
exceeded. if total_assigned_bytes == total_size, by definition there can't be 
any params left.

let me know how i can clarify.


Line 695: for (int j = 0; j < params.hosts.size(); ++j, 
++num_fragment_instances) {
> you could call reserve() before this line
i'll leave the old code as-is.


PS4, Line 933: if (!FLAGS_disable_admission_control) {
 :   
RETURN_IF_ERROR(admission_controller_->AdmitQuery(schedule));
 : }
 : if (!FLAGS_enable_rm) return Status::OK();
 : 
 : string user = 
GetEffectiveUser(schedule->request().query_ctx.session);
 : if (user.empty()) user = "default";
 : schedule->PrepareReservationRequest(resolved_pool, user);
 : const TResourceBrokerReservationRequest& reservation_request 
=
 : schedule->reservation_request();
 : if (!reservation_request.resources.empty()) {
 :   Status status = resource_broker_->Reserve(
 :   reservation_request, schedule->reservation());
 :   if (!status.ok()) {
 : // Warn about missing table and/or column stats if 
necessary.
 : const TQueryCtx& query_ctx = 
schedule->request().query_ctx;
 : if(!query_ctx.__isset.parent_query_id &&
 : 

[Impala-ASF-CR] IMPALA-3902: Scheduler improvements for running multiple fragment instances on a single backend

[Marcel Kornacker (Code Review)]

Marcel Kornacker has uploaded a new patch set (#5).

Change subject: IMPALA-3902: Scheduler improvements for running multiple 
fragment instances on a single backend
..

IMPALA-3902: Scheduler improvements for running multiple fragment instances on
a single backend

This is an extension of the scheduler and coordinator for multi-threaded
execution. It mainly removes the assumption of having one instance per
fragment per host. The approach taken here is to create parallel data
structures and control flow functions, where necessary, and otherwise to leave
the existing single-instance logic in place. The parallel structures' and
functions' names are prefixed with "Mt" to facilitate the enventual clean-up.
Not much of an attempt was made to factor out common functionality between
the Mt- and the single-threaded version, because the single-threaded version
will eventually disappear (once multi-threaded execution is the default)
and refactoring the existing code to fit into two parallel functions from
which it's being called might end up obscuring the code more than helping it.
Also, this code is relatively stable and having two parallel paths won't
cause much extra work (in terms of having to apply the same changes/fixes
twice) in the medium term.

Changes to data structures:
- QuerySchedule: per-instance and per-fragment structs with complete
  execution parameters (instead of partially relying on TQueryExecRequest);
  the per-instance execution parameter struct is a child of the per-fragment
  parameter struct
- explicit fragment id, with range 0..#fragments-1 (instead of relying on an
  index into an array in TQueryExecRequest)

Excluded:
- runtime filter handling
- anything related to RM

Change-Id: I240445199e22f009f4e72fdb8754eb8d77e3d680
---
M be/CMakeLists.txt
M be/src/common/global-types.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/scheduling/query-resource-mgr.cc
M be/src/scheduling/query-resource-mgr.h
M be/src/scheduling/query-schedule.cc
M be/src/scheduling/query-schedule.h
M be/src/scheduling/simple-scheduler.cc
M be/src/scheduling/simple-scheduler.h
M be/src/service/fragment-exec-state.cc
M be/src/service/fragment-mgr.cc
M be/src/service/impala-server.cc
M be/src/service/query-exec-state.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/util/CMakeLists.txt
M be/src/util/uid-util-test.cc
M be/src/util/uid-util.h
M common/thrift/ExecStats.thrift
M common/thrift/Frontend.thrift
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M common/thrift/Planner.thrift
M common/thrift/Types.thrift
M fe/src/main/java/com/cloudera/impala/common/TreeNode.java
M fe/src/main/java/com/cloudera/impala/planner/Planner.java
M fe/src/main/java/com/cloudera/impala/service/Frontend.java
M fe/src/test/java/com/cloudera/impala/planner/PlannerTestBase.java
29 files changed, 1,490 insertions(+), 546 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/4054/5
-- 
To view, visit http://gerrit.cloudera.org:8080/4054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I240445199e22f009f4e72fdb8754eb8d77e3d680
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Marcel Kornacker 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 

[Impala-ASF-CR] IMPALA-3918: Fix straggler Cloudera -> ASF license headers

[Jim Apple (Code Review)]

Jim Apple has posted comments on this change.

Change subject: IMPALA-3918: Fix straggler Cloudera -> ASF license headers
..


Patch Set 2: Code-Review+2

> lgtm, though it might be a good idea to document the reasoning that
 > the "relicense" is OK in the commit message so that if anyone is
 > auditing the provenance of these files through git history later,
 > you have some trail of why you decided to replace the license
 > header.

Done, carry Alex's +2.

-- 
To view, visit http://gerrit.cloudera.org:8080/4386
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief3e09ac93137f870b7fb31a0bc851f24a17573b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3918: Fix straggler Cloudera -> ASF license headers

[Jim Apple (Code Review)]

Hello Alex Behm,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4386

to look at the new patch set (#2).

Change subject: IMPALA-3918: Fix straggler Cloudera -> ASF license headers
..

IMPALA-3918: Fix straggler Cloudera -> ASF license headers

beeswax.thrift was originally copied from Hive, which is also an ASF
project.

Change-Id: Ief3e09ac93137f870b7fb31a0bc851f24a17573b
---
M common/thrift/beeswax.thrift
M fe/src/test/java/com/cloudera/impala/common/FrontendTestBase.java
2 files changed, 32 insertions(+), 30 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/4386/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4386
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ief3e09ac93137f870b7fb31a0bc851f24a17573b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Todd Lipcon 

[Impala-ASF-CR] IMPALA-3918: Fix straggler Cloudera -> ASF license headers

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: IMPALA-3918: Fix straggler Cloudera -> ASF license headers
..


Patch Set 1:

lgtm, though it might be a good idea to document the reasoning that the 
"relicense" is OK in the commit message so that if anyone is auditing the 
provenance of these files through git history later, you have some trail of why 
you decided to replace the license header.

-- 
To view, visit http://gerrit.cloudera.org:8080/4386
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief3e09ac93137f870b7fb31a0bc851f24a17573b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4099: Fix the error message while loading UDFs with no JARs

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-4099: Fix the error message while loading UDFs with no 
JARs
..


Patch Set 2: Code-Review+1

Henry, Marcel, can one of you sign off?

-- 
To view, visit http://gerrit.cloudera.org:8080/4365
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I167426ea96b0a41374227ef238b6f60e4184a9d7
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4099: Fix the error message while loading UDFs with no JARs

[Bharath Vissapragada (Code Review)]

Bharath Vissapragada has uploaded a new patch set (#2).

Change subject: IMPALA-4099: Fix the error message while loading UDFs with no 
JARs
..

IMPALA-4099: Fix the error message while loading UDFs with no JARs

Currently we throw a generic exception due to a missing check in
the code for cases where the UDF has no resources associated with
it. This is not super useful from supportability point of view.
This commit fixes that. This also cleans up the code a little
by moving these checks out of the extractFunctions() method.

Change-Id: I167426ea96b0a41374227ef238b6f60e4184a9d7
---
M fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java
1 file changed, 45 insertions(+), 25 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/4365/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4365
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I167426ea96b0a41374227ef238b6f60e4184a9d7
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 

[Impala-ASF-CR] IMPALA-4099: Fix the error message while loading UDFs with no JARs

[Bharath Vissapragada (Code Review)]

Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-4099: Fix the error message while loading UDFs with no 
JARs
..


Patch Set 1:

(5 comments)

Thanks Alex for the review.

http://gerrit.cloudera.org:8080/#/c/4365/1/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java:

Line 398:* 2. Has only one binary resource associated (We don't support 
loading
> Has exactly one binary ...
Done


Line 402:* Returns true if compatible and false otherwise. Incase of 
incompatible
> In case
Done


Line 403:* functions 'incompatMsg' has the reason for incompatibility.
> the incompatibility
Done


Line 415:   incompatMsg.append("No executable binary resource (like a JAR 
file) is " +
> Do we know how the user can fix this? If so, we should add that in the err 
Yes recreating the function should fix this. Impala doesn't allow creating 
functions without a location clause but looks like it works with Hive, may be 
it throws a runtime error if anyone tries to access it.


Line 416:   "associated with this function");
> "." at the end of err msg
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/4365
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I167426ea96b0a41374227ef238b6f60e4184a9d7
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3918: Fix straggler Cloudera -> ASF license headers

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-3918: Fix straggler Cloudera -> ASF license headers
..


Patch Set 1: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4386
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief3e09ac93137f870b7fb31a0bc851f24a17573b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4008: don't bake in hash table and hash join pointers

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4008: don't bake in hash table and hash join pointers
..


Patch Set 3:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/4326/3/be/src/exec/hash-table.h
File be/src/exec/hash-table.h:

PS3, Line 300: ExprValuesHash
CurExprValuesHash()  [see below for motiviation]


PS3, Line 303: SetExprValuesHash
Let's rename this to more closely match cur_expr_values() rather than matching 
ExprValuePtr (which used to make sense, but now no longer does given that 
ExprValuePtr doesn't necessarily operate on "current").

SetCurExprValuesHash()?


PS3, Line 309: cur_expr_values_null
comment explaining that there is one byte per null value (i.e. these are used 
as bools, but typed as uint8_t for codegen).  (to support the arithmetic above).


PS3, Line 403: in
the first time I read this, I read it as this functions stores the values that 
are in 'expr_values'.  Let's clarify by saying "to" or "into".


PS3, Line 403: in
same


PS3, Line 413: in
same


PS3, Line 431: This will be replaced by
 :   /// codegen.
nit: move this sentence to end of the comment so it's standardized with similar 
comments and not buried.


PS3, Line 438: with 'cur_expr_values_'
"to compare against the current row."  (since it also uses cur_expr_values_null 
and to give a little more info than what the C code tells us).


-- 
To view, visit http://gerrit.cloudera.org:8080/4326
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie353666dbb5c958f0094d169306fe930ec3014c5
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] Fix typo in buildall.sh introduced in IMPALA-4006

[Internal Jenkins (Code Review)]

Internal Jenkins has posted comments on this change.

Change subject: Fix typo in buildall.sh introduced in IMPALA-4006
..


Patch Set 1: Verified+1

-- 
To view, visit http://gerrit.cloudera.org:8080/4382
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7a0f74b93bb31bd0c56fc4c20f42f8ab1fc6de78
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4091: Fix backend unit to log in logs/be tests.

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change.

Change subject: IMPALA-4091: Fix backend unit to log in logs/be_tests.
..


Patch Set 1:

Thanks for fixing this. While we are at it, why don't we create a common 
library for the tests containing a default main() function and then just link 
all tests against it? This way we could get away with less code and wouldn't 
need to maintain main() functions in all tests.

-- 
To view, visit http://gerrit.cloudera.org:8080/4352
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaff0acf09bf192d54baeb0bb347e895fce6ed23b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4110: PREVIEW: Make RAT run on Impala tarballs.

[Jim Apple (Code Review)]

Jim Apple has posted comments on this change.

Change subject: IMPALA-4110: PREVIEW: Make RAT run on Impala tarballs.
..


Patch Set 1:

(3 comments)

> (5 comments)
 > 
 > looks pretty reasonable. Might want to do a surface level
 > understanding of why you need this ratcursive workaround and file a
 > JIRA upstream.

Looks like it was some 0.11 problems; 0.12 worked without ratcursive.

http://gerrit.cloudera.org:8080/#/c/4361/1/bin/rat.sh
File bin/rat.sh:

PS1, Line 38: RAT_JAR=$1
: # The location of the Impala directory (or subdirectory within 
the Impala tree) to check:
: START=$2
> rather than relying just on set -u, i think it's nicer to print a real 'usa
File removed.


PS1, Line 51:  PN
> what's PN?
PseudoNym


Line 63: echo "Failed to RAT ${FN}" >&2
> should set the exit code in this case, no? otherwise people might miss this
I thinkk RAT exits with 0 even when the license isn't OK. I've changed the 
checker python script I borrowed from Kudu to be a little stricter in what goes 
to stderr.


-- 
To view, visit http://gerrit.cloudera.org:8080/4361
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5bfe77f9a871018e7a67553ed270e2df53006962
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4111: backend death tests should not produce minidumps
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4353/2/be/src/testutil/death-test-util.h
File be/src/testutil/death-test-util.h:

PS2, Line 41: setrlimit(RLIMIT_CORE, );
: 
: minidumps_enabled_before_ = EnableMinidumpsForTest(false);
> Core dumps and minidumps / breakpad should be orthogonal, so enabling/disab
Thanks. Also I had chatted with Tim in person. I was trying to understand the 
relationship between enabling/disabling these two. In the past I had seen cases 
where I was getting minidumps but not cores, and thought we had disabled them. 
Anyway, this makes sense, thanks.


-- 
To view, visit http://gerrit.cloudera.org:8080/4353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change.

Change subject: IMPALA-4111: backend death tests should not produce minidumps
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4353/2/be/src/testutil/death-test-util.h
File be/src/testutil/death-test-util.h:

PS2, Line 41: setrlimit(RLIMIT_CORE, );
: 
: minidumps_enabled_before_ = EnableMinidumpsForTest(false);
> Ok, then a different question: why do we need to disable coredumps? Shouldn
Core dumps and minidumps / breakpad should be orthogonal, so enabling/disabling 
one won't interfere with the other. Policy-wise I don't think we made any 
effort to disable core-dumps in dev environments. I would assume that's mostly 
because they contain more information and these systems are well connected, so 
collecting and transferring them isn't that much of a concern.


-- 
To view, visit http://gerrit.cloudera.org:8080/4353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4014: PARALLEL HEADERS ONLY: Introduce query-wide execution state.

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-4014: PARALLEL HEADERS ONLY: Introduce query-wide 
execution state.
..


Patch Set 4:

(27 comments)

Thanks for the review Lars. I've made the changes.

http://gerrit.cloudera.org:8080/#/c/4301/1/be/src/runtime/fragment-instance-state.h
File be/src/runtime/fragment-instance-state.h:

PS1, Line 68:   /// on whether the fragment instance has a sink) indicated that 
execution is finished.
:   ~FInstanceState() { }
> This feels still unclear to me, especially the meaning of "it is an error".
It will be undefined behavior because if execution is not complete, the 
FInstanceState will be expected to be present in the map.


Line 106:   /// is > 0 and a callback was specified in the c'tor.
> If Open sees it, it should just do nothing, right?
Yes, Open() does nothing special if Cancel() happens.


http://gerrit.cloudera.org:8080/#/c/4301/2/be/src/runtime/fragment-instance-state.h
File be/src/runtime/fragment-instance-state.h:

PS2, Line 44: This
> Does "This" mean, that the profile is created during the teardown? How woul
Yes, it means this class. Done.


Line 74:   const TNetworkAddress& coord_address() const { return 
query_ctx_.coord_address; }
> single line?
Done


PS2, Line 109: us Open();
> Does the caller still need to call GetNext() in this case to make the destr
That sounds like a good idea. I unfortunately don't know all the possibilities 
well enough to draw the diagram. I will sit down with someone who does and add 
this state diagram.


PS2, Line 119: 
> What is this? I couldn't find it in the member variables. Should it say "Cl
That comment was copied over from the old PFE. Changed it.

This is called in the destructor, so it doesn't need to be explicitly called 
after Cancel().

It doesn't seem legal to call before Open()/GetNext() but I'm not sure. Will 
confirm and update.


PS2, Line 123: 
 :   /// Cancel() may be called more than once. Calls after the 
first will hav
> Another bit of the state machine that we seem to implement. Still not sure 
I don't understand what you mean by "collect them centrally". Could you clarify?


Line 160: 
> Is this only ever set there?
No, it was previously. But now it will be set in multiple places. So I've 
removed the comment.


PS2, Line 213: k> sink_;
> "sent by this"? Or is this really the incoming sink? Maybe rename to input_
I've rephrased this. "sent to/by this" leaves room for ambiguity.


PS2, Line 285:  sink, the sink will
> There's no status_ field. Maybe remove the comment altogether?
It should be exec_status_. Since we've merged both the classes, I've gotten rid 
of status_ and kept only exec_status_.


Line 300: 
> nit: remove blank line
Done


http://gerrit.cloudera.org:8080/#/c/4301/1/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:

Line 112:   }
> Maybe add a comment like "protected by xyz_lock_" and move it under that lo
Done


http://gerrit.cloudera.org:8080/#/c/4301/2/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:

Line 65:   ///   1. If the QueryState exists, it will not be destroyed in the 
scope of this class
> nit: line wrapping
Done


Line 84: 
> nit: newline above private: (not sure)?
Done


PS2, Line 100: cels a plan fragme
> CancelFinstance?
Done


PS2, Line 103: to a local frag
> dst_finstance_id?
Done


Line 142:   FInstanceStateMap finstance_exec_state_map_;
> Maybe say "non-owning pointer"?
This pointer is owned by QueryState however.


http://gerrit.cloudera.org:8080/#/c/4301/2/be/src/service/query-exec-mgr.h
File be/src/service/query-exec-mgr.h:

PS2, Line 21: unord
> std/unordered_map?
Done


Line 73:   /// Forwards a global runtime filter to the corresponding QueryState 
to publish to
> Do these need to be public at all? Can we make them private and just use Qu
This doesn't need to be public now. Made both this and ReleaseQS() private.


PS2, Line 84: eryState 
> fragment instances? Or really to all instances of a fragment? Maybe add  (s
Since we have the instance_id as the parameter, this should be fragment 
instance. Done.


PS2, Line 85: 
> then this might need to become dst_finstance_id.
Done


PS2, Line 96: s on 
> Can we use std::unordered_map?
Done


Line 105:   /// Caller must call ReleaseQueryExecState() once done with this.
> Will the caller have to call ReleaseQueryState on it?
Yes. Done.


Line 107: 
> nit: "...once a query_state..."
Done


PS2, Line 112: 
> finstance_state doesn't seem to have a 'state' field. Should this be finsta
It should become Open(). Changed it.


PS2, Line 117: once we 
> typo
Done


PS2, Line 116:   /// reserved during fragment instance registration time.
 :   /// TODO: Move this responsibility outside this class once we 
have per-query RPC. This
> Isn't this also called here because we want the QEM to start the threads? I
Yes, but we want the QEM to start the threads because we 

[Impala-ASF-CR] IMPALA-4014: PARALLEL HEADERS ONLY: Introduce query-wide execution state.

[Sailesh Mukil (Code Review)]

Sailesh Mukil has uploaded a new patch set (#3).

Change subject: IMPALA-4014: PARALLEL HEADERS ONLY: Introduce query-wide 
execution state.
..

IMPALA-4014: PARALLEL HEADERS ONLY: Introduce query-wide execution state.

This patch is a header preview of a query wide execution state for
a backend.

None of the existing headers are replaced, they are just added in
parallel to existing headers and not considered for compilation.

Changes in design are as follows:

FragmentExecState -> FInstanceState
  This is now what was previously the FragmentExecState and the
  PlanFragmentExecutor together. The ReportStatusCallback is removed
  and now SendReport() is in charge of sending the report directly.
  The callback was in place originally only so the PlanFragmentExecutor
  could call into the FragmentExecState without a reference. This is
  not necessary anymore.

PlanFragmentExecutor (collapsed into FInstanceState)

FragmentMgr -> QueryExecMgr
  The QueryExecMgr now receives incoming fragments, creates a new
  QueryState if it is the first fragment to arrive for that
  query, and passes this received fragment along to the QueryState.
  This class is responsible for cleaning up the QueryState.

QueryState (new):
  This is the query wide state for the backend. It is initialized by
  the first fragment to arrive to the QueryExecMgr. This class is now
  responsible for creating FInstanceStates and executing them.
  Its life is protected by a ref counting mechanism and it is
  scheduled for destruction once the ref count reaches zero.
  Once scheduled for destruction, a thread in the QueryExecMgr will
  destroy the QueryState.
  Every user of the QueryState must access it within the scope of a
  'Guard' object which guarantees ref count incrementing and
  decrementing at entry/exit of the scope.

Excludes:
 - All .cc files to make use of changes made.

Change-Id: If58292a5c377660d97e7e7cc0c3122328eba72ed
---
A be/src/runtime/fragment-instance-state.h
A be/src/runtime/query-state.h
M be/src/runtime/runtime-state.h
A be/src/service/query-exec-mgr.h
4 files changed, 585 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/4301/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If58292a5c377660d97e7e7cc0c3122328eba72ed
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps

[Tim Armstrong (Code Review)]

Hello Lars Volker,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4353

to look at the new patch set (#4).

Change subject: IMPALA-4111: backend death tests should not produce minidumps
..

IMPALA-4111: backend death tests should not produce minidumps

Move the existing core dump disabler into a shared utility header and
also disable minidumps too. Add a macro to simplify use of the disabler.

Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404
---
A be/src/testutil/death-test-util.h
M be/src/util/minidump.cc
M be/src/util/minidump.h
M be/src/util/promise-test.cc
4 files changed, 83 insertions(+), 24 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 

[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4111: backend death tests should not produce minidumps
..


Patch Set 4: Code-Review+1

Carry +1

-- 
To view, visit http://gerrit.cloudera.org:8080/4353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1616: Improve the Memory Limit Exceeded error report

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change.

Change subject: IMPALA-1616: Improve the Memory Limit Exceeded error report
..


Patch Set 4: Code-Review+1

-- 
To view, visit http://gerrit.cloudera.org:8080/4335
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb4e0c359d889938b4c351771ba539850bdb95ea
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1616: Improve the Memory Limit Exceeded error report

[Attila Jeges (Code Review)]

Attila Jeges has uploaded a new patch set (#4).

Change subject: IMPALA-1616: Improve the Memory Limit Exceeded error report
..

IMPALA-1616: Improve the Memory Limit Exceeded error report

The error report has been changed to include the id of the fragment
instance that exceeded the memory limit.

Change-Id: Ibb4e0c359d889938b4c351771ba539850bdb95ea
---
M be/src/runtime/runtime-state.cc
1 file changed, 1 insertion(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibb4e0c359d889938b4c351771ba539850bdb95ea
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-1616: Improve the Memory Limit Exceeded error report

[Attila Jeges (Code Review)]

Hello Michael Ho, Tim Armstrong,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4335

to look at the new patch set (#4).

Change subject: IMPALA-1616: Improve the Memory Limit Exceeded error report
..

IMPALA-1616: Improve the Memory Limit Exceeded error report

The error report has been changed to include the id of the fragment
instance that exceeded the memory limit.

Change-Id: Ibb4e0c359d889938b4c351771ba539850bdb95ea
---
M be/src/runtime/runtime-state.cc
1 file changed, 1 insertion(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibb4e0c359d889938b4c351771ba539850bdb95ea
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong