date:20170817

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3208: max_row_size option
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7629/10/fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java
File fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java:

Line 25: public class ResourceProfileBuilder {
> why is having this class better than just having a ResourceProfile construc
Yeah, plus it makes it easier to omit arguments that aren't needed. It was 
getting confusing with 5 different long arguments, particularly when most 
callsites don't care about most of them.

It's sort-of a workaround for the lack of keyword arguments in Java.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic70f6dddbcef124bb4b329ffa2e42a74a1826570
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3208: max row size option

Dan Hecht has posted comments on this change.

Change subject: IMPALA-3208: max_row_size option
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7629/10/fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java
File fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java:

Line 25: public class ResourceProfileBuilder {
why is having this class better than just having a ResourceProfile constructor 
that takes the four arguments? is the idea so you see the parameter names at 
the callsite, or something more?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic70f6dddbcef124bb4b329ffa2e42a74a1826570
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4863/IMPALA-5311: Correctly account the file type and compression codec

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4863/IMPALA-5311: Correctly account the file type and 
compression codec
..


Patch Set 7: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1094/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5809: Relax max minidumps in breakpad test

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5809: Relax max_minidumps in breakpad test
..


IMPALA-5809: Relax max_minidumps in breakpad test

The change to address IMPALA-5769 added periodic cleaning for minidumps,
which got in the way of the other minidump tests.

This change sets max_minidumps to the default value (9) for all tests to
keep the cleanup thread from interfering, and then sets a smaller limit
where needed.

Change-Id: I977930ae87b8d4671a89c1e07ba76b12eb92fa55
Reviewed-on: http://gerrit.cloudera.org:8080/7716
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
M tests/custom_cluster/test_breakpad.py
1 file changed, 9 insertions(+), 10 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Tim Armstrong: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I977930ae87b8d4671a89c1e07ba76b12eb92fa55
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5809: Relax max minidumps in breakpad test

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5809: Relax max_minidumps in breakpad test
..


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I977930ae87b8d4671a89c1e07ba76b12eb92fa55
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] MPALA-5776: Write partial tuple to the correct mempool

Alex Behm has posted comments on this change.

Change subject: MPALA-5776: Write partial tuple to the correct mempool
..


Patch Set 5: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7639/5/be/src/exec/hdfs-text-scanner.cc
File be/src/exec/hdfs-text-scanner.cc:

Line 297: boundary_column_.Clear();
> I tried it and it doesn't quite work (without clearing the boundary field)
Ahh you are right, I missed that one memcpy(). Thanks for investigating, I 
agree the current solution is least confusing.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I60ba5c113aefd17f697c1888fd46a237ef396540
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4833: Compute precise per-host reservation size, pt2
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5644,IMPALA-5810: Min reservation improvements

Dan Hecht has posted comments on this change.

Change subject: IMPALA-5644,IMPALA-5810: Min reservation improvements
..


Patch Set 5: Code-Review+2

(4 comments)

Code looks fine but I think those (pre-existing) comments could use some 
fixing/clarifying. I'll be on PTO tomorrow, so feel free to work with Tim to 
get that finished up (or I can look on Monday).

http://gerrit.cloudera.org:8080/#/c/7678/5/be/src/runtime/bufferpool/reservation-util.h
File be/src/runtime/bufferpool/reservation-util.h:

PS5, Line 32:  allocated to buffer reservations,
After thinking about these variables more, this paragraph doesn't seem right. 
does this amount actually get set aside to be used only be the buffer pool 
(i.e. allocated to buffer reservations), or does it just set the upper bound on 
the amount of buffer pool memory that the query can use?


PS5, Line 36: or
:   /// scans, exprs, row batches, etc.
I don't follow that. we don't use buffer reservations for those yet.

Oh, do you mean the amount of memory (for scans, exprs, row batches, etc) that 
should not be usable by the buffer pool?

This comment applies to both of these variables though, right? So maybe we 
should have a quick high level comment explaining that memory is divided into 
two classes and what they are and examples of how they're used, and then the 
variable specific comments can just explain how they put guard rails on each so 
neither is starved.


http://gerrit.cloudera.org:8080/#/c/7678/4/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:

Line 125: "more information.";
> Yeah this overall seemed simplest since at this point we've already resolve
I think we should soon make it so you can't disable admission control. Not sure 
that even has to wait for 3.0 if the default configuration is logically 
equivalent (other then fail fast vs fail slow).


PS4, Line 429: GetReservationLimitFromMemLimit(mem_l
> Do you mean something besides what gets handled on l416-l424?
I missed that. But should line 425 be an else-if then?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iabe87ce8f460356cfe4d1be4d7092c5900f9d79b
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5677: limit clean page memory consumption

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5677: limit clean page memory consumption
..


IMPALA-5677: limit clean page memory consumption

Adds the following flag:

  -buffer_pool_clean_pages_limit ((Advanced) Limit on bytes of clean spilled
pages that will be accumulated in the buffer pool. Specified as number of
bytes ('[bB]?'), megabytes ('[mM]'), gigabytes
('[gG]'), or percentage of the buffer pool limit ('%').
Defaults to bytes if no unit is given..) type: string default: "10%"

This helps prevent Impala accumulating excessive amounts of clean pages,
which can cause some problems in practice:
* The OS buffer cache is squeezed, reducing I/O performance from HDFS
  and potentially reducing the ability of the OS to absorb writes from
  Impala without blocking.
* Impala process memory consumption can expand more than users or admins
  might expect. E.g. if one query is running with a mem_limit of 1GB,
  many people will be surprised if the process inflates to the full
  process limit of 100GB. Impala doesn't provide any guarantees except
  from staying within the process mem_limit, but this could be a
  surprising divergence from past behaviour.

Observability:
A new metric buffer-pool.clean-pages-limit is added.

Testing:
Added a backend test to directly test that clean pages are evicted.
Ran in a loop to flush out any flakiness.

Ran exhaustive tests.

Change-Id: Ib6b687ab4bdddf07d9d6c997ff814aa3976042f9
Reviewed-on: http://gerrit.cloudera.org:8080/7653
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
M be/src/common/global-flags.cc
M be/src/runtime/bufferpool/buffer-allocator-test.cc
M be/src/runtime/bufferpool/buffer-allocator.cc
M be/src/runtime/bufferpool/buffer-allocator.h
M be/src/runtime/bufferpool/buffer-pool-test.cc
M be/src/runtime/bufferpool/buffer-pool.cc
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/suballocator-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/test-env.cc
M be/src/util/memory-metrics.cc
M be/src/util/memory-metrics.h
M common/thrift/metrics.json
14 files changed, 243 insertions(+), 84 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Tim Armstrong: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib6b687ab4bdddf07d9d6c997ff814aa3976042f9
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5677: limit clean page memory consumption

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5677: limit clean page memory consumption
..


Patch Set 7: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b687ab4bdddf07d9d6c997ff814aa3976042f9
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5452: Rewrite test case to avoid 'pos'.

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5452: Rewrite test case to avoid 'pos'.
..


IMPALA-5452: Rewrite test case to avoid 'pos'.

The original test case accessed the 'pos' field of nested
collections. The query results could vary when reloading
the data because the order of items within a nested
collection is not necessarily the same accross loads.

This patch reformulates the test to avoid 'pos'.

Change-Id: I32e47f0845da8b27652faaceae834e025ecff42a
Reviewed-on: http://gerrit.cloudera.org:8080/7708
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
1 file changed, 13 insertions(+), 0 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Tim Armstrong: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I32e47f0845da8b27652faaceae834e025ecff42a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5452: Rewrite test case to avoid 'pos'.

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5452: Rewrite test case to avoid 'pos'.
..


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32e47f0845da8b27652faaceae834e025ecff42a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] MPALA-5776: Write partial tuple to the correct mempool

2017-08-17 Thread Taras Bobrovytsky (Code Review)

Taras Bobrovytsky has posted comments on this change.

Change subject: MPALA-5776: Write partial tuple to the correct mempool
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7639/5/be/src/exec/hdfs-text-scanner.cc
File be/src/exec/hdfs-text-scanner.cc:

Line 297: boundary_column_.Clear();
> Can we use the existing CopyBoundaryField() function for consistency? Other
I tried it and it doesn't quite work (without clearing the boundary field)

This is a slightly different case. CopyBoundaryField() copies the contents of 
the field followed by the boundary column to memory allocated from the row 
batch.

In this case, after FillColumns(), field_locations is pointing at the data in 
the boundary column. We can trick CopyBoundaryField() by clearing the boundary 
column right before CopyBoundaryField(), but it's such a weird thing to do. (It 
looks weird because we just cleared the boundary column, why are we trying to 
copy out of it?)

Another thing we can do, is check that the field is pointing to a different 
memory location than the boundary column, but I also think that's weird.

I suggest we leave it as is (for me, the way it is now is the least confusing 
way). What do you think?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I60ba5c113aefd17f697c1888fd46a237ef396540
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5531: Fix correctness issue in correlated aggregate subqueries

Alex Behm has posted comments on this change.

Change subject: IMPALA-5531: Fix correctness issue in correlated aggregate 
subqueries
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7706/1/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
File fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java:

Line 681: List slotRefs = Lists.newArrayList();
Might be easier/shorter:
 
List tids = Lists.newArrayList();
getIds(tids, null);
return !Collections.disjoint(tids, subqueryTblIds);


Line 690: // Inequality correlated predicates are not currently supported 
in aggregate
This bug is not specific to inequality predicates. I think it's really that we 
only support equality predicates. For example, try queries like this:

select 1 from functional.alltypes t1
where int_col = (select max(id) from functional.alltypes t2 where t1.id = t2.id 
and coalesce(t1.bool_col, t2.bool_col));

select 1 from functional.alltypes t1
where int_col = (select max(id) from functional.alltypes t2 where t1.id = t2.id 
and t1.string_col like t2.string_col);

Those queries fail in other mysterious ways, but the point is that this bug is 
not specific to inequality.


Line 691: // subqueries (see IMPALA-5531).
We should be able to run queries with arbitrary correlated predicates if the 
subquery references relative nested collections.


Line 692: if (expr instanceof BinaryPredicate && 
!correlatedPredicates.isEmpty() &&
Let's report which predicate makes the subquery unsupported.

Also the BinaryPredicate check is redundant with Expr.IS_NEQ_BINARY_PREDICATE.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ca7b60ef0543430d2f5a802285254ebb52db2ab
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5352: Age out unused file handles from the cache

2017-08-17 Thread Joe McDonnell (Code Review)

Joe McDonnell has uploaded a new patch set (#2).

Change subject: IMPALA-5352: Age out unused file handles from the cache
..

IMPALA-5352: Age out unused file handles from the cache

Currently, a file handle in the file handle cache will
only be evicted if the cache reaches its capacity. This
means that file handles can be retained for an indefinite
amount of time. This is true even for files that have
been deleted, replaced, or modified. Since a file handle
maintains a file descriptor for local files, this can
prevent the disk space from being freed. Additionally,
unused file handles are wasted memory.

This adds code to evict file handles that have been
unused for longer than a specified threshold. A thread
periodically checks the file handle cache to see if
any file handle should be evicted. The threshold is
specified by 'unused_file_handle_timeout'; it
defaults to 6 hours.

This adds a test to custom_cluster/test_hdfs_fd_caching.py
to verify the eviction behavior.

Change-Id: Iefe04b3e2e22123ecb8b3e494934c93dfb29682e
---
M be/src/runtime/disk-io-mgr-handle-cache.h
M be/src/runtime/disk-io-mgr-handle-cache.inline.h
M be/src/runtime/disk-io-mgr.cc
M tests/custom_cluster/test_hdfs_fd_caching.py
4 files changed, 151 insertions(+), 25 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iefe04b3e2e22123ecb8b3e494934c93dfb29682e
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4863/IMPALA-5311: Correctly account the file type and compression codec

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4863/IMPALA-5311: Correctly account the file type and 
compression codec
..


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1094/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4863/IMPALA-5311: Correctly account the file type and compression codec

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4863/IMPALA-5311: Correctly account the file type and 
compression codec
..


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5809: Relax max minidumps in breakpad test

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5809: Relax max_minidumps in breakpad test
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1093/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I977930ae87b8d4671a89c1e07ba76b12eb92fa55
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5809: Relax max minidumps in breakpad test

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5809: Relax max_minidumps in breakpad test
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I977930ae87b8d4671a89c1e07ba76b12eb92fa55
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4669: [KRPC] Add kudu rpc library to build

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4669: [KRPC] Add kudu_rpc library to build
..


Patch Set 13: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1090/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I33203e95dff07c87a6ec5c7a31b7a583b91849bc
Gerrit-PatchSet: 13
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5602: Fix query optimization for kudu and datasource tables

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5602: Fix query optimization for kudu and datasource 
tables
..


Patch Set 5: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7560/4/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:

Line 166: del 
vector.get_value('exec_option')['exec_single_node_rows_threshold']
> I tried running all tests in this class to check for this and it seems the 
Hmm maybe it was fixed or something.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I93822d67ebda41d5d0456095c429e3915a3f40c4
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5677: limit clean page memory consumption

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5677: limit clean page memory consumption
..


Patch Set 7: Code-Review+2

A different test that is only built under ASAN failed.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b687ab4bdddf07d9d6c997ff814aa3976042f9
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5677: limit clean page memory consumption

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5677: limit clean page memory consumption
..


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1092/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b687ab4bdddf07d9d6c997ff814aa3976042f9
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5677: limit clean page memory consumption

Hello Impala Public Jenkins, Dan Hecht,

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

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

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

Change subject: IMPALA-5677: limit clean page memory consumption
..

IMPALA-5677: limit clean page memory consumption

Adds the following flag:

  -buffer_pool_clean_pages_limit ((Advanced) Limit on bytes of clean spilled
pages that will be accumulated in the buffer pool. Specified as number of
bytes ('[bB]?'), megabytes ('[mM]'), gigabytes
('[gG]'), or percentage of the buffer pool limit ('%').
Defaults to bytes if no unit is given..) type: string default: "10%"

This helps prevent Impala accumulating excessive amounts of clean pages,
which can cause some problems in practice:
* The OS buffer cache is squeezed, reducing I/O performance from HDFS
  and potentially reducing the ability of the OS to absorb writes from
  Impala without blocking.
* Impala process memory consumption can expand more than users or admins
  might expect. E.g. if one query is running with a mem_limit of 1GB,
  many people will be surprised if the process inflates to the full
  process limit of 100GB. Impala doesn't provide any guarantees except
  from staying within the process mem_limit, but this could be a
  surprising divergence from past behaviour.

Observability:
A new metric buffer-pool.clean-pages-limit is added.

Testing:
Added a backend test to directly test that clean pages are evicted.
Ran in a loop to flush out any flakiness.

Ran exhaustive tests.

Change-Id: Ib6b687ab4bdddf07d9d6c997ff814aa3976042f9
---
M be/src/common/global-flags.cc
M be/src/runtime/bufferpool/buffer-allocator-test.cc
M be/src/runtime/bufferpool/buffer-allocator.cc
M be/src/runtime/bufferpool/buffer-allocator.h
M be/src/runtime/bufferpool/buffer-pool-test.cc
M be/src/runtime/bufferpool/buffer-pool.cc
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/suballocator-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/test-env.cc
M be/src/util/memory-metrics.cc
M be/src/util/memory-metrics.h
M common/thrift/metrics.json
14 files changed, 243 insertions(+), 84 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib6b687ab4bdddf07d9d6c997ff814aa3976042f9
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5602: Fix query optimization for kudu and datasource tables

2017-08-17 Thread Bikramjeet Vig (Code Review)

Hello Matthew Jacobs, Tim Armstrong,

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

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

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

Change subject: IMPALA-5602: Fix query optimization for kudu and datasource 
tables
..

IMPALA-5602: Fix query optimization for kudu and datasource tables

Fix a bug where the following queries on kudu and datasource tables
were incorrectly being optimized as a 'small query' and therefore
running on a single node with a single scanner thread:

(A) that have all their predicates pushed to the underlying storage
layer and have a limit
(B) table stats missing + Conditions in (A)

Testing:
For (A): Added a frontend planner test for kudu tables
For (B): Added an end to end planner test for kudu and a query test
for datasource tables

Change-Id: I93822d67ebda41d5d0456095c429e3915a3f40c4
---
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/ScanNode.java
M fe/src/main/java/org/apache/impala/util/MaxRowsProcessedVisitor.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/data-source-tables.test
M tests/query_test/test_kudu.py
M tests/query_test/test_queries.py
9 files changed, 687 insertions(+), 262 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I93822d67ebda41d5d0456095c429e3915a3f40c4
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5602: Fix query optimization for kudu and datasource tables

2017-08-17 Thread Bikramjeet Vig (Code Review)

Bikramjeet Vig has posted comments on this change.

Change subject: IMPALA-5602: Fix query optimization for kudu and datasource 
tables
..


Patch Set 3:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/7560/4/fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
File fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java:

Line 367
> formatting
Done


http://gerrit.cloudera.org:8080/#/c/7560/3/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:

Line 527: return !kuduConjuncts_.isEmpty();
> nit oneline
Done


http://gerrit.cloudera.org:8080/#/c/7560/3/fe/src/main/java/org/apache/impala/planner/ScanNode.java
File fe/src/main/java/org/apache/impala/planner/ScanNode.java:

PS3, Line 219: (){
> nit: missing space
Done


PS3, Line 219: (){
> You are correct, that should be fixed as well.
Done


Line 219:   public boolean hasPushedConjuncts(){
> I think DataSourceScanNode can also push conjuncts.
Done


http://gerrit.cloudera.org:8080/#/c/7560/4/testdata/workloads/functional-query/queries/QueryTest/data-source-tables.test
File 
testdata/workloads/functional-query/queries/QueryTest/data-source-tables.test:

PS4, Line 138: 
> can you put the full key here, i.e. + "(non default)" or whatever? That way
adding "(non default)" for now. Will switch to whatever we decide to add in 
IMPALA-5784 for representing changes made by planner.


http://gerrit.cloudera.org:8080/#/c/7560/3/tests/query_test/test_kudu.py
File tests/query_test/test_kudu.py:

Line 1007:   def test_missing_table_stats(self, cursor, kudu_client, 
unique_database):
> This needs @SkipIfNotHdfsMinicluster.plans since it's assuming a 3 node min
Done


PS3, Line 1019: self.client.execute("set explain_level=3")
> you can actually use client.set_configuration(config_map)
Done


PS3, Line 1024: assert str(result).find("hosts=3 instances=3") != -1
> the more python-y way to say this is
Done


http://gerrit.cloudera.org:8080/#/c/7560/4/tests/query_test/test_kudu.py
File tests/query_test/test_kudu.py:

PS4, Line 1005: 
> creating a table.
Done


PS4, Line 1010:  k
> JIRA number:
Done


http://gerrit.cloudera.org:8080/#/c/7560/4/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:

Line 165: 
> nit: missing .
Done


Line 166:   def test_distinct_estimate(self, vector):
> I think this is modifying the vector in-place. I.e. the change can leak out
I tried running all tests in this class to check for this and it seems the 
changes to the vector do not carry on to other tests.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I93822d67ebda41d5d0456095c429e3915a3f40c4
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5677: limit clean page memory consumption

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5677: limit clean page memory consumption
..


Patch Set 6: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1089/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b687ab4bdddf07d9d6c997ff814aa3976042f9
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5811: Add 'backends' tab to query details pages

2017-08-17 Thread Bharath Vissapragada (Code Review)

Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-5811: Add 'backends' tab to query details pages
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7711/1/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

Line 556:   value->AddMember("num_instances", fragments_.size(), 
document->GetAllocator());
I think we should include some form of total_ranges_complete_ in this page. I'd 
prefer something like num_remaining_scan_ranges, if its easy to plumb that 
value (completed scan ranges is available directly in the backend state). I've 
seen many queries stuck in reading scan ranges (potentially due to some 
underlying FS problem) causing the whole pipeline to hang. So this metric might 
help such cases.


PS1, Line 569: status_
use GetStatus()? It takes the BackendState::lock_


http://gerrit.cloudera.org:8080/#/c/7711/1/be/src/service/impala-http-handler.cc
File be/src/service/impala-http-handler.cc:

PS1, Line 710: request_state
UNLIKELY(request_state.get()...) ?


http://gerrit.cloudera.org:8080/#/c/7711/1/www/query_backends.tmpl
File www/query_backends.tmpl:

Line 57: 
Does it make sense to add a checkbox to show only backends with unfinished 
fragments? Typically while debugging queries with tons of fragments we will be 
interested only in laggers.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5809: Relax max minidumps in breakpad test

Hello Tim Armstrong,

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

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

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

Change subject: IMPALA-5809: Relax max_minidumps in breakpad test
..

IMPALA-5809: Relax max_minidumps in breakpad test

The change to address IMPALA-5769 added periodic cleaning for minidumps,
which got in the way of the other minidump tests.

This change sets max_minidumps to the default value (9) for all tests to
keep the cleanup thread from interfering, and then sets a smaller limit
where needed.

Change-Id: I977930ae87b8d4671a89c1e07ba76b12eb92fa55
---
M tests/custom_cluster/test_breakpad.py
1 file changed, 9 insertions(+), 10 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I977930ae87b8d4671a89c1e07ba76b12eb92fa55
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5809: Relax max minidumps in breakpad test

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5809: Relax max_minidumps in breakpad test
..


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7716/2//COMMIT_MSG
Commit Message:

Line 12: This change increases the max_minidumps for all tests to keep the
Seems stale.


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

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

[Impala-ASF-CR] IMPALA-5809: Relax max minidumps in breakpad test

Lars Volker has uploaded a new patch set (#2).

Change subject: IMPALA-5809: Relax max_minidumps in breakpad test
..

IMPALA-5809: Relax max_minidumps in breakpad test

The change to address IMPALA-5769 added periodic cleaning for minidumps,
which got in the way of the other minidump tests.

This change increases the max_minidumps for all tests to keep the
cleanup thread from interfering, and then sets a smaller limit where
needed.

Change-Id: I977930ae87b8d4671a89c1e07ba76b12eb92fa55
---
M tests/custom_cluster/test_breakpad.py
1 file changed, 9 insertions(+), 10 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I977930ae87b8d4671a89c1e07ba76b12eb92fa55
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5811: Add 'backends' tab to query details pages

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5811: Add 'backends' tab to query details pages
..


Patch Set 2:

That's deliberate - preserve the last seen set of states. I could change 
something to add an indicator if the coordinator has stopped responding, but 
that would be pretty tricky to do. Note that this behaviour is the same as 
other pages that simply stop updating when the query is done.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2

Hello Dan Hecht,

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

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

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

Change subject: IMPALA-4833: Compute precise per-host reservation size, pt2
..

IMPALA-4833: Compute precise per-host reservation size, pt2

Addresses some comments from the CR:
https://gerrit.cloudera.org/#/c/7630/8

Notably changes the name of initial_reservation_total_bytes
to initial_reservation_total_claims and attempts to clarify
comments.

Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/query-state.cc
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M common/thrift/ImpalaInternalService.thrift
M common/thrift/Planner.thrift
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
7 files changed, 39 insertions(+), 27 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs

[Impala-ASF-CR] IMPALA-5811: Add 'backends' tab to query details pages

Lars Volker has posted comments on this change.

Change subject: IMPALA-5811: Add 'backends' tab to query details pages
..


Patch Set 2:

> Presumably the query was a DDL statement, so there was no
 > coordinator. Thanks for the report, hopefully fixed now.
 > 
 > Screenshot at 
 > https://gist.githubusercontent.com/henryr/1a50fc1ec7474353b84343815c9f867f/raw/f23d873c8d39c10dc00ffc4a1c30a9dbec3e0251/screen-shot.png

The segfault is gone and it works for me now. I noticed that after the query 
finishes the list is not updated anymore and looks like it's stuck: 
https://user-images.githubusercontent.com/151514/29438035-09de6f14-8369-11e7-9dba-83e3ef395baf.png

Is there an easy way around it?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4833: Compute precise per-host reservation size, pt2
..


Patch Set 1:

> This looks good to me, but let's still discuss.

I made some notes from a back-channel communication.

Potential improvements/changes:

1) Consider renaming min_reservation_bytes -> initial_reservation_bytes?

This isn't obviously the right thing to unless we also change 
resource_profile_.min_reservation, and I don't think anyone feels strongly 
about that.

2) look into removing initial_reservation_bytes on the thrift structures. I.e. 
because it's a basically trivially calculation, compute it in the BE. This is 
interesting to me because the concept doesn't really make sense outside of the 
implementation in initial-reservation.cc so it's not ideal to keep in the very 
visible thrift structures. That said, Tim has pointed out that it does make 
sense for the FE to compute as much as possible, and for the BE to plumb it 
through. I'm on the fence, but I'll leave it for now and just try to improve 
the comments we have.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5811: Add 'backends' tab to query details pages

Henry Robinson has uploaded a new patch set (#2).

Change subject: IMPALA-5811: Add 'backends' tab to query details pages
..

IMPALA-5811: Add 'backends' tab to query details pages

Add a 'backends' tab to query details pages which shows:

  * host
  * total number of fragment instances for that query on that backend
  * number of still-running fragment instances
  * if the backend is complete (i.e. all instances finished)
  * peak memory consumption
  * the time, in ms, since a status report was received at the
  * coordinator from that backend.

The table refreshes itself every second, controllable by a check-box. If
the query has completed, no information is displayed.

Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/service/impala-http-handler.cc
M be/src/service/impala-http-handler.h
A www/query_backends.tmpl
M www/query_detail_tabs.tmpl
8 files changed, 171 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker

[Impala-ASF-CR] IMPALA-5811: Add 'backends' tab to query details pages

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5811: Add 'backends' tab to query details pages
..


Patch Set 1:

Presumably the query was a DDL statement, so there was no coordinator. Thanks 
for the report, hopefully fixed now. 

Screenshot at 
https://gist.githubusercontent.com/henryr/1a50fc1ec7474353b84343815c9f867f/raw/f23d873c8d39c10dc00ffc4a1c30a9dbec3e0251/screen-shot.png

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: No

[Impala-ASF-CR] Propagate HAVE PIPE2 compile time value to files that use it

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged.

Change subject: Propagate HAVE_PIPE2 compile time value to files that use it
..


Propagate HAVE_PIPE2 compile time value to files that use it

The HAVE_PIPE2 is a variable that tracks whether a platform has the
system function pipe2() present.

This value was not propagated to the appropriate file that uses it,
causing its value to always be 0, and the wrong branch to be taken
at compile time.

This fixes it by propagating the value to the file.

Change-Id: I6cdc343da35a34be8d95fbea3543d080dbc1ec29
Reviewed-on: http://gerrit.cloudera.org:8080/7705
Reviewed-by: Henry Robinson 
Tested-by: Impala Public Jenkins
---
M be/src/kudu/util/subprocess.cc
1 file changed, 2 insertions(+), 0 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Henry Robinson: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I6cdc343da35a34be8d95fbea3543d080dbc1ec29
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] Propagate HAVE PIPE2 compile time value to files that use it

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: Propagate HAVE_PIPE2 compile time value to files that use it
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6cdc343da35a34be8d95fbea3543d080dbc1ec29
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5810: reduce minimum non-reservation memory

Matthew Jacobs has abandoned this change.

Change subject: IMPALA-5810: reduce minimum non-reservation memory
..


Abandoned

absorbed by https://gerrit.cloudera.org/#/c/7678/

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I28d7cc4b969b5ac47890a673eb4a6716047d284d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5811: Add 'backends' tab to query details pages

2017-08-17 Thread Bharath Vissapragada (Code Review)

Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-5811: Add 'backends' tab to query details pages
..


Patch Set 1:

Great supportability change. Mind adding a screenshot of the newly added tab?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5452: Rewrite test case to avoid 'pos'.

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5452: Rewrite test case to avoid 'pos'.
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1091/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32e47f0845da8b27652faaceae834e025ecff42a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5644: Reject queries if min reservation is too large

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5644: Reject queries if min reservation is too large
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7678/4/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:

PS4, Line 153: if (query_options().__isset.buffer_pool_limit
 :   && query_options().buffer_pool_limit > 0) {
 : max_reservation = query_options().buffer_pool_limit;
 :   }
> Question for Tim & Dan:
I tend to think of buffer_pool_limit as advanced option to directly override 
the default buffer pool limit, instead of being an additional constraint.


http://gerrit.cloudera.org:8080/#/c/7678/4/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:

Line 125: "more information.";
> That's right, at least not as we currently think about admission control. I
Yeah this overall seemed simplest since at this point we've already resolved 
the resource pool and figured out the applicable limits.

It seemed ok that it would fail later with admission control disabled, since 
that is an unusual suboptimal configuration.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iabe87ce8f460356cfe4d1be4d7092c5900f9d79b
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5788: Fix agg node crash when grouping by nondeterministic exprs

2017-08-17 Thread Bikramjeet Vig (Code Review)

Bikramjeet Vig has uploaded a new change for review.

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

Change subject: IMPALA-5788: Fix agg node crash when grouping by 
nondeterministic exprs
..

IMPALA-5788: Fix agg node crash when grouping by nondeterministic exprs

Fixed a bug where impala crashes during execution of an aggregation
query using nondeterministic grouping expressions. This happens when
it tries to rebuild a spilled partition that can fit in memory and rows
get re-hashed to a partition other than the spilled one due to the use
of nondeterministic expressions.

Testing:
Added a query test to verify successful execution.

Change-Id: Ibdb09239577b3f0a19d710b0d148e882b0b73e23
---
M be/src/exec/partitioned-aggregation-node.cc
M be/src/exec/partitioned-aggregation-node.h
M testdata/workloads/functional-query/queries/QueryTest/spilling.test
3 files changed, 44 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/7714/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibdb09239577b3f0a19d710b0d148e882b0b73e23
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig

[Impala-ASF-CR] Python profile decoding tools

Lars Volker has abandoned this change.

Change subject: Python profile decoding tools
..


Abandoned

Wrong change

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I48d4d82649857f648344bb94e42d2f5d51383606
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker

[Impala-ASF-CR] IMPALA-5809: Relax max minidumps in breakpad test

Lars Volker has uploaded a new change for review.

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

Change subject: IMPALA-5809: Relax max_minidumps in breakpad test
..

IMPALA-5809: Relax max_minidumps in breakpad test

The change to address IMPALA-5769 added periodic cleaning for minidumps,
which got in the way of the other minidump tests.

This change increases the max_minidumps for all tests to keep the
cleanup thread from interfering, and then sets a smaller limit where
needed.

Change-Id: I977930ae87b8d4671a89c1e07ba76b12eb92fa55
---
M tests/custom_cluster/test_breakpad.py
1 file changed, 7 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/7716/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I977930ae87b8d4671a89c1e07ba76b12eb92fa55
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker

[Impala-ASF-CR] Python profile decoding tools

Lars Volker has uploaded a new change for review.

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

Change subject: Python profile decoding tools
..

Python profile decoding tools

Change-Id: I48d4d82649857f648344bb94e42d2f5d51383606
---
A analyze.py
A decode_profile.py
2 files changed, 277 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/7715/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I48d4d82649857f648344bb94e42d2f5d51383606
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker

[Impala-ASF-CR] IMPALA-5644: Reject queries if min reservation is too large

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5644: Reject queries if min reservation is too large
..


Patch Set 4:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/7678/4/be/src/runtime/bufferpool/reservation-util.h
File be/src/runtime/bufferpool/reservation-util.h:

PS4, Line 44: buffer reservation limi
> is it clear that this is talking about the query limit as opposed to the bu
Done


PS4, Line 53: minimum
> i think that should be deleted, i.e. you might want to ask this for a poten
Done


PS4, Line 57: GetMemLimitFromBufferReservation
> the name makes it sound like an inverse of GetBufferReservationLimitFromMem
Done


http://gerrit.cloudera.org:8080/#/c/7678/4/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:

PS4, Line 153: if (query_options().__isset.buffer_pool_limit
 :   && query_options().buffer_pool_limit > 0) {
 : max_reservation = query_options().buffer_pool_limit;
 :   }
Question for Tim & Dan:

Should this really be exclusive w/ checking the mem limit? Don't we want:


max_reservation = mem_limit == -1 ? max_int64 : 
ResUtil::GetBufferReservationLimitFromMemLimit(mem_limit)
if (buffer_pool_limit is set):
  max_reservation = min(query_options.buffer_pool_limit,
 max_reservation)


http://gerrit.cloudera.org:8080/#/c/7678/4/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:

PS4, Line 119: greater than
> at least? (off by one?) 
Done


Line 125: "more information.";
> both of these two reasons aren't really admission control specific, right? 
That's right, at least not as we currently think about admission control. It 
could go somewhere else but I thought it made sense to do all of our 
resource-related query 'rejecting' in one place, and I think it makes sense for 
admission control to be that place.

I also don't think people should be running with AC disabled anymore. It's 
better to leave AC enabled but w/ unlimited limits (which is the default 
behavior). I think there shouldn't be many users with AC explicitly disabled 
anymore. Maybe I'll mark those flags as deprecated and remove in Impala 3.0?


PS4, Line 428: buffer_reservation_limit
> might be nice to name this consistently with query-state.cc naming, to make
Done


PS4, Line 429: GetBufferReservationLimitFromMemLimit
> what about the case of buffer_pool_limit set explicitly?
Do you mean something besides what gets handled on l416-l424?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iabe87ce8f460356cfe4d1be4d7092c5900f9d79b
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

Pranay Singh has posted comments on this change.

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7712/1//COMMIT_MSG
Commit Message:

PS1, Line 20: Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
> please remove this line and resubmit - Gerrit uses this as its unique ID fo
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5811: Add 'backends' tab to query details pages

Lars Volker has posted comments on this change.

Change subject: IMPALA-5811: Add 'backends' tab to query details pages
..


Patch Set 1:

I tried this on my local machine while a query was running and hit a segfault 
in BackendsToJson:

C  [libpthread.so.0+0xa404]  pthread_mutex_lock+0x4
C  [impalad+0xe122fa]  
boost::lock_guard::lock_guard(boost::mutex&)+0x2a
C  [impalad+0x16dc68f]  
impala::Coordinator::BackendsToJson(rapidjson::GenericDocument*)+0x37
C  [impalad+0x1160188]  
impala::ImpalaHttpHandler::QueryBackendsHandler(std::map > const&, 
rapidjson::GenericDocument*)+0x28c

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5810: reduce minimum non-reservation memory

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5810: reduce minimum non-reservation memory
..


Patch Set 1:

works for me

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I28d7cc4b969b5ac47890a673eb4a6716047d284d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

Pranay Singh has uploaded a new patch set (#2).

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..

IMPALA-5108:idle_session_timeout kicks in later than expected

Fix: The issue was caused because polling frequency to check for
the session to be expired was half of session timeout, which is
around ~1 min. The polling frequency has been increased, it's changed
to be around 1 sec with this change.

The test changes in session-expiry-test is based on introducing
variable names for the numeric constants and to reduce the time of
max_idle_timeout_ms.

Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
---
M be/src/service/impala-server.cc
M be/src/service/session-expiry-test.cc
2 files changed, 10 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-4786: Clean up how ImpalaServers are created

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4786: Clean up how ImpalaServers are created
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7673/1/be/src/testutil/in-process-servers.cc
File be/src/testutil/in-process-servers.cc:

Line 96:   RETURN_IF_ERROR(exec_env_->Init());
> Hm, I didn't see much of a rebase conflict here. I'm really hoping we can g
Yeah I reverted the change - changing the startup order caused other problems.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d55fbe0f4f7a1fd48993da46863b66e521feaae
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

Pranay Singh has uploaded a new patch set (#2).

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..

IMPALA-5108:idle_session_timeout kicks in later than expected

Fix: The issue was caused because polling frequency to check for
the session to be expired was half of session timeout, which is
around ~1 min. The polling frequency has been increased, it's changed
to be around 1 sec with this change.

The test changes in session-expiry-test is based on introducing
variable names for the numeric constants and to reduce the time of
max_idle_timeout_ms.

Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
---
M be/src/service/impala-server.cc
M be/src/service/session-expiry-test.cc
2 files changed, 10 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-4786: Clean up how ImpalaServers are created

Henry Robinson has uploaded a new patch set (#2).

Change subject: IMPALA-4786: Clean up how ImpalaServers are created
..

IMPALA-4786: Clean up how ImpalaServers are created

ImpalaServer had to be created via an awkward CreateImpalaServer()
method that took a lot of arguments. This patch refactors that code (in
anticipation of some KRPC changes) to follow a more normal lifecycle:

1. ImpalaServer* server = new ImpalaServer(ExecEnv*)
2. RETURN_IF_ERROR(server->Init()) // for error-returning init operations
3. RETURN_IF_ERROR(server->Start())
4. server->Join()

Also add ExecEnv::Init(), and move calls to ExecEnv::StartServices() to
ImpalaServer::StartServices(). This captures a dependency that KRPC will
rely on - where initialization of both ExecEnv and ImpalaServer need to
happen before services are started.

This sets up a clean-up of InProcessImpalaServer, which is too heavy for
the work that it does. That work is deferred to a follow-on patch.

Change-Id: I5d55fbe0f4f7a1fd48993da46863b66e521feaae
---
M be/src/exprs/expr-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/scheduler.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M be/src/util/hdfs-util-test.cc
10 files changed, 108 insertions(+), 137 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5d55fbe0f4f7a1fd48993da46863b66e521feaae
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5810: reduce minimum non-reservation memory

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5810: reduce minimum non-reservation memory
..


Patch Set 1: Code-Review+1

Since I'm touching this code as well in https://gerrit.cloudera.org/#/c/7678/

How about I just make this change and then I don't have to deal with merging?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I28d7cc4b969b5ac47890a673eb4a6716047d284d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7712/1//COMMIT_MSG
Commit Message:

PS1, Line 20: Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
please remove this line and resubmit - Gerrit uses this as its unique ID for 
patch sets, so it thinks that there's two different patches for this one issue.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Henry Robinson 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3208: max row size option

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3208: max_row_size option
..


Patch Set 10:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7629/10/common/thrift/PlanNodes.thrift
File common/thrift/PlanNodes.thrift:

PS10, Line 503:   4: optional i64 max_row_buffer_size
  : }
> maybe this will become clear to me as I review more code, but why does this
There's an invariant that I forgot to document that this is >= 
spillable_buffer_size. So if max_row_size is low and different nodes have 
different buffer sizes, this can have different values for different nodes.

I've generally tended to put all of the logic for deciding buffer sizes in the 
frontend rather than the backend, so the idea here is that the frontend should 
figure this out per-node then the backend doesn't need to do anything with the 
sizes except from pass the values through to BufferedTupleStream (or whatever)


http://gerrit.cloudera.org:8080/#/c/7629/10/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
File fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java:

PS10, Line 251: // Analytic uses a single buffer size.
> Why? It looks like every other node has the same behavior - can this just d
The hash join and the agg use multiple buffer sizes - buffers use the default 
buffer size and larger buffers are allocated only for larger rows.


http://gerrit.cloudera.org:8080/#/c/7629/10/fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java
File fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java:

PS10, Line 37: et
set


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic70f6dddbcef124bb4b329ffa2e42a74a1826570
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

Pranay Singh has uploaded a new change for review.

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

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..

IMPALA-5108:idle_session_timeout kicks in later than expected

Fix: The issue was caused because polling frequency to check for
the session to be expired was half of session timeout, which is
around ~1 min. The polling frequency has been increased, it's changed
to be around 1 sec with this change.

The test changes in session-expiry-test is based on introducing
variable names for the numeric constants and to reduce the time of
max_idle_timeout_ms.

Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76

Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
---
M be/src/service/impala-server.cc
M be/src/service/session-expiry-test.cc
2 files changed, 10 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/7712/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh

[Impala-ASF-CR] (PREVIEW) IMPALA-5684: Optionally run be tests in sharded mode

Henry Robinson has posted comments on this change.

Change subject: (PREVIEW) IMPALA-5684: Optionally run be tests in sharded mode
..


Patch Set 2:

Thanks for the review, btw - I'm working on some ergonomic improvements before 
resubmitting which is why this is taking a little while to resurface.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I865db25b07728f3886133316ded5122c60490967
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5810: reduce minimum non-reservation memory

Dan Hecht has posted comments on this change.

Change subject: IMPALA-5810: reduce minimum non-reservation memory
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I28d7cc4b969b5ac47890a673eb4a6716047d284d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5644: Reject queries if min reservation is too large

Dan Hecht has posted comments on this change.

Change subject: IMPALA-5644: Reject queries if min reservation is too large
..


Patch Set 4:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/7678/4/be/src/runtime/bufferpool/reservation-util.h
File be/src/runtime/bufferpool/reservation-util.h:

PS4, Line 44: buffer reservation limi
is it clear that this is talking about the query limit as opposed to the 
buffer-pool wide limit? I guess maybe, since we refer to the latter as the 
"buffer pool limit", but maybe worth clarifying.


PS4, Line 53: minimum
i think that should be deleted, i.e. you might want to ask this for a potential 
initial reservation that's larger than the minimum.


PS4, Line 57: GetMemLimitFromBufferReservation
the name makes it sound like an inverse of 
GetBufferReservationLimitFromMemLimit(), but it's a little different 
conceptually. How about GetMinMemLimitFromBufferReservation or 
GetRequiredMemLimitFromBufferReservation...

also, if you want to shorten, I think removing the word "buffer" from the name 
would be okay.


http://gerrit.cloudera.org:8080/#/c/7678/4/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:

PS4, Line 119: greater than
at least? (off by one?) 
and are we suggesting that mem_limit be set to this value? If so, maybe say 
that more explicitly: Set mem_limit to at least $2?


Line 125: "more information.";
both of these two reasons aren't really admission control specific, right? i'm 
okay with having them here though, but what happens when admission control is 
disabled? it fails later to get the initial reservation, I guess?


PS4, Line 428: buffer_reservation_limit
might be nice to name this consistently with query-state.cc naming, to make it 
clear we're talking about the same thing, which I think calls it 
max_reservation (or rename that one).


PS4, Line 429: GetBufferReservationLimitFromMemLimit
what about the case of buffer_pool_limit set explicitly?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iabe87ce8f460356cfe4d1be4d7092c5900f9d79b
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5811: Add 'backends' tab to query details pages

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-5811: Add 'backends' tab to query details pages
..

IMPALA-5811: Add 'backends' tab to query details pages

Add a 'backends' tab to query details pages which shows:

  * host
  * total number of fragment instances for that query on that backend
  * number of still-running fragment instances
  * if the backend is complete (i.e. all instances finished)
  * peak memory consumption
  * the time, in ms, since a status report was received at the
  * coordinator from that backend.

The table refreshes itself every second, controllable by a check-box. If
the query has completed, no information is displayed.

Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/service/impala-http-handler.cc
M be/src/service/impala-http-handler.h
A www/query_backends.tmpl
M www/query_detail_tabs.tmpl
8 files changed, 171 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/7711/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7711
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson

[Impala-ASF-CR] IMPALA-3208: max row size option

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3208: max_row_size option
..

Patch Set 10:

(3 comments)

Starting with my first high level question as I may be missing something

http://gerrit.cloudera.org:8080/#/c/7629/10/common/thrift/PlanNodes.thrift
File common/thrift/PlanNodes.thrift:

PS10, Line 503: 4: optional i64 max_row_buffer_size
: }
maybe this will become clear to me as I review more code, but why does this
have to go in this struct? Does it really be different for different nodes? I
think it'd be easier to reason about if you didn't have to think about this
extra per-node resource knob.

http://gerrit.cloudera.org:8080/#/c/7629/10/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
File fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java:

PS10, Line 251: // Analytic uses a single buffer size.
Why? It looks like every other node has the same behavior - can this just do
the same? Then we could avoid making the max row buffer bytes a new per-node
option (i.e. it'd be the same max).

http://gerrit.cloudera.org:8080/#/c/7629/10/fe/src/main/java/org/apache/impala/planner/Planner.java
File fe/src/main/java/org/apache/impala/planner/Planner.java:

PS10, Line 66: setMemEstimateBytes(MIN_PER_HOST_MEM_ESTIMATE_BYTES)
: .setMinReservationBytes(0)
I think you could give this last line another tab, for good measure

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic70f6dddbcef124bb4b329ffa2e42a74a1826570
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5452: Rewrite test case to avoid 'pos'.

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5452: Rewrite test case to avoid 'pos'.
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32e47f0845da8b27652faaceae834e025ecff42a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5810: reduce minimum non-reservation memory

Tim Armstrong has uploaded a new change for review.

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

Change subject: IMPALA-5810: reduce minimum non-reservation memory
..

IMPALA-5810: reduce minimum non-reservation memory

See JIRA for experimental results that show this slightly
improves min memory requirements for small queries.

One reason to tweak this is to compensate for the fact that
BufferedBlockMgr didn't count small buffers against the
BlockMgr limit, but BufferPool counts all buffers against it.

Change-Id: I28d7cc4b969b5ac47890a673eb4a6716047d284d
---
M be/src/runtime/query-state.cc
1 file changed, 3 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/7710/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7710
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I28d7cc4b969b5ac47890a673eb4a6716047d284d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong

[Impala-ASF-CR] IMPALA-5452: Rewrite test case to avoid 'pos'.

Alex Behm has uploaded a new patch set (#3).

Change subject: IMPALA-5452: Rewrite test case to avoid 'pos'.
..

IMPALA-5452: Rewrite test case to avoid 'pos'.

The original test case accessed the 'pos' field of nested
collections. The query results could vary when reloading
the data because the order of items within a nested
collection is not necessarily the same accross loads.

This patch reformulates the test to avoid 'pos'.

Change-Id: I32e47f0845da8b27652faaceae834e025ecff42a
---
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
1 file changed, 13 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32e47f0845da8b27652faaceae834e025ecff42a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5452: Rewrite test case to avoid 'pos'.

Alex Behm has posted comments on this change.

Change subject: IMPALA-5452: Rewrite test case to avoid 'pos'.
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7708/1/testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
File 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test:

Line 666:  (select lead(l.l_linenumber) over (order by l.l_shipdate)
> I'm not sure that l_shipdate gives a total order. I think the below query g
You are right. I changed the query to have a total ordering (the PK of lineitem 
is (o_orderkey,l_linenum))


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32e47f0845da8b27652faaceae834e025ecff42a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5452: Rewrite test case to avoid 'pos'.

Alex Behm has uploaded a new patch set (#2).

Change subject: IMPALA-5452: Rewrite test case to avoid 'pos'.
..

IMPALA-5452: Rewrite test case to avoid 'pos'.

The original test case accessed the 'pos' field of nested
collections. The query results could vary when reloading
the data because the order of items within a nested
collection is not necessarily the same accross loads.

This patch reformulates the test to avoid 'pos'.

Change-Id: I32e47f0845da8b27652faaceae834e025ecff42a
---
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
1 file changed, 13 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32e47f0845da8b27652faaceae834e025ecff42a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

Pranay Singh has posted comments on this change.

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7709/1/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

PS1, Line 1817: that
> whether
that replaced by whether, done


PS1, Line 1817:   
> double space
Double space Removed


PS1, Line 1819: int64_t session_timeout_min_ms = 1000;
> Rename this to something like sleep_time_ms , and make it a constant - e.g.
Made it a constant and changed it to uppercase


PS1, Line 1844: FLAGS_use_local_tz_for_unix_timestamp_conversions = true;
> This is very dangerous: what if some other thread reads FLAGS_use_local_tz_
Removed it


http://gerrit.cloudera.org:8080/#/c/7709/1/be/src/service/session-expiry-test.cc
File be/src/service/session-expiry-test.cc:

PS1, Line 45: num_clients
> constants should be ALL_CAPS
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5602: Fix query optimization for kudu and datasource tables

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5602: Fix query optimization for kudu and datasource 
tables
..


Patch Set 4: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7560/4/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:

Line 165: # that rely on it
nit: missing .


Line 166: del 
vector.get_value('exec_option')['exec_single_node_rows_threshold']
I think this is modifying the vector in-place. I.e. the change can leak out to 
other places. I'm not a py.test expert but we definitely copy the vector in 
other places to avoid this problem.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I93822d67ebda41d5d0456095c429e3915a3f40c4
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5602: Fix query optimization for kudu and datasource tables

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5602: Fix query optimization for kudu and datasource 
tables
..


Patch Set 4:

also it looks like this needs a rebase

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I93822d67ebda41d5d0456095c429e3915a3f40c4
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4863/IMPALA-5311: Correctly account the file type and compression codec

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4863/IMPALA-5311: Correctly account the file type and 
compression codec
..


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5602: Fix query optimization for kudu and datasource tables

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5602: Fix query optimization for kudu and datasource 
tables
..


Patch Set 4: Code-Review+1

(4 comments)

Let's see if Alex and or Tim want to take another look

http://gerrit.cloudera.org:8080/#/c/7560/4/fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
File fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java:

Line 367:   public boolean hasPushedConjuncts(){ return 
!acceptedConjuncts_.isEmpty(); }
formatting


http://gerrit.cloudera.org:8080/#/c/7560/4/testdata/workloads/functional-query/queries/QueryTest/data-source-tables.test
File 
testdata/workloads/functional-query/queries/QueryTest/data-source-tables.test:

PS4, Line 138: s(
can you put the full key here, i.e. + "(non default)" or whatever? That way 
we'll be sure to update it after IMPALA-5784 and then I think this will become 
a more convincing test case because we know the planner was the one that set 
this option.


http://gerrit.cloudera.org:8080/#/c/7560/4/tests/query_test/test_kudu.py
File tests/query_test/test_kudu.py:

PS4, Line 1005: creating table
creating a table.


PS4, Line 1010: "T
JIRA number:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I93822d67ebda41d5d0456095c429e3915a3f40c4
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5452: Rewrite test case to avoid 'pos'.

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5452: Rewrite test case to avoid 'pos'.
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7708/1/testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
File 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test:

Line 666:  (select lead(l.l_linenumber) over (order by l.l_shipdate)
I'm not sure that l_shipdate gives a total order. I think the below query gives 
back a list of the lineitems belonging to orders where there were duplicate 
l_shipdates

  select o_custkey, l_orderkey, l_shipdate
  from tpch_parquet.lineitem join tpch_parquet.orders on o_orderkey = l_orderkey
  where o_custkey < 10 and l_orderkey in (
 select l_orderkey
 from tpch_parquet.lineitem join tpch_parquet.orders on o_orderkey = 
l_orderkey
 where o_custkey < 10 group by 1 having count(l_shipdate) != count(distinct 
l_shipdate))
  order by 1, 2, 3;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32e47f0845da8b27652faaceae834e025ecff42a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7709/1/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

PS1, Line 1817:   
double space


PS1, Line 1817: that
whether


PS1, Line 1819: int64_t session_timeout_min_ms = 1000;
Rename this to something like sleep_time_ms , and make it a constant - e.g.:

  const int64_t SLEEP_TIME_MS = 1000;


PS1, Line 1844: FLAGS_use_local_tz_for_unix_timestamp_conversions = true;
This is very dangerous: what if some other thread reads 
FLAGS_use_local_tz_for_unix_timestamp_conversions concurrently?

I'd leave this change out for now.


http://gerrit.cloudera.org:8080/#/c/7709/1/be/src/service/session-expiry-test.cc
File be/src/service/session-expiry-test.cc:

PS1, Line 45: num_clients
constants should be ALL_CAPS


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-HasComments: Yes

[Impala-ASF-CR] MPALA-5776: Write partial tuple to the correct mempool

Alex Behm has posted comments on this change.

Change subject: MPALA-5776: Write partial tuple to the correct mempool
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7639/5/be/src/exec/hdfs-text-scanner.cc
File be/src/exec/hdfs-text-scanner.cc:

Line 297: boundary_column_.Clear();
Can we use the existing CopyBoundaryField() function for consistency? 
Otherwise, a future reader might be confused why we are not using 
CopyBoundaryField() here.

I believe we could call that after delimited_text_parser_->FillColumns(), 
instead of coping before calling FillColumns().


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I60ba5c113aefd17f697c1888fd46a237ef396540
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

Pranay Singh has uploaded a new change for review.

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

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..

IMPALA-5108:idle_session_timeout kicks in later than expected

Fix: The issue was caused because polling frequency to check for
the session to be expired was half of session timeout, which is
around ~1 min. The polling frequency has been increased, it's changed
to be around 1 sec with this change.

In addition to the above change, the time that displays the Expiry
time of session is displayed in local time zone, this is to better
co-relate with the time detail in the logs.

The test changes in session-expiry-test is based on introducing
variable names for the numeric constants and to reduce the time of
max_idle_timeout_ms.

Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
---
M be/src/service/impala-server.cc
M be/src/service/session-expiry-test.cc
2 files changed, 14 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/7709/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7709
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh

[Impala-ASF-CR] IMPALA-5452: Rewrite test case to avoid 'pos'.

Alex Behm has uploaded a new change for review.

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

Change subject: IMPALA-5452: Rewrite test case to avoid 'pos'.
..

IMPALA-5452: Rewrite test case to avoid 'pos'.

The original test case accessed the 'pos' field of nested
collections. The query results could vary when reloading
the data because the order of items within a nested
collection is not necessarily the same accross loads.

This patch reformulates the test to avoid 'pos'.

Change-Id: I32e47f0845da8b27652faaceae834e025ecff42a
---
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
1 file changed, 13 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/7708/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I32e47f0845da8b27652faaceae834e025ecff42a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm

[Impala-ASF-CR] IMPALA-4669: [KRPC] Add kudu rpc library to build

Hello Michael Ho,

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

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

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

Change subject: IMPALA-4669: [KRPC] Add kudu_rpc library to build
..

IMPALA-4669: [KRPC] Add kudu_rpc library to build

Import FindKRPC.cmake from Apache Kudu.

Change-Id: I33203e95dff07c87a6ec5c7a31b7a583b91849bc
---
M CMakeLists.txt
M be/CMakeLists.txt
A cmake_modules/FindKRPC.cmake
3 files changed, 129 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/5719/13
-- 
To view, visit http://gerrit.cloudera.org:8080/5719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I33203e95dff07c87a6ec5c7a31b7a583b91849bc
Gerrit-PatchSet: 13
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho

[Impala-ASF-CR] IMPALA-5681: release reservation from blocking operators

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5681: release reservation from blocking operators
..


Patch Set 9: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6f4d0ad127d5fcd14b9821a7c127eec11d98692f
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5681: release reservation from blocking operators

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5681: release reservation from blocking operators
..


IMPALA-5681: release reservation from blocking operators

When an in-memory blocking aggregation or join is in the GetNext()
phase where it is outputting accumulated rows then we expect
memory consumption to monotonically decrease because no more
rows will be accumulated in memory.

This change adds support to release unused reservation and makes
use of it for in-memory aggregations and sorts.

We don't release memory for operators with spilled data, since they
may need the reservation to bring it back into memory. We also
don't release memory in subplans, since it will probably be used
in a later iteration of the subplan.

Testing:
Updated spilling test that now requires less memory.

Ran stress test binary search on tpch_parquet. No changes, except
Q18 now requires 325MB instead of 450MB to execute without spilling.

Ran query with two sorts in the same pipeline and watched /memz to
confirm that the first node in the pipeline was incrementally releasing
memory. Added a regression test based on this experiment.

Added a backend test to directly test reservation decreasing.

Change-Id: I6f4d0ad127d5fcd14b9821a7c127eec11d98692f
Reviewed-on: http://gerrit.cloudera.org:8080/7619
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
M be/src/exec/exec-node.cc
M be/src/exec/exec-node.h
M be/src/exec/partitioned-aggregation-node.cc
M be/src/exec/sort-node.cc
M be/src/exec/sort-node.h
M be/src/runtime/bufferpool/buffer-pool-internal.h
M be/src/runtime/bufferpool/buffer-pool-test.cc
M be/src/runtime/bufferpool/buffer-pool.cc
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/sorter.cc
M be/src/runtime/sorter.h
M testdata/workloads/functional-query/queries/QueryTest/spilling-aggs.test
A testdata/workloads/tpch/queries/sort-reservation-usage.test
M tests/query_test/test_sort.py
14 files changed, 187 insertions(+), 11 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Tim Armstrong: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I6f4d0ad127d5fcd14b9821a7c127eec11d98692f
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5641: mem-estimate should never be less than mem-reservation

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5641: mem-estimate should never be less than 
mem-reservation
..


Patch Set 2: Code-Review+1

carry +1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2853300371e31b13d81a763dbafb21709b16c4
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5677: limit clean page memory consumption

Hello Impala Public Jenkins, Dan Hecht,

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

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

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

Change subject: IMPALA-5677: limit clean page memory consumption
..

IMPALA-5677: limit clean page memory consumption

Adds the following flag:

  -buffer_pool_clean_pages_limit ((Advanced) Limit on bytes of clean spilled
pages that will be accumulated in the buffer pool. Specified as number of
bytes ('[bB]?'), megabytes ('[mM]'), gigabytes
('[gG]'), or percentage of the buffer pool limit ('%').
Defaults to bytes if no unit is given..) type: string default: "10%"

This helps prevent Impala accumulating excessive amounts of clean pages,
which can cause some problems in practice:
* The OS buffer cache is squeezed, reducing I/O performance from HDFS
  and potentially reducing the ability of the OS to absorb writes from
  Impala without blocking.
* Impala process memory consumption can expand more than users or admins
  might expect. E.g. if one query is running with a mem_limit of 1GB,
  many people will be surprised if the process inflates to the full
  process limit of 100GB. Impala doesn't provide any guarantees except
  from staying within the process mem_limit, but this could be a
  surprising divergence from past behaviour.

Observability:
A new metric buffer-pool.clean-pages-limit is added.

Testing:
Added a backend test to directly test that clean pages are evicted.
Ran in a loop to flush out any flakiness.

Ran exhaustive tests.

Change-Id: Ib6b687ab4bdddf07d9d6c997ff814aa3976042f9
---
M be/src/common/global-flags.cc
M be/src/runtime/bufferpool/buffer-allocator-test.cc
M be/src/runtime/bufferpool/buffer-allocator.cc
M be/src/runtime/bufferpool/buffer-allocator.h
M be/src/runtime/bufferpool/buffer-pool-test.cc
M be/src/runtime/bufferpool/buffer-pool.cc
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/suballocator-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/test-env.cc
M be/src/util/memory-metrics.cc
M be/src/util/memory-metrics.h
M common/thrift/metrics.json
14 files changed, 241 insertions(+), 83 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib6b687ab4bdddf07d9d6c997ff814aa3976042f9
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5677: limit clean page memory consumption

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5677: limit clean page memory consumption
..


Patch Set 6: Code-Review+2

Rebase caused a compilation error in unit test

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b687ab4bdddf07d9d6c997ff814aa3976042f9
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5796: CTAS for Kudu fails with expr rewrite

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5796: CTAS for Kudu fails with expr rewrite
..


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia67bb33736b5a843663b226cdd0fa5bd839cbea1
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5796: CTAS for Kudu fails with expr rewrite

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5796: CTAS for Kudu fails with expr rewrite
..


IMPALA-5796: CTAS for Kudu fails with expr rewrite

When an expr rewrite occurs, we reanalyze the statement. Some state
that is set in TableDef::analyze() wasn't being reset() first, causing
a failure during reanalysis.

This patch adds TableDef::reset(), which clears the TableDef state
that is set during analyze().

Testing:
- Added a regression test in AnalyzeDDLTest

Change-Id: Ia67bb33736b5a843663b226cdd0fa5bd839cbea1
Reviewed-on: http://gerrit.cloudera.org:8080/7666
Reviewed-by: Thomas Tauber-Marshall 
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
4 files changed, 32 insertions(+), 5 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Thomas Tauber-Marshall: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia67bb33736b5a843663b226cdd0fa5bd839cbea1
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall

[Impala-ASF-CR] IMPALA-5531: Fix correctness issue in correlated aggregate subqueries

2017-08-17 Thread Dimitris Tsirogiannis (Code Review)

Dimitris Tsirogiannis has uploaded a new change for review.

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

Change subject: IMPALA-5531: Fix correctness issue in correlated aggregate 
subqueries
..

IMPALA-5531: Fix correctness issue in correlated aggregate subqueries

This commit fixes an issue where a query will return wrong results if it
has an aggregate subquery with a correlated inequality predicate.
Since the rewrite for this case is not currently supported, an
exception is now thrown during the analysis.

Change-Id: I6ca7b60ef0543430d2f5a802285254ebb52db2ab
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
3 files changed, 64 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/7706/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7706
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6ca7b60ef0543430d2f5a802285254ebb52db2ab
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis

[Impala-ASF-CR] Propagate HAVE PIPE2 compile time value to files that use it

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: Propagate HAVE_PIPE2 compile time value to files that use it
..


Patch Set 1:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1088/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6cdc343da35a34be8d95fbea3543d080dbc1ec29
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No

[Impala-ASF-CR] Propagate HAVE PIPE2 compile time value to files that use it

Henry Robinson has posted comments on this change.

Change subject: Propagate HAVE_PIPE2 compile time value to files that use it
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6cdc343da35a34be8d95fbea3543d080dbc1ec29
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-HasComments: No

[Impala-ASF-CR] Propagate HAVE PIPE2 compile time value to files that use it

2017-08-17 Thread Sailesh Mukil (Code Review)

Sailesh Mukil has uploaded a new change for review.

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

Change subject: Propagate HAVE_PIPE2 compile time value to files that use it
..

Propagate HAVE_PIPE2 compile time value to files that use it

The HAVE_PIPE2 is a variable that tracks whether a platform has the
system function pipe2() present.

This value was not propagated to the appropriate file that uses it,
causing its value to always be 0, and the wrong branch to be taken
at compile time.

This fixes it by propagating the value to the file.

Change-Id: I6cdc343da35a34be8d95fbea3543d080dbc1ec29
---
M be/src/kudu/util/subprocess.cc
1 file changed, 2 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/7705/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7705
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6cdc343da35a34be8d95fbea3543d080dbc1ec29
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil

[Impala-ASF-CR] IMPALA-5677: limit clean page memory consumption

2017-08-17 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5677: limit clean page memory consumption
..


Patch Set 5: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1087/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b687ab4bdddf07d9d6c997ff814aa3976042f9
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5573: Add decimal codegen in text scanner

2017-08-17 Thread Tianyi Wang (Code Review)

Tianyi Wang has posted comments on this change.

Change subject: IMPALA-5573: Add decimal codegen in text scanner
..


Patch Set 8:

LLVM still cannot find symbols with static linking, which I haven't tested 
before.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia65820e969d59094dc92d912a5279fa90f6b179d
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5749: coordinator race hits DCHECK 'num remaining backends > 0'

2017-08-17 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-5749: coordinator race hits DCHECK 
'num_remaining_backends_ > 0'
..


Patch Set 4: Code-Review+1

Carrying forward +1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1528661e5df6d9732ebfeb414576c82ec5c92241
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5749: coordinator race hits DCHECK 'num remaining backends > 0'

2017-08-17 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-5749: coordinator race hits DCHECK 
'num_remaining_backends_ > 0'
..


Patch Set 4:

(4 comments)

> (3 comments)
 > 
 > Thanks - this seems a lot better to me. Did you re-run
 > test_finst_cancel?

Yes, I ran it in a loop again.

http://gerrit.cloudera.org:8080/#/c/7577/2/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

PS2, Line 947: 
 : // for now, abort the query if we see any error except if 
returned_all_results_ is
 : // true (UpdateStatus() initiates cancellation, if it hasn't 
already been)
 : // TODO: clarify control flow here,
> We've got two 'done' concepts here that are close enough to make this a bit
Done


http://gerrit.cloudera.org:8080/#/c/7577/3/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

PS3, Line 971:  << backend_state->impalad_address();
 :   break;
 : }
 :   }
> not your change, but while we're here: can you hoist the VLOG_QUERY_IS_ON c
Done


Line 987: 
> might be clearer just to return Status::OK() here, and avoid the need for a
Done


PS3, Line 991: nsert_exec_status.per_pa
> let's remove this. I can see the race where IsDone() becomes true after lin
I filed IMPALA-5807


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1528661e5df6d9732ebfeb414576c82ec5c92241
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5749: coordinator race hits DCHECK 'num remaining backends > 0'

2017-08-17 Thread Thomas Tauber-Marshall (Code Review)

Hello Henry Robinson, Michael Ho, Sailesh Mukil,

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

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

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

Change subject: IMPALA-5749: coordinator race hits DCHECK 
'num_remaining_backends_ > 0'
..

IMPALA-5749: coordinator race hits DCHECK 'num_remaining_backends_ > 0'

In Coordinator::UpdateBackendExecStatus(), we check if the backend
has already completed with BackendState::IsDone() and return without
applying the update if so to avoid updating num_remaining_backends_
twice for the same completed backend.

The problem is that the value of BackendState::IsDone() is updated by
the call to BackendState::ApplyExecStatusReport() that comes after it,
but these operations are not performed atomically, so if there are
two simultaneous calls to UpdateBackendExecStatus(), they can both
call IsDone(), both get 'false', and then proceed to erroneously both
update num_remaining_backends_, hitting a DCHECK.

This patch modifies ApplyExecStatusReport to return true iff this
report transitioned the backend to a done status, and then only
updates num_remaining_backends_ in this case, ensuring it is only
updated once per backend.

Testing:
- Ran test_finst_cancel_when_query_complete 10,000 times without
  hitting the DCHECK (previously, it would hit about once per 300
  runs).

Change-Id: I1528661e5df6d9732ebfeb414576c82ec5c92241
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
3 files changed, 34 insertions(+), 34 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1528661e5df6d9732ebfeb414576c82ec5c92241
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall

[native-toolchain-CR] Bump Kudu version to b198ed8

Matthew Jacobs has posted comments on this change.

Change subject: Bump Kudu version to b198ed8
..


Patch Set 1:

> > corresponding Impala change?
 > 
 > This was included in the toolchain bump Lars did: 
 > https://gerrit.cloudera.org/#/c/7677/

Right, thanks. My algorithm for making sure we don't drop patches missed that 
combined fix.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia62deb0e61f06de4ce4e95476b85988e1e2754bb
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5158,IMPALA-5236: account for unused buffer pool reservations

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5158,IMPALA-5236: account for unused buffer pool 
reservations
..


Patch Set 7:

PS7 includes the proposed changes. I manually tested and it produces the same 
output as the earlier PS.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idb1fa3110dc893321f9f4e8ced6b7ede12194dad
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5158,IMPALA-5236: account for unused buffer pool reservations

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5158,IMPALA-5236: account for unused buffer pool 
reservations
..


Patch Set 6:

Yeah what Henry is saying makes sense, although I think it's good to make any 
text we expose as clear as possible.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idb1fa3110dc893321f9f4e8ced6b7ede12194dad
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5158,IMPALA-5236: account for unused buffer pool reservations