[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/12004 )
Change subject: IMPALA-7889: Write new logical types in Parquet
..
Patch Set 5:
(17 comments)
http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/hdfs-parquet-table-writer.cc
File be/src/exec/parquet/hdfs-parquet-table-writer.cc:
http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/hdfs-parquet-table-writer.cc@973
PS5, Line 973: parquet::SchemaElement& node = file_metadata_.schema[i + 1];
Maybe choose a better name here?
http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.h
File be/src/exec/parquet/parquet-metadata-utils.h:
http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.h@62
PS5, Line 62: static void FillSchemaElement(parquet::SchemaElement& node,
const ColumnType& type,
We usually put input parameters first, then output, i.e. move node to the end.
By convention we also pass output parameters by pointer.
http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc
File be/src/exec/parquet/parquet-metadata-utils.cc:
http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc@73
PS5, Line 73: };
This is actually the end of the anonymous namespace and confused me. Please
remove the semicolon and add a comment // anonymous namespace.
http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc@78
PS5, Line 78: bool IsSupportedType(PrimitiveType impala_type,
This method should be in the anonymous namespace
http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc@153
PS5, Line 153: static bool IsEncodingSupported(parquet::Encoding::type e) {
This can be in the anonymous namespace, too
http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc@283
PS5, Line 283: void SetIntLogicalType(parquet::SchemaElement& node, int
bitwidth) {
Move to the anonymous namespace. Pass output parameters by pointer and move to
the end of the parameter list. Also add a brief function comment.
http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc@292
PS5, Line 292: node
Can you think of a better name for the schema element than "node"?
http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc@297
PS5, Line 297: if (type.type == TYPE_DECIMAL) {
Should this be a case statement instead?
http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc@310
PS5, Line 310: else if (type.type == TYPE_VARCHAR || type.type == TYPE_CHAR ||
Move else to previous line.
http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc@311
PS5, Line 311: (type.type == TYPE_STRING &&
query_options.parquet_annotate_strings_utf8)) {
Please add a brief comment that VARCHAR and CHAR are always UTF8 annotated, but
STRING is only when set by the query option (I had to look it up).
http://gerrit.cloudera.org:8080/#/c/12004/5/tests/query_test/test_insert_parquet.py
File tests/query_test/test_insert_parquet.py:
http://gerrit.cloudera.org:8080/#/c/12004/5/tests/query_test/test_insert_parquet.py@362
PS5, Line 362: def _ctas_and_get_metadata(self, vector, unique_database,
tmp_dir, source_table):
Add function comment (here and for all other new functions
http://gerrit.cloudera.org:8080/#/c/12004/5/tests/query_test/test_insert_parquet.py@376
PS5, Line 376: file_metadata_list =
get_parquet_metadata_from_hdfs_folder(hdfs_path, tmp_dir)
Maybe you can clean up some of the other uses of os.walk() in this file?
http://gerrit.cloudera.org:8080/#/c/12004/5/tests/query_test/test_insert_parquet.py@388
PS5, Line 388: schema = self._find_schema(schemas, column_name)
You could add
assert schema is not None
You could also add that in _find_schema, since it seems required that the
schema exists.
http://gerrit.cloudera.org:8080/#/c/12004/5/tests/query_test/test_insert_parquet.py@394
PS5, Line 394: bit_width_to_converted_type_map = {8: 15, 16: 16, 32: 17,
64: 18}
Can you replace the values with the names from parquet.thrift?
http://gerrit.cloudera.org:8080/#/c/12004/5/tests/query_test/test_insert_parquet.py@397
PS5, Line 397: assert schema.logicalType.INTEGER is not None
The other types have to be None, right? If so, maybe create a helper
_check_only_one_type_is_set(the_type) and then make sure that all others are
None.
http://gerrit.cloudera.org:8080/#/c/12004/5/tests/query_test/test_insert_parquet.py@432
PS5, Line 432: if
nit: once
http://gerrit.cloudera.org:8080/#/c/12004/5/tests/util/get_parquet_metadata.py
File tests/util/get_parquet_metadata.py:
http://gerrit.cloudera.org:8080/#/c/12004/5/tests/util/get_parquet_metadata.py@181
PS5, Line 181: for f in files:
Add
[Impala-ASF-CR] Add a handy tool to collapse threads in pstacks
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11926 ) Change subject: Add a handy tool to collapse threads in pstacks .. Add a handy tool to collapse threads in pstacks This commit imports a very useful tool that summarizes/collapses threads from gdb pstacks. I found it very useful in analyzing pstacks with large number of threads, specially in Impala's context. Credit to the original author at https://poormansprofiler.org/ (Slightly modified for better input handling). Example Usages: --- summarize-pstacks foo.pstack summarize-pstacks foo_*.pstack cat foo.pstack | summarize-pstacks cat foo_*.pstack | summarize-pstacks Change-Id: I0d00f7cea5fe01ff8b2511f4e04db5d5ad1c82f4 Reviewed-on: http://gerrit.cloudera.org:8080/11926 Reviewed-by: Bharath Vissapragada Tested-by: Impala Public Jenkins --- M bin/rat_exclude_files.txt A bin/summarize-pstacks 2 files changed, 43 insertions(+), 0 deletions(-) Approvals: Bharath Vissapragada: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I0d00f7cea5fe01ff8b2511f4e04db5d5ad1c82f4 Gerrit-Change-Number: 11926 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Add a handy tool to collapse threads in pstacks
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11926 ) Change subject: Add a handy tool to collapse threads in pstacks .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d00f7cea5fe01ff8b2511f4e04db5d5ad1c82f4 Gerrit-Change-Number: 11926 Gerrit-PatchSet: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 05:05:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1048: show sinks in exec summary
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/11967 )
Change subject: IMPALA-1048: show sinks in exec summary
..
Patch Set 8:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/11967/8/be/src/exec/data-sink.h
File be/src/exec/data-sink.h:
http://gerrit.cloudera.org:8080/#/c/11967/8/be/src/exec/data-sink.h@58
PS8, Line 58: Otherwise
: /// 'fragment_idx' should be -1.
Are there any other cases than join builders that are sinks but not at the root
of the fragment?
http://gerrit.cloudera.org:8080/#/c/11967/8/be/src/exec/data-sink.h@60
PS8, Line 60: fragment_idx
I think this could be easier to understand if it was named "data_sink_id"
because that's what we use it for. The comment could still point out that it
needs to be unique and that by convention we pass the fragment index here. The
.cc file could also become easier to read.
http://gerrit.cloudera.org:8080/#/c/11967/8/be/src/exec/data-sink.cc
File be/src/exec/data-sink.cc:
http://gerrit.cloudera.org:8080/#/c/11967/8/be/src/exec/data-sink.cc@72
PS8, Line 72: int fragment_idx = fragment_ctx.fragment.idx;
While you're here, could you add a DCHECK that this is not a JOIN_BUILD_SINK?
http://gerrit.cloudera.org:8080/#/c/11967/8/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:
http://gerrit.cloudera.org:8080/#/c/11967/8/be/src/runtime/coordinator-backend-state.cc@501
PS8, Line 501: // TODO: why do this every time we get an updated instance
profile?
While you're here, do you know why we do this?
http://gerrit.cloudera.org:8080/#/c/11967/8/be/src/runtime/coordinator-backend-state.cc@518
PS8, Line 518: node_exec_summary
rename to exec_summary?
http://gerrit.cloudera.org:8080/#/c/11967/8/be/src/runtime/coordinator.h
File be/src/runtime/coordinator.h:
http://gerrit.cloudera.org:8080/#/c/11967/8/be/src/runtime/coordinator.h@285
PS8, Line 285: TPlanNodeId
Maybe create a separate TDataSinkId (see comment elsewhere)?
http://gerrit.cloudera.org:8080/#/c/11967/8/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:
http://gerrit.cloudera.org:8080/#/c/11967/8/be/src/runtime/coordinator.cc@246
PS8, Line 246: the
nit: be
http://gerrit.cloudera.org:8080/#/c/11967/8/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:
http://gerrit.cloudera.org:8080/#/c/11967/8/be/src/util/runtime-profile.cc@870
PS8, Line 870: // that rely on the plan node id being set there.
Should we create a follow up jira to drop this in Impala 4.0 / on the next
compatibility breaking release?
http://gerrit.cloudera.org:8080/#/c/11967/8/common/thrift/RuntimeProfile.thrift
File common/thrift/RuntimeProfile.thrift:
http://gerrit.cloudera.org:8080/#/c/11967/8/common/thrift/RuntimeProfile.thrift@68
PS8, Line 68: struct TRuntimeProfileNodeMetadata {
Out of curiosity, is this approach preferable to storing the id and the type in
two separate fields, with the type identified by some enum? Or a tagged union
with TPlanNodeId and TDataSinkId?
http://gerrit.cloudera.org:8080/#/c/11967/8/common/thrift/RuntimeProfile.thrift@70
PS8, Line 70: i32
We also have Types.TPlanNodeId, would that fit here?
http://gerrit.cloudera.org:8080/#/c/11967/8/fe/src/main/java/org/apache/impala/planner/DataSink.java
File fe/src/main/java/org/apache/impala/planner/DataSink.java:
http://gerrit.cloudera.org:8080/#/c/11967/8/fe/src/main/java/org/apache/impala/planner/DataSink.java@84
PS8, Line 84: toThrift
I find overloads harder to follow when they have different visibility and such
and would call this toThriftInternal(), but I don't feel strongly about it.
http://gerrit.cloudera.org:8080/#/c/11967/8/shell/impala_client.py
File shell/impala_client.py:
http://gerrit.cloudera.org:8080/#/c/11967/8/shell/impala_client.py@211
PS8, Line 211: "" if is_sink else prettyprint_units(cardinality),
nit: you can write this as
is_sink and "" or prettyprint_units(...)
It is closer to c++ ternary expressions and I find it easier to read. I don't
feel strongly about it though.
http://gerrit.cloudera.org:8080/#/c/11967/8/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:
http://gerrit.cloudera.org:8080/#/c/11967/8/tests/query_test/test_observability.py@97
PS8, Line 97: result.exec_summary[0]
You could name this part to root_sink to make the code less repetitive and
possibly easier to read.
http://gerrit.cloudera.org:8080/#/c/11967/8/tests/query_test/test_observability.py@109
PS8, Line 109: *
nit: + instead of *
--
To view, visit http://gerrit.cloudera.org:8080/11967
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3fdf7bacae8ff597b255da65af453e174ba53544
Gerrit-Change-Number: 11967
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Impala Public
[Impala-ASF-CR] IMPALA-7804: Mitigate s3 consistency issues for test scanners
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11959 ) Change subject: IMPALA-7804: Mitigate s3 consistency issues for test_scanners .. IMPALA-7804: Mitigate s3 consistency issues for test_scanners test_scanners.py has seen several flaky failures on s3 due to eventual consistency. The symptom is Impala being unable to read a file that it just loaded to s3. A large number of tables used in test_scanners.py use the file_utils helper functions for creating the tables. These follow the pattern: 1. Copy files to temporary directory in HDFS/S3/etc 2. Create table 3. Run LOAD DATA to move the files to the table In step #3, LOAD DATA gets the metadata for the table before it runs the move statement on the files. Subsequent queries on the table will not need to reload metadata and can access the file quickly after the move. This changes the ordering to put the files in place before loading metadata. This may improve the likelihood that the filesystem is consistent by the time we read it. Specifically, we now do: 1. Put the files in directory that the table will use when it is created. 2. Create table Neither of these steps load metadata, so the next query that runs will load metadata. Change-Id: Id042496beabe0d0226b347e0653b820fee369f4e Reviewed-on: http://gerrit.cloudera.org:8080/11959 Reviewed-by: Joe McDonnell Tested-by: Impala Public Jenkins --- M tests/common/file_utils.py 1 file changed, 24 insertions(+), 13 deletions(-) Approvals: Joe McDonnell: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11959 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id042496beabe0d0226b347e0653b820fee369f4e Gerrit-Change-Number: 11959 Gerrit-PatchSet: 5 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-7804: Mitigate s3 consistency issues for test scanners
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11959 ) Change subject: IMPALA-7804: Mitigate s3 consistency issues for test_scanners .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11959 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id042496beabe0d0226b347e0653b820fee369f4e Gerrit-Change-Number: 11959 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 30 Nov 2018 01:53:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7047. Refreshing partitions should not make an RPC per file
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11227 ) Change subject: IMPALA-7047. Refreshing partitions should not make an RPC per file .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1478/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11227 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2051b96599206164aaa06ecbdf64374c46eda956 Gerrit-Change-Number: 11227 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 30 Nov 2018 01:50:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2343: Add lifecycle timeline to plan nodes
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/11992 ) Change subject: IMPALA-2343: Add lifecycle timeline to plan nodes .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/11992/7/be/src/exec/kudu-scan-node.cc File be/src/exec/kudu-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/11992/7/be/src/exec/kudu-scan-node.cc@96 PS7, Line 96: > I was kind of ignoring this because I didn't want to deal with it :). I thi Yeah, lets fix it while we're here. -- To view, visit http://gerrit.cloudera.org:8080/11992 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I15341bdb15022bad9814882689ce5cb2939f4653 Gerrit-Change-Number: 11992 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 01:42:00 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7047. Refreshing partitions should not make an RPC per file
Paul Rogers has uploaded a new patch set (#7) to the change originally created by Todd Lipcon. ( http://gerrit.cloudera.org:8080/11227 ) Change subject: IMPALA-7047. Refreshing partitions should not make an RPC per file .. IMPALA-7047. Refreshing partitions should not make an RPC per file The code to handle REFRESH of a single partition was incorrectly ignoring the previously-known file descriptors. This meant that, instead of only calling 'getFileBlockLocations' on the files that had changed since the prior load, it was instead calling it on every file. In addition to refresh of single partitions this also affected refresh of unpartitioned tables (which is implemented as a refresh of its single "default" partition). This patch fixes the behavior by copying over the existing file descriptor list into the re-created partition before refreshing it. A new unit test uses FS statistics to verify the change. The new assertions act as a regression test and fail if I comment out the fix. Additionally, this fixes the case where the old partition had no files to use the optimized 'listLocatedStatus' call. This is triggered when 'REFRESH' picks up a new partition from the HMS added by an external system. I also tested this by pointing my dev box at a remote filesystem that was approximately 60ms away. The initial load of an unpartitioned table with approximately 45000 files takes around 23 seconds in this setup. Without the patch in place, REFRESH was taking upwards of 35 minutes (I got tired and gave up at this point). Multiplying the 60ms round trip by 45000 files estimates 45 minutes. With the fix in place, REFRESH of the same table took around 4.5 seconds. Clearly, in typical setups where catalogd and HDFS are on a shared local network, the gains won't be so dramatic. But, even with a 1ms round trip (plausible when including fixed RPC overhead and potentially congested datacenter networks) this would save 45 seconds on this example table with 45000 files. Change-Id: I2051b96599206164aaa06ecbdf64374c46eda956 --- M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java 3 files changed, 82 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/11227/7 -- To view, visit http://gerrit.cloudera.org:8080/11227 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2051b96599206164aaa06ecbdf64374c46eda956 Gerrit-Change-Number: 11227 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] Add a handy tool to collapse threads in pstacks
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11926 ) Change subject: Add a handy tool to collapse threads in pstacks .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1477/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d00f7cea5fe01ff8b2511f4e04db5d5ad1c82f4 Gerrit-Change-Number: 11926 Gerrit-PatchSet: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 01:27:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7047. Refreshing partitions should not make an RPC per file
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/11227 ) Change subject: IMPALA-7047. Refreshing partitions should not make an RPC per file .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/11227/6/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java File fe/src/test/java/org/apache/impala/catalog/CatalogTest.java: http://gerrit.cloudera.org:8080/#/c/11227/6/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java@392 PS6, Line 392: catalog_.reloadTable(unpartTable); > I tried this patch locally and this test passes regardless of the if/else b Agree. This line should fail if we do an incremental refresh. But, as it turns out, on discovering a new partition, we seem to go down a code path different than the refresh path. So, this code does not actually check the refresh path. (The refresh path is checked by later test code.) To complete the tests, we need a table that has an empty partition (directory exists, but no files in it.) This will trigger the full reload path. Need to work out how to create such a table. -- To view, visit http://gerrit.cloudera.org:8080/11227 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2051b96599206164aaa06ecbdf64374c46eda956 Gerrit-Change-Number: 11227 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 30 Nov 2018 01:23:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7047. Refreshing partitions should not make an RPC per file
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/11227 ) Change subject: IMPALA-7047. Refreshing partitions should not make an RPC per file .. Patch Set 7: Cleaned up the unit tests a bit more. -- To view, visit http://gerrit.cloudera.org:8080/11227 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2051b96599206164aaa06ecbdf64374c46eda956 Gerrit-Change-Number: 11227 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 30 Nov 2018 01:19:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control
Alex Rodoni has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12008 ) Change subject: IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control .. IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control Change-Id: Ie2272433e8dc464c188e0c82a24bf3d0409aa05b Reviewed-on: http://gerrit.cloudera.org:8080/12008 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M docs/shared/impala_common.xml M docs/topics/impala_admission.xml M docs/topics/impala_resource_management.xml 3 files changed, 206 insertions(+), 200 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/12008 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ie2272433e8dc464c188e0c82a24bf3d0409aa05b Gerrit-Change-Number: 12008 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12008 ) Change subject: IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control .. Patch Set 2: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/163/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/12008 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2272433e8dc464c188e0c82a24bf3d0409aa05b Gerrit-Change-Number: 12008 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 01:15:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12008 ) Change subject: IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12008 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2272433e8dc464c188e0c82a24bf3d0409aa05b Gerrit-Change-Number: 12008 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 01:15:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7759: Add Levenshtein edit distance built-in function
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11793 ) Change subject: IMPALA-7759: Add Levenshtein edit distance built-in function .. Patch Set 3: The changes look good to me, just need the length validation -- To view, visit http://gerrit.cloudera.org:8080/11793 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I549d33ab7cebfa10db2934461c8ec91e2cc1cdcb Gerrit-Change-Number: 11793 Gerrit-PatchSet: 3 Gerrit-Owner: Greg Rahn Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 01:13:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/12008 ) Change subject: IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/12008/1/docs/shared/impala_common.xml File docs/shared/impala_common.xml: http://gerrit.cloudera.org:8080/#/c/12008/1/docs/shared/impala_common.xml@3668 PS1, Line 3668: an Impala dynamic resource pool, you must also specify the Default Query > This looks like it needs updating since "Minimum Query Memory Limit" and "M This conref is not used anywhere. I will add a comment to remove it at some point http://gerrit.cloudera.org:8080/#/c/12008/1/docs/topics/impala_admission.xml File docs/topics/impala_admission.xml: http://gerrit.cloudera.org:8080/#/c/12008/1/docs/topics/impala_admission.xml@139 PS1, Line 139: This is the technique to use once you have a stable workload with well-understood memory requirements. > I feel like this sentence doesn't add anything. Removed http://gerrit.cloudera.org:8080/#/c/12008/1/docs/topics/impala_admission.xml@167 PS1, Line 167: e Default Query Memory Limit unset > I missed this in the draft I sent you - this note only applies if you set n Done http://gerrit.cloudera.org:8080/#/c/12008/1/docs/topics/impala_admission.xml@697 PS1, Line 697: In a real : deployment they might contain other settings for use with various : aspects of the YARN component. > I find this sentence confusing. Maybe the paragraph should just say that th Done -- To view, visit http://gerrit.cloudera.org:8080/12008 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2272433e8dc464c188e0c82a24bf3d0409aa05b Gerrit-Change-Number: 12008 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 01:06:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12008 ) Change subject: IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control .. Patch Set 2: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/163/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/12008 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2272433e8dc464c188e0c82a24bf3d0409aa05b Gerrit-Change-Number: 12008 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 01:06:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control
Hello Tim Armstrong, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12008 to look at the new patch set (#2). Change subject: IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control .. IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control Change-Id: Ie2272433e8dc464c188e0c82a24bf3d0409aa05b --- M docs/shared/impala_common.xml M docs/topics/impala_admission.xml M docs/topics/impala_resource_management.xml 3 files changed, 206 insertions(+), 200 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/12008/2 -- To view, visit http://gerrit.cloudera.org:8080/12008 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie2272433e8dc464c188e0c82a24bf3d0409aa05b Gerrit-Change-Number: 12008 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12005 ) Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands .. IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands JAVA_TOOL_OPTIONS is a variable used by JVM to specify the JVM options. Impala has several flags that specify shell commands for Impala to run, such as - s3a_access_key_cmd - s3a_secret_key_cmd - ssl_private_key_password_cmd - webserver_private_key_password_cmd When debugging the JVM inside the Impala process, it is useful to specify JAVA_TOOL_OPTIONS to run the Java debugger on a particular port. However, JAVA_TOOL_OPTIONS remains in the environment, so it is passed to these shell commands. If any of these shell commands run Java, then that JVM will attempt to use the JAVA_TOOL_OPTIONS specified and thus try to bind to the same port. The Impala process JVM is already bound to that port, so this will fail. This patch fixes the issue by unsetting JAVA_TOOL_OPTIONS before running these shell commands. Testing: - Added a new test for os-util.cc Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183 Reviewed-on: http://gerrit.cloudera.org:8080/12005 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/rpc/thrift-server.cc M be/src/runtime/hdfs-fs-cache.cc M be/src/util/CMakeLists.txt A be/src/util/os-util-test.cc M be/src/util/os-util.cc M be/src/util/os-util.h M be/src/util/webserver.cc 7 files changed, 86 insertions(+), 8 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183 Gerrit-Change-Number: 12005 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] Add a handy tool to collapse threads in pstacks
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11926 ) Change subject: Add a handy tool to collapse threads in pstacks .. Patch Set 3: Code-Review+2 Carrying +2. (RAT check passes locally on the last PS) -- To view, visit http://gerrit.cloudera.org:8080/11926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d00f7cea5fe01ff8b2511f4e04db5d5ad1c82f4 Gerrit-Change-Number: 11926 Gerrit-PatchSet: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 00:53:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12005 ) Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183 Gerrit-Change-Number: 12005 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Fri, 30 Nov 2018 01:00:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12000 ) Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates .. Patch Set 3: (5 comments) Apologies for being late, I only had some small comments. http://gerrit.cloudera.org:8080/#/c/12000/3/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/12000/3/be/src/runtime/coordinator-backend-state.cc@571 PS3, Line 571: UnixMillis() - last_report_time_ms_ Maybe clamp this at 0 to make sure we won't return a negative value if someone refreshes the page at the wrong time? http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@83 PS3, Line 83: get_thrift_profile get_thrift_profile takes a timeout, so you can call it once before the loop to wait until the profile shows up. http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@90 PS3, Line 90: 2: Can you make this a constant and add a comment. Otherwise this test will fail if someone adds a new profile node in front of them? Alternatively you could make the search a bit more generic and iterate over all nodes. http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@108 PS3, Line 108: # Minimum NTP poll period is 8 seconds. I found this part tricky to read. Why is it 8 seconds? Can you elaborate in the comment what the intent here is? http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@112 PS3, Line 112: self.client.fetch(query, handle) Should we just call close_query() here? -- To view, visit http://gerrit.cloudera.org:8080/12000 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa Gerrit-Change-Number: 12000 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Fri, 30 Nov 2018 00:58:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Add a handy tool to collapse threads in pstacks
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11926 ) Change subject: Add a handy tool to collapse threads in pstacks .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3508/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d00f7cea5fe01ff8b2511f4e04db5d5ad1c82f4 Gerrit-Change-Number: 11926 Gerrit-PatchSet: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 00:53:56 + Gerrit-HasComments: No
[Impala-ASF-CR] Add a handy tool to collapse threads in pstacks
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11926 ) Change subject: Add a handy tool to collapse threads in pstacks .. Patch Set 3: Looks like RAT doesn't work with non-Apache licenses. So I added it to the excludes file. We took a similar approach for another file with BSD 3-Clause license. I'm hoping that's fine. -- To view, visit http://gerrit.cloudera.org:8080/11926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d00f7cea5fe01ff8b2511f4e04db5d5ad1c82f4 Gerrit-Change-Number: 11926 Gerrit-PatchSet: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 00:51:51 + Gerrit-HasComments: No
[Impala-ASF-CR] Add a handy tool to collapse threads in pstacks
Hello Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11926 to look at the new patch set (#3). Change subject: Add a handy tool to collapse threads in pstacks .. Add a handy tool to collapse threads in pstacks This commit imports a very useful tool that summarizes/collapses threads from gdb pstacks. I found it very useful in analyzing pstacks with large number of threads, specially in Impala's context. Credit to the original author at https://poormansprofiler.org/ (Slightly modified for better input handling). Example Usages: --- summarize-pstacks foo.pstack summarize-pstacks foo_*.pstack cat foo.pstack | summarize-pstacks cat foo_*.pstack | summarize-pstacks Change-Id: I0d00f7cea5fe01ff8b2511f4e04db5d5ad1c82f4 --- M bin/rat_exclude_files.txt A bin/summarize-pstacks 2 files changed, 43 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/11926/3 -- To view, visit http://gerrit.cloudera.org:8080/11926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0d00f7cea5fe01ff8b2511f4e04db5d5ad1c82f4 Gerrit-Change-Number: 11926 Gerrit-PatchSet: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6964: Track stats about column and page sizes in Parquet reader
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/11575 )
Change subject: IMPALA-6964: Track stats about column and page sizes in Parquet
reader
..
Patch Set 14:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11575/12/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:
http://gerrit.cloudera.org:8080/#/c/11575/12/be/src/exec/hdfs-scan-node-base.cc@962
PS12, Line 962: if (bytes_read_itr != bytes_read_per_col_.end()) {
> boost:upgrade_lock doesn't really work for this use case. For upgrade_lock
Thanks for the explanation. I'm out of ideas how to get this to a more
RAII-style use of locks then. We could also just have a single lock around the
map. This would simplify the code and remove the need for the atomic. I don't
think that the lock would be very contended, given the operation is short and
the granularity is reasonably large. It also helps that pages are not
row-aligned.
I'm curious what others think, though. If you stick with the atomic, please use
our own AtomicInt from common/atomic.h.
--
To view, visit http://gerrit.cloudera.org:8080/11575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I322f9b324b6828df28e5caf79529085c43d7c817
Gerrit-Change-Number: 11575
Gerrit-PatchSet: 14
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Fri, 30 Nov 2018 00:34:06 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12008 ) Change subject: IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/12008/1/docs/shared/impala_common.xml File docs/shared/impala_common.xml: http://gerrit.cloudera.org:8080/#/c/12008/1/docs/shared/impala_common.xml@3668 PS1, Line 3668: an Impala dynamic resource pool, you must also specify the Default Query This looks like it needs updating since "Minimum Query Memory Limit" and "Maximum Query Memory Limit" is also an option. http://gerrit.cloudera.org:8080/#/c/12008/1/docs/topics/impala_admission.xml File docs/topics/impala_admission.xml: http://gerrit.cloudera.org:8080/#/c/12008/1/docs/topics/impala_admission.xml@139 PS1, Line 139: This is the technique to use once you have a stable workload with well-understood memory requirements. I feel like this sentence doesn't add anything. http://gerrit.cloudera.org:8080/#/c/12008/1/docs/topics/impala_admission.xml@167 PS1, Line 167: e Default Query Memory Limit unset I missed this in the draft I sent you - this note only applies if you set neither "Default..." or the Min/Max. http://gerrit.cloudera.org:8080/#/c/12008/1/docs/topics/impala_admission.xml@697 PS1, Line 697: In a real : deployment they might contain other settings for use with various : aspects of the YARN component. I find this sentence confusing. Maybe the paragraph should just say that these files define resource pools for Impala Admission Control and are separate from the similar fair-scheduler.xml that defines resource pools for YARN. -- To view, visit http://gerrit.cloudera.org:8080/12008 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2272433e8dc464c188e0c82a24bf3d0409aa05b Gerrit-Change-Number: 12008 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 00:31:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Add a handy tool to collapse threads in pstacks
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11926 ) Change subject: Add a handy tool to collapse threads in pstacks .. Patch Set 2: Code-Review+2 LGTM once you figure out the RAT issue. -- To view, visit http://gerrit.cloudera.org:8080/11926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d00f7cea5fe01ff8b2511f4e04db5d5ad1c82f4 Gerrit-Change-Number: 11926 Gerrit-PatchSet: 2 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 00:25:19 + Gerrit-HasComments: No
[Impala-ASF-CR] Add a handy tool to collapse threads in pstacks
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11926 ) Change subject: Add a handy tool to collapse threads in pstacks .. Patch Set 2: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/1476/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/11926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d00f7cea5fe01ff8b2511f4e04db5d5ad1c82f4 Gerrit-Change-Number: 11926 Gerrit-PatchSet: 2 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 00:17:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7867 (Part 2): ArrayList cleanup in analyzer
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11995 ) Change subject: IMPALA-7867 (Part 2): ArrayList cleanup in analyzer .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1475/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c0b5f40a0504fc2d324055bd2a962e35f8e744a Gerrit-Change-Number: 11995 Gerrit-PatchSet: 3 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Fri, 30 Nov 2018 00:08:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7867 (Part 2): ArrayList cleanup in analyzer
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11995 ) Change subject: IMPALA-7867 (Part 2): ArrayList cleanup in analyzer .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1474/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c0b5f40a0504fc2d324055bd2a962e35f8e744a Gerrit-Change-Number: 11995 Gerrit-PatchSet: 2 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Fri, 30 Nov 2018 00:00:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12008 ) Change subject: IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control .. Patch Set 1: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/162/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/12008 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2272433e8dc464c188e0c82a24bf3d0409aa05b Gerrit-Change-Number: 12008 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 29 Nov 2018 23:47:44 + Gerrit-HasComments: No
[Impala-ASF-CR] Add a handy tool to collapse threads in pstacks
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11926 ) Change subject: Add a handy tool to collapse threads in pstacks .. Patch Set 2: Reached out to Mark Callaghan via email. He was generous enough to give this script a 2-clause BSD license which is compatible with Apache 2.0. I retained the copyright accordingly. The original source code is at https://github.com/mdcallag/mytools/blob/master/scripts/pmp.sh -- To view, visit http://gerrit.cloudera.org:8080/11926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d00f7cea5fe01ff8b2511f4e04db5d5ad1c82f4 Gerrit-Change-Number: 11926 Gerrit-PatchSet: 2 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 29 Nov 2018 23:39:12 + Gerrit-HasComments: No
[Impala-ASF-CR] Add a handy tool to collapse threads in pstacks
Hello Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11926 to look at the new patch set (#2). Change subject: Add a handy tool to collapse threads in pstacks .. Add a handy tool to collapse threads in pstacks This commit imports a very useful tool that summarizes/collapses threads from gdb pstacks. I found it very useful in analyzing pstacks with large number of threads, specially in Impala's context. Credit to the original author at https://poormansprofiler.org/ (Slightly modified for better input handling). Example Usages: --- summarize-pstacks foo.pstack summarize-pstacks foo_*.pstack cat foo.pstack | summarize-pstacks cat foo_*.pstack | summarize-pstacks Change-Id: I0d00f7cea5fe01ff8b2511f4e04db5d5ad1c82f4 --- A bin/summarize-pstacks 1 file changed, 40 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/11926/2 -- To view, visit http://gerrit.cloudera.org:8080/11926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0d00f7cea5fe01ff8b2511f4e04db5d5ad1c82f4 Gerrit-Change-Number: 11926 Gerrit-PatchSet: 2 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7867 (Part 2): ArrayList cleanup in analyzer
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/11995 ) Change subject: IMPALA-7867 (Part 2): ArrayList cleanup in analyzer .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11995/2/fe/src/main/java/org/apache/impala/analysis/CreateUdfStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateUdfStmt.java: http://gerrit.cloudera.org:8080/#/c/11995/2/fe/src/main/java/org/apache/impala/analysis/CreateUdfStmt.java@133 PS2, Line 133: protected Function createFunction(FunctionName fnName, List argTypes, > line too long (91 > 90) Done -- To view, visit http://gerrit.cloudera.org:8080/11995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c0b5f40a0504fc2d324055bd2a962e35f8e744a Gerrit-Change-Number: 11995 Gerrit-PatchSet: 3 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Thu, 29 Nov 2018 23:18:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7867 (Part 2): ArrayList cleanup in analyzer
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/11995 ) Change subject: IMPALA-7867 (Part 2): ArrayList cleanup in analyzer .. Patch Set 2: (2 comments) Thanks Fredy for the review. Went ahead and cleaned up the other items you suggested. Rebased on latest master. Please take another look at your convenience. Thanks! http://gerrit.cloudera.org:8080/#/c/11995/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11995/1//COMMIT_MSG@9 PS1, Line 9: Replaces all use of ArrayList : in variable and method declarations with the interface Li > Instead of just ArrayList, why don't to go further to clean up all other co Sure. More code to review, but does make sense. http://gerrit.cloudera.org:8080/#/c/11995/1/fe/src/main/java/org/apache/impala/analysis/CollectionStructType.java File fe/src/main/java/org/apache/impala/analysis/CollectionStructType.java: http://gerrit.cloudera.org:8080/#/c/11995/1/fe/src/main/java/org/apache/impala/analysis/CollectionStructType.java@22 PS1, Line 22: import org.apache.impala.catalog.Arr > This is probably unintentional and should use Guava Preconditions instead. Done -- To view, visit http://gerrit.cloudera.org:8080/11995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c0b5f40a0504fc2d324055bd2a962e35f8e744a Gerrit-Change-Number: 11995 Gerrit-PatchSet: 2 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Thu, 29 Nov 2018 23:18:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7867 (Part 2): ArrayList cleanup in analyzer
Hello Fredy Wijaya, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11995 to look at the new patch set (#3). Change subject: IMPALA-7867 (Part 2): ArrayList cleanup in analyzer .. IMPALA-7867 (Part 2): ArrayList cleanup in analyzer Follow-on to prior patch for this JIRA. Replaces all use of ArrayList in variable and method declarations with the interface List. Retains the use of ArrayList for list implementations (i.e. "new" statements.) Also replaces "List.newArrayList()" with the more modern "new ArrayList<>()". Cleaned up a few Map and Set declarations and added a few missing @Override annotations found during this process. Similar changes made for HashMap/newHashMap() and HashSet/newHashSet(). Where I noticed old-style new calls (List = new ArrayList()) these were replaced with diamond-notation: new ArrayList<>(). Tests: This is purely a source-level change; no functional change. Ran all client unit tests. Change-Id: I7c0b5f40a0504fc2d324055bd2a962e35f8e744a --- M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java M fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableAddReplaceColsStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetColumnStats.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java M fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java M fe/src/main/java/org/apache/impala/analysis/CollectionStructType.java M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java M fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateUdaStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateUdfStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java M fe/src/main/java/org/apache/impala/analysis/ParquetHelper.java M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/analysis/Path.java M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java M fe/src/main/java/org/apache/impala/analysis/StatementBase.java M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java M fe/src/main/java/org/apache/impala/analysis/Subquery.java M fe/src/main/java/org/apache/impala/analysis/TableDef.java M fe/src/main/java/org/apache/impala/analysis/TableRef.java M fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/analysis/TypeDef.java M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java M fe/src/main/java/org/apache/impala/catalog/AggregateFunction.java M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java 56 files changed, 390 insertions(+), 392 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/11995/3 -- To view,
[Impala-ASF-CR] IMPALA-7867 (Part 2): ArrayList cleanup in analyzer
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11995 ) Change subject: IMPALA-7867 (Part 2): ArrayList cleanup in analyzer .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11995/2/fe/src/main/java/org/apache/impala/analysis/CreateUdfStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateUdfStmt.java: http://gerrit.cloudera.org:8080/#/c/11995/2/fe/src/main/java/org/apache/impala/analysis/CreateUdfStmt.java@133 PS2, Line 133: protected Function createFunction(FunctionName fnName, List argTypes, Type retType, line too long (91 > 90) -- To view, visit http://gerrit.cloudera.org:8080/11995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c0b5f40a0504fc2d324055bd2a962e35f8e744a Gerrit-Change-Number: 11995 Gerrit-PatchSet: 2 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 29 Nov 2018 23:16:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7867 (Part 2): ArrayList cleanup in analyzer
Hello Fredy Wijaya, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11995 to look at the new patch set (#2). Change subject: IMPALA-7867 (Part 2): ArrayList cleanup in analyzer .. IMPALA-7867 (Part 2): ArrayList cleanup in analyzer Follow-on to prior patch for this JIRA. Replaces all use of ArrayList in variable and method declarations with the interface List. Retains the use of ArrayList for list implementations (i.e. "new" statements.) Also replaces "List.newArrayList()" with the more modern "new ArrayList<>()". Cleaned up a few Map and Set declarations and added a few missing @Override annotations found during this process. Similar changes made for HashMap/newHashMap() and HashSet/newHashSet(). Where I noticed old-style new calls (List = new ArrayList()) these were replaced with diamond-notation: new ArrayList<>(). Tests: This is purely a source-level change; no functional change. Ran all client unit tests. Change-Id: I7c0b5f40a0504fc2d324055bd2a962e35f8e744a --- M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java M fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableAddReplaceColsStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetColumnStats.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java M fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java M fe/src/main/java/org/apache/impala/analysis/CollectionStructType.java M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java M fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateUdaStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateUdfStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java M fe/src/main/java/org/apache/impala/analysis/ParquetHelper.java M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/analysis/Path.java M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java M fe/src/main/java/org/apache/impala/analysis/StatementBase.java M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java M fe/src/main/java/org/apache/impala/analysis/Subquery.java M fe/src/main/java/org/apache/impala/analysis/TableDef.java M fe/src/main/java/org/apache/impala/analysis/TableRef.java M fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/analysis/TypeDef.java M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java M fe/src/main/java/org/apache/impala/catalog/AggregateFunction.java M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java 56 files changed, 389 insertions(+), 391 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/11995/2 -- To view,
[Impala-ASF-CR] IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12008 ) Change subject: IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control .. Patch Set 1: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/162/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/12008 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2272433e8dc464c188e0c82a24bf3d0409aa05b Gerrit-Change-Number: 12008 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 29 Nov 2018 23:14:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12008 Change subject: IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control .. IMPALA-7908: [DOCS] Doc automatic setting of memory limit for a query in admission control Change-Id: Ie2272433e8dc464c188e0c82a24bf3d0409aa05b --- M docs/shared/impala_common.xml M docs/topics/impala_admission.xml M docs/topics/impala_resource_management.xml 3 files changed, 204 insertions(+), 200 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/12008/1 -- To view, visit http://gerrit.cloudera.org:8080/12008 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie2272433e8dc464c188e0c82a24bf3d0409aa05b Gerrit-Change-Number: 12008 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12000 ) Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3507/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12000 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa Gerrit-Change-Number: 12000 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Thu, 29 Nov 2018 23:09:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12000 ) Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12000 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa Gerrit-Change-Number: 12000 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Thu, 29 Nov 2018 23:09:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6964: Track stats about column and page sizes in Parquet reader
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11575 ) Change subject: IMPALA-6964: Track stats about column and page sizes in Parquet reader .. Patch Set 14: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1473/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11575 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I322f9b324b6828df28e5caf79529085c43d7c817 Gerrit-Change-Number: 11575 Gerrit-PatchSet: 14 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 29 Nov 2018 22:22:38 + Gerrit-HasComments: No
[Impala-ASF-CR] Regenerate missing configuration files in run-all.sh
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12006 ) Change subject: Regenerate missing configuration files in run-all.sh .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12006 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9f5955574d304b343e904851cfb9081648e350f8 Gerrit-Change-Number: 12006 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 29 Nov 2018 22:08:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6964: Track stats about column and page sizes in Parquet reader
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11575 ) Change subject: IMPALA-6964: Track stats about column and page sizes in Parquet reader .. Patch Set 13: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1472/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11575 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I322f9b324b6828df28e5caf79529085c43d7c817 Gerrit-Change-Number: 11575 Gerrit-PatchSet: 13 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 29 Nov 2018 22:14:00 + Gerrit-HasComments: No
[Impala-ASF-CR] Regenerate missing configuration files in run-all.sh
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12006 ) Change subject: Regenerate missing configuration files in run-all.sh .. Regenerate missing configuration files in run-all.sh When bin/clean.sh runs, it removes some configuration files in fe/src/test/resources that are required for the minicluster to function. This changes testdata/bin/run-all.sh to detect when these configurations are missing and regenerate them. It detects issues due to running clean.sh, which is the most common case. It is not intended to fix anything else. Change-Id: I9f5955574d304b343e904851cfb9081648e350f8 Reviewed-on: http://gerrit.cloudera.org:8080/12006 Reviewed-by: Philip Zeyliger Tested-by: Impala Public Jenkins --- M testdata/bin/run-all.sh 1 file changed, 10 insertions(+), 0 deletions(-) Approvals: Philip Zeyliger: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/12006 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9f5955574d304b343e904851cfb9081648e350f8 Gerrit-Change-Number: 12006 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7804: Mitigate s3 consistency issues for test scanners
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/11959 ) Change subject: IMPALA-7804: Mitigate s3 consistency issues for test_scanners .. Patch Set 4: Code-Review+2 Rebased, carry +2 -- To view, visit http://gerrit.cloudera.org:8080/11959 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id042496beabe0d0226b347e0653b820fee369f4e Gerrit-Change-Number: 11959 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 29 Nov 2018 21:54:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7804: Mitigate s3 consistency issues for test scanners
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11959 ) Change subject: IMPALA-7804: Mitigate s3 consistency issues for test_scanners .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3506/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11959 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id042496beabe0d0226b347e0653b820fee369f4e Gerrit-Change-Number: 11959 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 29 Nov 2018 21:55:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6964: Track stats about column and page sizes in Parquet reader
Sahil Takiar has posted comments on this change. (
http://gerrit.cloudera.org:8080/11575 )
Change subject: IMPALA-6964: Track stats about column and page sizes in Parquet
reader
..
Patch Set 14:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/11575/12/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:
http://gerrit.cloudera.org:8080/#/c/11575/12/be/src/exec/hdfs-scan-node-base.cc@904
PS12, Line 904: boost::shared_lock
bytes_read_per_col_guard_read_lock(
> Please wrap the lock in a scope block to release it after the for() stateme
Done
http://gerrit.cloudera.org:8080/#/c/11575/12/be/src/exec/hdfs-scan-node-base.cc@962
PS12, Line 962: if (bytes_read_itr != bytes_read_per_col_.end()) {
> There's a race between unlocking and re-locking here. You can use boost:upg
boost:upgrade_lock doesn't really work for this use case. For upgrade_lock to
work, you first have to obtain an upgradeable lock. Only a single thread can
acquire the upgrade lock at a time. So for this use case it would be equivalent
of just using a single exclusive lock.
There is a race condition, but it should be harmless. If the slot_id key, value
pair is created after the shared_lock is released, but before the
exclusive_lock is acquired, the code will just increment the the value of the
existing key, value pair.
The R/W locking mechanism is just meant to ensure that multiple threads don't
try to add a new key, value pair to the map at the same time.
Looking at the code a bit more though, I realized that the map should contain a
std::atomic, which I've added in the most recent patch.
http://gerrit.cloudera.org:8080/#/c/11575/12/tests/infra/test_utils.py
File tests/infra/test_utils.py:
http://gerrit.cloudera.org:8080/#/c/11575/12/tests/infra/test_utils.py@36
PS12, Line 36: def test_get_bytes_summary_stats_counter():
> Can you double-check that this actually gets executed? :)
Yeah it does. I double checked the Jenkins job and it runs the tests in this
class.
http://gerrit.cloudera.org:8080/#/c/11575/12/tests/infra/test_utils.py@37
PS12, Line 37: T
> nit: remove this whitespace
Done
http://gerrit.cloudera.org:8080/#/c/11575/12/tests/infra/test_utils.py@40
PS12, Line 40: runtime_profile = "- ExampleCounter: (Avg: 8.00 KB (8192) ; " \
> maybe pick an example that's not all the same values?
Done
--
To view, visit http://gerrit.cloudera.org:8080/11575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I322f9b324b6828df28e5caf79529085c43d7c817
Gerrit-Change-Number: 11575
Gerrit-PatchSet: 14
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Thu, 29 Nov 2018 21:42:10 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6964: Track stats about column and page sizes in Parquet reader
Hello Lars Volker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11575 to look at the new patch set (#14). Change subject: IMPALA-6964: Track stats about column and page sizes in Parquet reader .. IMPALA-6964: Track stats about column and page sizes in Parquet reader Adds the following new stats: * ParquetCompressedPageSize - a summary (average, min, max) counter that tracks the size of compressed pages read, if no compressed pages are read then this counter is empty * ParquetUncompressedPageSize - a summary counter that tracks the size of uncompressed pages read, it is updated in two places: (1) when a compressed page is de-compressed, and (2) when a page that is not compressed is read * ParquetCompressedDataReadPerColumn - a summary counter that tracks the amount of compressed data read per column for a scan node * ParquetUncompressedDataReadPerColumn - a summary counter that tracks the amount of uncompressed data read per column for a scan node The PerColumn counters are calculated by aggregating the number of bytes read for each column across all scan ranges processed by a scan node. Each sample in the counter is the size of a single column. Here is an example of what the updated HDFS scan profile looks like: - ParquetCompressedDataReadPerColumn: (Avg: 227.56 KB (233018) ; Min: 225.14 KB (230540) ; Max: 229.98 KB (235496) ; Number of samples: 2) - ParquetUncompressedDataReadPerColumn: (Avg: 227.96 KB (233426) ; Min: 224.91 KB (230306) ; Max: 231.00 KB (236547) ; Number of samples: 2) - ParquetCompressedPageSize: (Avg: 4.46 KB (4568) ; Min: 3.86 KB (3955) ; Max: 5.19 KB (5315) ; Number of samples: 102) - ParquetDecompressedPageSize: (Avg: 4.47 KB (4576) ; Min: 3.86 KB (3950) ; Max: 5.22 KB (5349) ; Number of samples: 102) Testing: * Added new tests to test_scanners.py that do some basic validation of the new counters above Change-Id: I322f9b324b6828df28e5caf79529085c43d7c817 --- M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/hdfs-scan-node-base.h M be/src/exec/parquet/hdfs-parquet-scanner.cc M be/src/exec/parquet/hdfs-parquet-scanner.h M be/src/exec/parquet/parquet-column-readers.cc M be/src/util/runtime-profile.cc M tests/infra/test_utils.py M tests/query_test/test_scanners.py M tests/util/parse_util.py 9 files changed, 266 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/11575/14 -- To view, visit http://gerrit.cloudera.org:8080/11575 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I322f9b324b6828df28e5caf79529085c43d7c817 Gerrit-Change-Number: 11575 Gerrit-PatchSet: 14 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-6964: Track stats about column and page sizes in Parquet reader
Hello Lars Volker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11575 to look at the new patch set (#13). Change subject: IMPALA-6964: Track stats about column and page sizes in Parquet reader .. IMPALA-6964: Track stats about column and page sizes in Parquet reader Adds the following new stats: * ParquetCompressedPageSize - a summary (average, min, max) counter that tracks the size of compressed pages read, if no compressed pages are read then this counter is empty * ParquetUncompressedPageSize - a summary counter that tracks the size of uncompressed pages read, it is updated in two places: (1) when a compressed page is de-compressed, and (2) when a page that is not compressed is read * ParquetCompressedDataReadPerColumn - a summary counter that tracks the amount of compressed data read per column for a scan node * ParquetUncompressedDataReadPerColumn - a summary counter that tracks the amount of uncompressed data read per column for a scan node The PerColumn counters are calculated by aggregating the number of bytes read for each column across all scan ranges processed by a scan node. Each sample in the counter is the size of a single column. Here is an example of what the updated HDFS scan profile looks like: - ParquetCompressedDataReadPerColumn: (Avg: 227.56 KB (233018) ; Min: 225.14 KB (230540) ; Max: 229.98 KB (235496) ; Number of samples: 2) - ParquetUncompressedDataReadPerColumn: (Avg: 227.96 KB (233426) ; Min: 224.91 KB (230306) ; Max: 231.00 KB (236547) ; Number of samples: 2) - ParquetCompressedPageSize: (Avg: 4.46 KB (4568) ; Min: 3.86 KB (3955) ; Max: 5.19 KB (5315) ; Number of samples: 102) - ParquetDecompressedPageSize: (Avg: 4.47 KB (4576) ; Min: 3.86 KB (3950) ; Max: 5.22 KB (5349) ; Number of samples: 102) Testing: * Added new tests to test_scanners.py that do some basic validation of the new counters above Change-Id: I322f9b324b6828df28e5caf79529085c43d7c817 --- M be/src/exec/hdfs-parquet-scanner.cc M be/src/exec/hdfs-parquet-scanner.h M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/hdfs-scan-node-base.h M be/src/exec/parquet-column-readers.cc M be/src/util/runtime-profile.cc M tests/infra/test_utils.py M tests/query_test/test_scanners.py M tests/util/parse_util.py 9 files changed, 260 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/11575/13 -- To view, visit http://gerrit.cloudera.org:8080/11575 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I322f9b324b6828df28e5caf79529085c43d7c817 Gerrit-Change-Number: 11575 Gerrit-PatchSet: 13 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12005 ) Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1471/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183 Gerrit-Change-Number: 12005 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 29 Nov 2018 21:16:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12005 ) Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183 Gerrit-Change-Number: 12005 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 29 Nov 2018 21:08:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12005 ) Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3505/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183 Gerrit-Change-Number: 12005 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 29 Nov 2018 21:08:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12005 ) Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183 Gerrit-Change-Number: 12005 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 29 Nov 2018 20:49:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/12005 )
Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell
commands
..
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12005/4/be/src/util/os-util.cc
File be/src/util/os-util.cc:
http://gerrit.cloudera.org:8080/#/c/12005/4/be/src/util/os-util.cc@100
PS4, Line 100: for (const auto& env: unset_environment) {
> unused?
Done
http://gerrit.cloudera.org:8080/#/c/12005/4/be/src/util/os-util.cc@103
PS4, Line 103: string new_cmd = Substitute("$0$1", unset_cmd, cmd);
> unused?
Done
--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Thu, 29 Nov 2018 20:42:09 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/12005 ) Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands .. IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands JAVA_TOOL_OPTIONS is a variable used by JVM to specify the JVM options. Impala has several flags that specify shell commands for Impala to run, such as - s3a_access_key_cmd - s3a_secret_key_cmd - ssl_private_key_password_cmd - webserver_private_key_password_cmd When debugging the JVM inside the Impala process, it is useful to specify JAVA_TOOL_OPTIONS to run the Java debugger on a particular port. However, JAVA_TOOL_OPTIONS remains in the environment, so it is passed to these shell commands. If any of these shell commands run Java, then that JVM will attempt to use the JAVA_TOOL_OPTIONS specified and thus try to bind to the same port. The Impala process JVM is already bound to that port, so this will fail. This patch fixes the issue by unsetting JAVA_TOOL_OPTIONS before running these shell commands. Testing: - Added a new test for os-util.cc Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183 --- M be/src/rpc/thrift-server.cc M be/src/runtime/hdfs-fs-cache.cc M be/src/util/CMakeLists.txt A be/src/util/os-util-test.cc M be/src/util/os-util.cc M be/src/util/os-util.h M be/src/util/webserver.cc 7 files changed, 86 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/12005/5 -- To view, visit http://gerrit.cloudera.org:8080/12005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183 Gerrit-Change-Number: 12005 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12005 ) Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/12005/4/be/src/util/os-util.cc File be/src/util/os-util.cc: http://gerrit.cloudera.org:8080/#/c/12005/4/be/src/util/os-util.cc@100 PS4, Line 100: int i = 0; unused? http://gerrit.cloudera.org:8080/#/c/12005/4/be/src/util/os-util.cc@103 PS4, Line 103: i++; unused? -- To view, visit http://gerrit.cloudera.org:8080/12005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183 Gerrit-Change-Number: 12005 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 29 Nov 2018 20:38:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12005 ) Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1470/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183 Gerrit-Change-Number: 12005 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 29 Nov 2018 20:36:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12004 ) Change subject: IMPALA-7889: Write new logical types in Parquet .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1469/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 29 Nov 2018 20:15:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12005 ) Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands .. Patch Set 4: Code-Review+1 (1 comment) Carry Phil's +1 http://gerrit.cloudera.org:8080/#/c/12005/3/be/src/util/os-util.cc File be/src/util/os-util.cc: http://gerrit.cloudera.org:8080/#/c/12005/3/be/src/util/os-util.cc@102 PS3, Line 102: unset_cmd += "unset " + env + "; "; > It's a nit, but you can just do: Good idea. Done. -- To view, visit http://gerrit.cloudera.org:8080/12005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183 Gerrit-Change-Number: 12005 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 29 Nov 2018 20:06:19 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands
Fredy Wijaya has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/12005 ) Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands .. IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands JAVA_TOOL_OPTIONS is a variable used by JVM to specify the JVM options. Impala has several flags that specify shell commands for Impala to run, such as - s3a_access_key_cmd - s3a_secret_key_cmd - ssl_private_key_password_cmd - webserver_private_key_password_cmd When debugging the JVM inside the Impala process, it is useful to specify JAVA_TOOL_OPTIONS to run the Java debugger on a particular port. However, JAVA_TOOL_OPTIONS remains in the environment, so it is passed to these shell commands. If any of these shell commands run Java, then that JVM will attempt to use the JAVA_TOOL_OPTIONS specified and thus try to bind to the same port. The Impala process JVM is already bound to that port, so this will fail. This patch fixes the issue by unsetting JAVA_TOOL_OPTIONS before running these shell commands. Testing: - Added a new test for os-util.cc Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183 --- M be/src/rpc/thrift-server.cc M be/src/runtime/hdfs-fs-cache.cc M be/src/util/CMakeLists.txt A be/src/util/os-util-test.cc M be/src/util/os-util.cc M be/src/util/os-util.h M be/src/util/webserver.cc 7 files changed, 88 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/12005/4 -- To view, visit http://gerrit.cloudera.org:8080/12005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183 Gerrit-Change-Number: 12005 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12004 to look at the new patch set (#5). Change subject: IMPALA-7889: Write new logical types in Parquet .. IMPALA-7889: Write new logical types in Parquet Fill the LogicalType field in Parquet schemas for columns that have an associated logical type. ConvertedType still has to be filled to remain compatible with older readers. Testing: - added new tests to check both logical and converted types to test_insert_parquet.py Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 --- M be/src/exec/parquet/hdfs-parquet-table-writer.cc M be/src/exec/parquet/parquet-metadata-utils.cc M be/src/exec/parquet/parquet-metadata-utils.h M tests/query_test/test_insert_parquet.py M tests/util/get_parquet_metadata.py 5 files changed, 162 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/12004/5 -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12004 ) Change subject: IMPALA-7889: Write new logical types in Parquet .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/12004/4/tests/query_test/test_insert_parquet.py File tests/query_test/test_insert_parquet.py: http://gerrit.cloudera.org:8080/#/c/12004/4/tests/query_test/test_insert_parquet.py@407 PS4, Line 407: Sorry, I thought that I have run this test, but this was completely wrong and should have failed. Patch set 5 fixed the test. -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 29 Nov 2018 19:47:06 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands
Philip Zeyliger has posted comments on this change. (
http://gerrit.cloudera.org:8080/12005 )
Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell
commands
..
Patch Set 3: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12005/3/be/src/util/os-util.cc
File be/src/util/os-util.cc:
http://gerrit.cloudera.org:8080/#/c/12005/3/be/src/util/os-util.cc@102
PS3, Line 102: unset_cmd += string(((i > 0) ? "; " : "")) + "unset " + env;
It's a nit, but you can just do:
for (...) {
unset_cmd += "unset " + env + ";"
}
and then you don't have to worry about the "i>0" check, which makes life a
little bit easier to think about. Likewise, ignore the unset_environment.size()
check and just always do the substitution.
--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Thu, 29 Nov 2018 19:30:02 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates
Zoram Thanga has posted comments on this change. ( http://gerrit.cloudera.org:8080/12000 ) Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12000 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa Gerrit-Change-Number: 12000 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Thu, 29 Nov 2018 19:16:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7738: Implement timeouts for HDFS open calls
Philip Zeyliger has posted comments on this change. (
http://gerrit.cloudera.org:8080/11874 )
Change subject: IMPALA-7738: Implement timeouts for HDFS open calls
..
Patch Set 9:
(5 comments)
Looks generally good to me. Some minor comments.
http://gerrit.cloudera.org:8080/#/c/11874/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11874/9//COMMIT_MSG@28
PS9, Line 28: 2. Core tests
I'd like to see a test that calls "kill -STOP" to suspend the namenode and
trigger a timeout. Alternatively, we might be able to inject a sleep using
https://github.com/btraceio/btrace for the namenode, or something like that.
Or, conceptually, we could just add a sleep() right before the real hdfsOpen()
call that's controlled by one of our debug options... Anyway, it seems like we
should make sure that the error case of the hdfsOpen() timing out is covered.
http://gerrit.cloudera.org:8080/#/c/11874/9/be/src/runtime/io/hdfs-monitored-ops.h
File be/src/runtime/io/hdfs-monitored-ops.h:
http://gerrit.cloudera.org:8080/#/c/11874/9/be/src/runtime/io/hdfs-monitored-ops.h@50
PS9, Line 50: Status OpenHdfsFileWTimeout(const hdfsFS& fs, const
std::string* fname, int flags,
I see "With" spelled out in other cases. Not a big deal.
http://gerrit.cloudera.org:8080/#/c/11874/9/be/src/runtime/io/hdfs-monitored-ops.cc
File be/src/runtime/io/hdfs-monitored-ops.cc:
http://gerrit.cloudera.org:8080/#/c/11874/9/be/src/runtime/io/hdfs-monitored-ops.cc@48
PS9, Line 48: : fs_(fs), fname_(std::string(*fname)), flags_(flags),
blocksize_(blocksize) {}
Should this just be fname_(*fname) to invoke the copy constructor?
http://gerrit.cloudera.org:8080/#/c/11874/9/be/src/util/thread-pool.h
File be/src/util/thread-pool.h:
http://gerrit.cloudera.org:8080/#/c/11874/9/be/src/util/thread-pool.h@272
PS9, Line 272: /// Otherwise, it returns the status returned by ExecuteImpl().
s/ExecuteImpl/Execute/?
http://gerrit.cloudera.org:8080/#/c/11874/9/be/src/util/thread-pool.h@337
PS9, Line 337: LOG(WARNING) << timeout_status.GetDetail();
In practice, does this end up logging twice?
--
To view, visit http://gerrit.cloudera.org:8080/11874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia14403ca5f3f19c6d5f61b9ab2306b0ad3267454
Gerrit-Change-Number: 11874
Gerrit-PatchSet: 9
Gerrit-Owner: Joe McDonnell
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Thu, 29 Nov 2018 19:13:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1048: show sinks in exec summary
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11967 ) Change subject: IMPALA-1048: show sinks in exec summary .. Patch Set 8: Code-Review+1 I suppose it's safe to assume that no external software depends on the format of the exec summary, right ? I suppose not but just wanna double check. -- To view, visit http://gerrit.cloudera.org:8080/11967 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3fdf7bacae8ff597b255da65af453e174ba53544 Gerrit-Change-Number: 11967 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 29 Nov 2018 18:52:29 + Gerrit-HasComments: No
[Impala-ASF-CR] Regenerate missing configuration files in run-all.sh
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12006 ) Change subject: Regenerate missing configuration files in run-all.sh .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3504/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/12006 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9f5955574d304b343e904851cfb9081648e350f8 Gerrit-Change-Number: 12006 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 29 Nov 2018 18:14:32 + Gerrit-HasComments: No
[Impala-ASF-CR] Regenerate missing configuration files in run-all.sh
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12006 ) Change subject: Regenerate missing configuration files in run-all.sh .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12006 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9f5955574d304b343e904851cfb9081648e350f8 Gerrit-Change-Number: 12006 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 29 Nov 2018 18:13:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates
Balazs Jeszenszky has posted comments on this change. ( http://gerrit.cloudera.org:8080/12000 ) Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates .. Patch Set 2: Code-Review+1 LGTM -- To view, visit http://gerrit.cloudera.org:8080/12000 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa Gerrit-Change-Number: 12000 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Thu, 29 Nov 2018 12:52:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12004 ) Change subject: IMPALA-7889: Write new logical types in Parquet .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1468/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 4 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 29 Nov 2018 12:28:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12004 ) Change subject: IMPALA-7889: Write new logical types in Parquet .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/12004/2/tests/query_test/test_insert_parquet.py File tests/query_test/test_insert_parquet.py: http://gerrit.cloudera.org:8080/#/c/12004/2/tests/query_test/test_insert_parquet.py@776 PS2, Line 776: > flake8: W292 no newline at end of file Done http://gerrit.cloudera.org:8080/#/c/12004/2/tests/util/get_parquet_metadata.py File tests/util/get_parquet_metadata.py: http://gerrit.cloudera.org:8080/#/c/12004/2/tests/util/get_parquet_metadata.py@186 PS2, Line 186: > flake8: W292 no newline at end of file Done -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 2 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 29 Nov 2018 11:54:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12004 to look at the new patch set (#4). Change subject: IMPALA-7889: Write new logical types in Parquet .. IMPALA-7889: Write new logical types in Parquet Fill the LogicalType field in Parquet schemas for columns that have an associated logical type. ConvertedType still has to be filled to remain compatible with older readers. Testing: - added new tests to check both logical and converted types to test_insert_parquet.py Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 --- M be/src/exec/parquet/hdfs-parquet-table-writer.cc M be/src/exec/parquet/parquet-metadata-utils.cc M be/src/exec/parquet/parquet-metadata-utils.h M tests/query_test/test_insert_parquet.py M tests/util/get_parquet_metadata.py 5 files changed, 159 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/12004/4 -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 4 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7902: NumericLiteral fixes, refactoring
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12001 ) Change subject: IMPALA-7902: NumericLiteral fixes, refactoring .. Patch Set 2: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/1467/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/12001 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I484600747b2871d3a6fe9153751973af9a8534f2 Gerrit-Change-Number: 12001 Gerrit-PatchSet: 2 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 29 Nov 2018 08:53:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7902: NumericLiteral fixes, refactoring
Paul Rogers has abandoned this change. ( http://gerrit.cloudera.org:8080/12007 ) Change subject: IMPALA-7902: NumericLiteral fixes, refactoring .. Abandoned Wrong branch -- To view, visit http://gerrit.cloudera.org:8080/12007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I75836774271058a047e8bef3ac8ef9f2b96ee511 Gerrit-Change-Number: 12007 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7902: NumericLiteral fixes, refactoring
Paul Rogers has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/12001 ) Change subject: IMPALA-7902: NumericLiteral fixes, refactoring .. IMPALA-7902: NumericLiteral fixes, refactoring The work to clean up the rewriter logic must start with a stable AST, which must start with sprucing up some issues with the leaf nodes. This CR tackles the NumericLiteral used to hold numbers. IMPALA-7896: Literals should not need explicit analyze step Partial fix: removes the need to analyze a numeric literal: analyze() is a no-op. This eliminates the need to do a "fake" analysis with a null analyzer: numeric literals are now created analyzed. This is useful because the catalog module creates numeric literals outside of a query (and outside of an analyzer.) Since a literal is (mostly) immutable (except for type), the constructor now sets the type and cost, then marks the node as analyzed. A later call to analyze() has nothing to do. Code that created and dummy-analyzed numeric literals changed to use static create() methods resulting in simpler literal creation. IMPALA-7886: NumericLiteral constructor fails to round values to Decimal type IMPALA-7887: NumericLiteral fails to detect numeric overflow IMPALA-7888: Incorrect NumericLiteral overflow checks for FLOAT, DOUBLE IMPALA-7891: Analyzer does not detect numeric overflow in CAST IMPALA-7894: Parser does not catch double overflow These are all caused by the somewhat messy state of the numeric check code after years of incremental changes. This patch centralizes all checks into a series of constants and methods so that they are uniform. All values are set in constructors which now all uniformly check that the value is legal for the type. (Explicit) cast operations verify that the cast is valid. Multiple semi-parallel versions of the same logic is replaced by calls to a single implementation. The numeric checks now follow the SQL standard which says that implementations should fail if a cast would trucate most significant digits, but round when truncating the least significant. In this commit, the rules apply only to numeric literals created outside of the parser. The existing analysis rules are unchanged. IMPALA-7865: Repeated type widening of arithmetic expressions Partial fix. Replaces the "is explicit cast" flag in the numeric literal with the explicit type. This allows reseting an implicit type back to the explciit type if an arithmetic expression is analyzed multiple times. A later patch will feed this type information into the type inference mechanism to complete the fix. Finally, adds a set of new exceptions that begin to unify error reporting. These handle casts (SqlCastException), value validation (InvalidValueException) and unsupported features (UnsupportedFeatureException.) These all derive from AnalysisException for backward compatibility. Tests use the new exceptions to check for expected errors rather than parsing text strings (which tend to change.) Testing: * Added unit tests just for numeric literals. Refactored code to simplify the tests. * Added a test case for the obscure case in Decimal V1 of an implicit cast overflow. * Ran all FE tests. Change-Id: I484600747b2871d3a6fe9153751973af9a8534f2 --- M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java M fe/src/main/java/org/apache/impala/catalog/ScalarType.java A fe/src/main/java/org/apache/impala/common/InvalidValueException.java A fe/src/main/java/org/apache/impala/common/SqlCastException.java A fe/src/main/java/org/apache/impala/common/UnsupportedFeatureException.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java A fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java M fe/src/test/java/org/apache/impala/catalog/HdfsPartitionTest.java 15 files changed, 1,063 insertions(+), 170 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/12001/2 -- To view, visit http://gerrit.cloudera.org:8080/12001 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I484600747b2871d3a6fe9153751973af9a8534f2 Gerrit-Change-Number: 12001 Gerrit-PatchSet: 2 Gerrit-Owner: Paul Rogers
[Impala-ASF-CR] IMPALA-7902: NumericLiteral fixes, refactoring
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/12001 )
Change subject: IMPALA-7902: NumericLiteral fixes, refactoring
..
Patch Set 2:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/12001/2/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
File fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java:
http://gerrit.cloudera.org:8080/#/c/12001/2/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java@668
PS2, Line 668:
paramExprs.add(NumericLiteral.create(window_.getRightBoundary().getOffsetValue(),
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/12001/2/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java
File fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java:
http://gerrit.cloudera.org:8080/#/c/12001/2/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@247
PS2, Line 247:
assertFalse(NumericLiteral.isOverflow(BigDecimal.valueOf(Float.MIN_VALUE),
Type.FLOAT));
line too long (92 > 90)
http://gerrit.cloudera.org:8080/#/c/12001/2/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@259
PS2, Line 259:
assertFalse(NumericLiteral.isOverflow(BigDecimal.valueOf(Double.MIN_VALUE),
Type.DOUBLE));
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/12001/2/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@269
PS2, Line 269: assertFalse(NumericLiteral.isOverflow(new
BigDecimal(genDecimal(10,5)), Type.DECIMAL));
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/12001/2/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@270
PS2, Line 270: assertFalse(NumericLiteral.isOverflow(new
BigDecimal(genDecimal(10,5)), Type.DECIMAL));
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/12001/2/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@271
PS2, Line 271: assertFalse(NumericLiteral.isOverflow(new
BigDecimal(genDecimal(ScalarType.MAX_PRECISION, 0)), Type.DECIMAL));
line too long (114 > 90)
http://gerrit.cloudera.org:8080/#/c/12001/2/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@272
PS2, Line 272: assertFalse(NumericLiteral.isOverflow(new
BigDecimal(genDecimal(0, ScalarType.MAX_PRECISION)), Type.DECIMAL));
line too long (114 > 90)
http://gerrit.cloudera.org:8080/#/c/12001/2/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@273
PS2, Line 273: assertTrue(NumericLiteral.isOverflow(new
BigDecimal(genDecimal(ScalarType.MAX_PRECISION + 1, 0)), Type.DECIMAL));
line too long (117 > 90)
http://gerrit.cloudera.org:8080/#/c/12001/2/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@274
PS2, Line 274: assertTrue(NumericLiteral.isOverflow(new
BigDecimal(genDecimal(ScalarType.MAX_PRECISION + 1, 1)), Type.DECIMAL));
line too long (117 > 90)
http://gerrit.cloudera.org:8080/#/c/12001/2/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@275
PS2, Line 275: assertTrue(NumericLiteral.isOverflow(new
BigDecimal(genDecimal(0, ScalarType.MAX_PRECISION + 1)), Type.DECIMAL));
line too long (117 > 90)
http://gerrit.cloudera.org:8080/#/c/12001/2/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@438
PS2, Line 438: result = NumericLiteral.convertValue(BigDecimal.ZERO,
ScalarType.createDecimalType(2, 2));
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/12001/2/fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java
File
fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java:
http://gerrit.cloudera.org:8080/#/c/12001/2/fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java@235
PS2, Line 235: Assert.assertTrue(e.getMessage().contains("Value 11.1
cannot be cast to type DECIMAL(1,0)"));
line too long (99 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/12001
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I484600747b2871d3a6fe9153751973af9a8534f2
Gerrit-Change-Number: 12001
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Thu, 29 Nov 2018 08:09:33 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7902: NumericLiteral fixes, refactoring
Paul Rogers has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12007 Change subject: IMPALA-7902: NumericLiteral fixes, refactoring .. IMPALA-7902: NumericLiteral fixes, refactoring The work to clean up the rewriter logic must start with a stable AST, which must start with sprucing up some issues with the leaf nodes. This CR tackles the NumericLiteral used to hold numbers. IMPALA-7896: Literals should not need explicit analyze step Partial fix: removes the need to analyze a numeric literal: analyze() is a no-op. This eliminates the need to do a "fake" analysis with a null analyzer: numeric literals are now created analyzed. This is useful because the catalog module creates numeric literals outside of a query (and outside of an analyzer.) Since a literal is (mostly) immutable (except for type), the constructor now sets the type and cost, then marks the node as analyzed. A later call to analyze() has nothing to do. Code that created and dummy-analyzed numeric literals changed to use static create() methods resulting in simpler literal creation. IMPALA-7886: NumericLiteral constructor fails to round values to Decimal type IMPALA-7887: NumericLiteral fails to detect numeric overflow IMPALA-7888: Incorrect NumericLiteral overflow checks for FLOAT, DOUBLE IMPALA-7891: Analyzer does not detect numeric overflow in CAST IMPALA-7894: Parser does not catch double overflow These are all caused by the somewhat messy state of the numeric check code after years of incremental changes. This patch centralizes all checks into a series of constants and methods so that they are uniform. All values are set in constructors which now all uniformly check that the value is legal for the type. (Explicit) cast operations verify that the cast is valid. Multiple semi-parallel versions of the same logic is replaced by calls to a single implementation. The numeric checks now follow the SQL standard which says that implementations should fail if a cast would trucate most significant digits, but round when truncating the least significant. In this commit, the rules apply only to numeric literals created outside of the parser. The existing analysis rules are unchanged. IMPALA-7865: Repeated type widening of arithmetic expressions Partial fix. Replaces the "is explicit cast" flag in the numeric literal with the explicit type. This allows reseting an implicit type back to the explciit type if an arithmetic expression is analyzed multiple times. A later patch will feed this type information into the type inference mechanism to complete the fix. Finally, adds a set of new exceptions that begin to unify error reporting. These handle casts (SqlCastException), value validation (InvalidValueException) and unsupported features (UnsupportedFeatureException.) These all derive from AnalysisException for backward compatibility. Tests use the new exceptions to check for expected errors rather than parsing text strings (which tend to change.) Testing: * Added unit tests just for numeric literals. Refactored code to simplify the tests. * Added a test case for the obscure case in Decimal V1 of an implicit cast overflow. * Ran all FE tests. Change-Id: I484600747b2871d3a6fe9153751973af9a8534f2 Fix Change-Id: I75836774271058a047e8bef3ac8ef9f2b96ee511 --- M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java M fe/src/main/java/org/apache/impala/catalog/ScalarType.java A fe/src/main/java/org/apache/impala/common/InvalidValueException.java A fe/src/main/java/org/apache/impala/common/SqlCastException.java A fe/src/main/java/org/apache/impala/common/UnsupportedFeatureException.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java A fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java M fe/src/test/java/org/apache/impala/catalog/HdfsPartitionTest.java 15 files changed, 1,063 insertions(+), 170 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/12007/1 -- To view, visit http://gerrit.cloudera.org:8080/12007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I75836774271058a047e8bef3ac8ef9f2b96ee511 Gerrit-Change-Number: 12007 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Rogers
[Impala-ASF-CR] IMPALA-7902: NumericLiteral fixes, refactoring
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/12007 )
Change subject: IMPALA-7902: NumericLiteral fixes, refactoring
..
Patch Set 1:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/12007/1/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
File fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java:
http://gerrit.cloudera.org:8080/#/c/12007/1/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java@668
PS1, Line 668:
paramExprs.add(NumericLiteral.create(window_.getRightBoundary().getOffsetValue(),
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/12007/1/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java
File fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java:
http://gerrit.cloudera.org:8080/#/c/12007/1/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@247
PS1, Line 247:
assertFalse(NumericLiteral.isOverflow(BigDecimal.valueOf(Float.MIN_VALUE),
Type.FLOAT));
line too long (92 > 90)
http://gerrit.cloudera.org:8080/#/c/12007/1/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@259
PS1, Line 259:
assertFalse(NumericLiteral.isOverflow(BigDecimal.valueOf(Double.MIN_VALUE),
Type.DOUBLE));
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/12007/1/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@269
PS1, Line 269: assertFalse(NumericLiteral.isOverflow(new
BigDecimal(genDecimal(10,5)), Type.DECIMAL));
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/12007/1/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@270
PS1, Line 270: assertFalse(NumericLiteral.isOverflow(new
BigDecimal(genDecimal(10,5)), Type.DECIMAL));
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/12007/1/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@271
PS1, Line 271: assertFalse(NumericLiteral.isOverflow(new
BigDecimal(genDecimal(ScalarType.MAX_PRECISION, 0)), Type.DECIMAL));
line too long (114 > 90)
http://gerrit.cloudera.org:8080/#/c/12007/1/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@272
PS1, Line 272: assertFalse(NumericLiteral.isOverflow(new
BigDecimal(genDecimal(0, ScalarType.MAX_PRECISION)), Type.DECIMAL));
line too long (114 > 90)
http://gerrit.cloudera.org:8080/#/c/12007/1/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@273
PS1, Line 273: assertTrue(NumericLiteral.isOverflow(new
BigDecimal(genDecimal(ScalarType.MAX_PRECISION + 1, 0)), Type.DECIMAL));
line too long (117 > 90)
http://gerrit.cloudera.org:8080/#/c/12007/1/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@274
PS1, Line 274: assertTrue(NumericLiteral.isOverflow(new
BigDecimal(genDecimal(ScalarType.MAX_PRECISION + 1, 1)), Type.DECIMAL));
line too long (117 > 90)
http://gerrit.cloudera.org:8080/#/c/12007/1/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@275
PS1, Line 275: assertTrue(NumericLiteral.isOverflow(new
BigDecimal(genDecimal(0, ScalarType.MAX_PRECISION + 1)), Type.DECIMAL));
line too long (117 > 90)
http://gerrit.cloudera.org:8080/#/c/12007/1/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java@438
PS1, Line 438: result = NumericLiteral.convertValue(BigDecimal.ZERO,
ScalarType.createDecimalType(2, 2));
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/12007/1/fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java
File
fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java:
http://gerrit.cloudera.org:8080/#/c/12007/1/fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java@235
PS1, Line 235: Assert.assertTrue(e.getMessage().contains("Value 11.1
cannot be cast to type DECIMAL(1,0)"));
line too long (99 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/12007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I75836774271058a047e8bef3ac8ef9f2b96ee511
Gerrit-Change-Number: 12007
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Thu, 29 Nov 2018 08:08:37 +
Gerrit-HasComments: Yes
