[Impala-ASF-CR] IMPALA-6137: fix text scanner split delim mem mgmt
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8438 Change subject: IMPALA-6137: fix text scanner split delim mem mgmt .. IMPALA-6137: fix text scanner split delim mem mgmt The bug was that the buffer pointed to by byte_buffer_ could be freed by ReleaseCompletedResources() before CheckForSplitDelimiter() was called. The simple fix is to copy out the single byte that is needed each time the buffer is filled. Testing: Ran exhaustive query tests under ASAN with --disable_mem_pools=true. Before the change test_text_split_delimiters reliably caused an ASAN failure when run with --disable_mem_pools=true. We should get this coverage automatically once the I/O mgr switches to using the buffer pool, which uses ASAN poisoning on freed buffers. Change-Id: Iddbb5cf6acc8f0b0e0b4c205c334f21e03d06f1c --- M be/src/exec/hdfs-text-scanner.cc M be/src/exec/hdfs-text-scanner.h 2 files changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/8438/1 -- To view, visit http://gerrit.cloudera.org:8080/8438 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iddbb5cf6acc8f0b0e0b4c205c334f21e03d06f1c Gerrit-Change-Number: 8438 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-4835: Part 1: simplify I/O mgr mem mgmt
Tim Armstrong has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/8414 ) Change subject: IMPALA-4835: Part 1: simplify I/O mgr mem mgmt .. IMPALA-4835: Part 1: simplify I/O mgr mem mgmt In preparation for switching the I/O mgr to the buffer pool, this removes and cleans up a lot of code so that the switchover patch starts from a cleaner slate. * Remove the free buffer cache (which will be replaced by buffer pool's own caching). * Make memory limit exceeded error checking synchronous (in anticipation of having to propagate buffer pool errors synchronously). * Misc other refactoring and simplification. This will probably regress performance somewhat because of the cache removal, so my plan is to merge it around the same time as switching the I/O mgr to allocate from the buffer pool. I'm keeping the patches separate to make reviewing easier. Change-Id: If5cb42437d11c13bc4a55c3ab426b66777332bd1 --- M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/hdfs-scan-node.cc M be/src/runtime/disk-io-mgr-reader-context.cc M be/src/runtime/disk-io-mgr-reader-context.h M be/src/runtime/disk-io-mgr-scan-range.cc M be/src/runtime/disk-io-mgr-stress.cc M be/src/runtime/disk-io-mgr-test.cc M be/src/runtime/disk-io-mgr.cc M be/src/runtime/disk-io-mgr.h M be/src/runtime/exec-env.cc M be/src/runtime/mem-tracker.h M be/src/runtime/test-env.cc M be/src/runtime/tmp-file-mgr-test.cc M be/src/util/impalad-metrics.cc M be/src/util/impalad-metrics.h 15 files changed, 216 insertions(+), 575 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/8414/4 -- To view, visit http://gerrit.cloudera.org:8080/8414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If5cb42437d11c13bc4a55c3ab426b66777332bd1 Gerrit-Change-Number: 8414 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-6121: remove I/O mgr request context cache
Hello Michael Ho, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8408 to look at the new patch set (#4). Change subject: IMPALA-6121: remove I/O mgr request context cache .. IMPALA-6121: remove I/O mgr request context cache This simplifies the lifecycle of the request contexts and eliminates some code. The comments claim that request context cache improves performance when allocating smallish the objects. But allocating from TCMalloc's thread caches should scale much better than a global object pool protected by a lock. I needed to move the definition to a non-internal header file so that it was visible to clients that manage it by unique_ptr. We also do not need to transfer the request contexts to the RuntimeState since I/O buffers do not leave scanners now. Testing: Ran exhaustive tests. Change-Id: I91414eceaa4938fccd74686fe6bebede6ef36108 --- M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/hdfs-scan-node-base.h M be/src/exec/hdfs-scan-node-mt.cc M be/src/exec/hdfs-scan-node.cc M be/src/runtime/disk-io-mgr-internal.h M be/src/runtime/disk-io-mgr-reader-context.cc A be/src/runtime/disk-io-mgr-reader-context.h M be/src/runtime/disk-io-mgr-stress.cc M be/src/runtime/disk-io-mgr-test.cc M be/src/runtime/disk-io-mgr.cc M be/src/runtime/disk-io-mgr.h M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/runtime/tmp-file-mgr-test.cc M be/src/runtime/tmp-file-mgr.cc M be/src/runtime/tmp-file-mgr.h 16 files changed, 614 insertions(+), 780 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/8408/4 -- To view, visit http://gerrit.cloudera.org:8080/8408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I91414eceaa4938fccd74686fe6bebede6ef36108 Gerrit-Change-Number: 8408 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6136: Part 1: Query duration should not be normally negative.
Zoram Thanga has posted comments on this change. ( http://gerrit.cloudera.org:8080/8430 ) Change subject: IMPALA-6136: Part 1: Query duration should not be normally negative. .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/8430/1/be/src/service/impala-http-handler.cc File be/src/service/impala-http-handler.cc: http://gerrit.cloudera.org:8080/#/c/8430/1/be/src/service/impala-http-handler.cc@306 PS1, Line 306: record.end_time_us > record.end_time_us > 0 Done http://gerrit.cloudera.org:8080/#/c/8430/1/be/src/service/impala-http-handler.cc@306 PS1, Line 306: ending_time_us > end_time_us Changed as suggested. -- To view, visit http://gerrit.cloudera.org:8080/8430 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6f971ebd5f0f00f3e38df0495692ffe9d52ef90 Gerrit-Change-Number: 8430 Gerrit-PatchSet: 1 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Wed, 01 Nov 2017 06:28:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6136: Part 1: Query duration should not be normally negative.
Hello Michael Ho, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8430 to look at the new patch set (#2). Change subject: IMPALA-6136: Part 1: Query duration should not be normally negative. .. IMPALA-6136: Part 1: Query duration should not be normally negative. The second commit for IMPALA-5599, 1640aa97, introduced a regression where calculating the duration of an in-progress query can result in negative values. This can happen for two reasons: 1. The value of ClientRequestState::end_time_us_ is not initialized in the constructor, and may have some random value until ClientRequestState::Done() is called. 2. If ClientRequestState::end_time_us_ happens to have a positive value less than ClientRequestState::start_time_us_, the query duration will be negative. Here, since the query is still in flight, we need to use the local current time as the end time. The fix has been manually verified by executing long-running queries and checking the query profiles to ensure the durations are not some random junks. A new test case will be added to check for sane time values in a follow-on commit. Since we are using Unix time (system clock) for time stamps, it is still possible for end_time_us_ to be less than start_time_us_ if the system clock gets reset while the query is executing. If there are clients that assume that a query duration is never negative, we really should switch to a monotonic clock to time queries. Change-Id: Ib6f971ebd5f0f00f3e38df0495692ffe9d52ef90 --- M be/src/service/client-request-state.cc M be/src/service/impala-http-handler.cc 2 files changed, 7 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/8430/2 -- To view, visit http://gerrit.cloudera.org:8080/8430 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib6f971ebd5f0f00f3e38df0495692ffe9d52ef90 Gerrit-Change-Number: 8430 Gerrit-PatchSet: 2 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6136: Part 1: Query duration should not be normally negative.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/8430 ) Change subject: IMPALA-6136: Part 1: Query duration should not be normally negative. .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/8430/1/be/src/service/impala-http-handler.cc File be/src/service/impala-http-handler.cc: http://gerrit.cloudera.org:8080/#/c/8430/1/be/src/service/impala-http-handler.cc@306 PS1, Line 306: ending_time_us end_time_us http://gerrit.cloudera.org:8080/#/c/8430/1/be/src/service/impala-http-handler.cc@306 PS1, Line 306: record.end_time_us record.end_time_us > 0 -- To view, visit http://gerrit.cloudera.org:8080/8430 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6f971ebd5f0f00f3e38df0495692ffe9d52ef90 Gerrit-Change-Number: 8430 Gerrit-PatchSet: 1 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Wed, 01 Nov 2017 05:21:34 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5307: Part 2: copy out strings in uncompressed Avro
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8146 ) Change subject: IMPALA-5307: Part 2: copy out strings in uncompressed Avro .. Patch Set 18: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/8146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If1fc78790d778c874f5aafa5958c3c045a88d233 Gerrit-Change-Number: 8146 Gerrit-PatchSet: 18 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Nov 2017 04:26:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5307: Part 2: copy out strings in uncompressed Avro
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8146 ) Change subject: IMPALA-5307: Part 2: copy out strings in uncompressed Avro .. IMPALA-5307: Part 2: copy out strings in uncompressed Avro The approach is to re-materialize strings in those tuples that survive conjunct evaluation and may reference disk I/O buffers directly. This means that perf should not regress for the following cases: * Compressed Avro files. * Non-string columns. * Selective scans where the majority of tuples are filtered out. This approach will also work for the Sequence and Text scanners. Includes some improvements to Avro codegen to replace more constants to help win back some performance (with limited success): replaced InitTuple() with an optimised version and substituted tuple_byte_size() with a constant. Removes dead code for handling CHAR(n) - CHAR(n) is now always fixed length. Perf: Did microbenchmarks on uncompressed Avro files, one with all columns from lineitem and one with only l_comment. Tests were run with: set num_scanner_threads=1; I ran the query 5 times and extracted MaterializeTupleTime from the profile to measure CPU cost of materialization. Overall string materialization got significantly slower, mainly because of the extra memcpy() calls required. Selecting one string from a table with multiple columns: select min(l_comment) from biglineitem_avro 1.814 -> 2.096 Selecting one string from a table with one column: select min(l_comment) from biglineitem_comment; profile; 1.708 -> 3.7 Selecting one string from a table with one column with predicate: select min(l_comment) from biglineitem_comment where length(l_comment) > 1; 1.691 -> 1.449 Selecting all columns: select min(l_orderkey), min(l_partkey), min(l_suppkey), min(l_linenumber), min(l_quantity), min(l_extendedprice), min(l_discount), min(l_tax), min(l_returnflag), min(l_linestatus), min(l_shipdate), min(l_commitdate), min(l_receiptdate), min(l_shipinstruct), min(l_shipmode), min(l_comment) from biglineitem_avro; profile; 2.335 -> 3.711 Selecting an int column (no strings): select min(l_linenumber) from biglineitem_avro 1.806 -> 1.819 Testing: Ran exhaustive tests. Change-Id: If1fc78790d778c874f5aafa5958c3c045a88d233 Reviewed-on: http://gerrit.cloudera.org:8080/8146 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M be/src/codegen/gen_ir_descriptions.py M be/src/codegen/impala-ir.cc M be/src/codegen/llvm-codegen.cc M be/src/codegen/llvm-codegen.h M be/src/common/status.cc M be/src/common/status.h M be/src/exec/hdfs-avro-scanner-ir.cc M be/src/exec/hdfs-avro-scanner.cc M be/src/exec/hdfs-avro-scanner.h M be/src/exec/hdfs-scan-node-base.h M be/src/exec/hdfs-scanner.cc M be/src/exec/hdfs-scanner.h M be/src/exec/scan-node.h M be/src/runtime/CMakeLists.txt M be/src/runtime/descriptors.cc M be/src/runtime/descriptors.h M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h A be/src/runtime/tuple-ir.cc M be/src/runtime/tuple.cc M be/src/runtime/tuple.h M be/src/util/avro-util.h 22 files changed, 401 insertions(+), 114 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/8146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: If1fc78790d778c874f5aafa5958c3c045a88d233 Gerrit-Change-Number: 8146 Gerrit-PatchSet: 19 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/8034 ) Change subject: IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder .. Patch Set 14: (1 comment) http://gerrit.cloudera.org:8080/#/c/8034/12/be/src/util/dict-encoding.h File be/src/util/dict-encoding.h: http://gerrit.cloudera.org:8080/#/c/8034/12/be/src/util/dict-encoding.h@63 PS12, Line 63: /// dict_encoded_size() bytes. : virtual void Writ > implemented the logic of Close() inside ClearIndices() Close() should call ClearIndices() as one of its steps, ClearIndices() still needs to exist separately and cannot incorporate the logic of Close(). -- To view, visit http://gerrit.cloudera.org:8080/8034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02a3b54f6c107d19b62ad9e1c49df94175964299 Gerrit-Change-Number: 8034 Gerrit-PatchSet: 14 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Nov 2017 01:16:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder
Hello Joe McDonnell, Tim Armstrong, Bikramjeet Vig, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8034 to look at the new patch set (#14). Change subject: IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder .. IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder Currently DictDecoder class and DictEncoder class uses std::vector to store the tables mapping codeword to value and vice-versa. It is hard to detect the memory usage by these tables when they becomes very large, since this memory is not accounted by Impala's memory mangement infrastructure. This patch uses the memory tracker of HdfsScanner to track the memory used by dictionary in DictDecoder class. Similary it uses memory tracker of HdfsTableSink to track the memory used by dictionary in DictEncoder class. Memory for the dictionary, stored as std::vector is still allocated from std:allocator but the amount allocated is accounted by introducing a counter which is incremented and decremented as the memory is consumed and released by vector. Testing --- Ran all the backend and end-end tests with no failures. Change-Id: I02a3b54f6c107d19b62ad9e1c49df94175964299 --- M be/src/exec/hdfs-parquet-table-writer.cc M be/src/exec/parquet-column-readers.cc M be/src/exec/parquet-column-readers.h M be/src/util/dict-encoding.h M be/src/util/dict-test.cc 5 files changed, 134 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/8034/14 -- To view, visit http://gerrit.cloudera.org:8080/8034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I02a3b54f6c107d19b62ad9e1c49df94175964299 Gerrit-Change-Number: 8034 Gerrit-PatchSet: 14 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder
Hello Joe McDonnell, Tim Armstrong, Bikramjeet Vig, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8034 to look at the new patch set (#13). Change subject: IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder .. IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder Currently DictDecoder class and DictEncoder class uses std::vector to store the tables mapping codeword to value and vice-versa. It is hard to detect the memory usage by these tables when they becomes very large, since this memory is not accounted by Impala's memory mangement infrastructure. This patch uses the memory tracker of HdfsScanner to track the memory used by dictionary in DictDecoder class. Similary it uses memory tracker of HdfsTableSink to track the memory used by dictionary in DictEncoder class. Memory for the dictionary, stored as std::vector is still allocated from std:allocator but the amount allocated is accounted by introducing a counter which is incremented and decremented as the memory is consumed and released by vector. Testing --- Ran all the backend and end-end tests with no failures. Change-Id: I02a3b54f6c107d19b62ad9e1c49df94175964299 --- M be/src/exec/hdfs-parquet-table-writer.cc M be/src/exec/parquet-column-readers.cc M be/src/exec/parquet-column-readers.h M be/src/util/dict-encoding.h M be/src/util/dict-test.cc 5 files changed, 137 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/8034/13 -- To view, visit http://gerrit.cloudera.org:8080/8034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I02a3b54f6c107d19b62ad9e1c49df94175964299 Gerrit-Change-Number: 8034 Gerrit-PatchSet: 13 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder
Pranay Singh has posted comments on this change. (
http://gerrit.cloudera.org:8080/8034 )
Change subject: IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder
..
Patch Set 12:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/8034/12/be/src/exec/hdfs-parquet-scanner.h
File be/src/exec/hdfs-parquet-scanner.h:
http://gerrit.cloudera.org:8080/#/c/8034/12/be/src/exec/hdfs-parquet-scanner.h@352
PS12, Line 352: MemTracker* dict_mem_tracker() { return
scan_node_->mem_tracker(); }
> See comment in HdfsParquetTableWriter. Also, see how we pass the mem_tracke
Done
http://gerrit.cloudera.org:8080/#/c/8034/12/be/src/exec/hdfs-parquet-table-writer.h
File be/src/exec/hdfs-parquet-table-writer.h:
http://gerrit.cloudera.org:8080/#/c/8034/12/be/src/exec/hdfs-parquet-table-writer.h@81
PS12, Line 81: /// Memory tracker to track the memory used by encoder.
: MemTracker* dict_mem_tracker();
> When I'm reading the code, I'm thinking that this is hiding more than it is
Removed the function made it in line where the memtracker is used so it's more
explicit
http://gerrit.cloudera.org:8080/#/c/8034/12/be/src/exec/hdfs-parquet-table-writer.cc
File be/src/exec/hdfs-parquet-table-writer.cc:
http://gerrit.cloudera.org:8080/#/c/8034/12/be/src/exec/hdfs-parquet-table-writer.cc@179
PS12, Line 179: dict_encoder_base_->ClearIndices();
: dict_encoder_base_->Close();
> When Close() does a call to ClearIndices(), this can be simplified to just
implemented the Close() logic inside ClearIndices()
http://gerrit.cloudera.org:8080/#/c/8034/12/be/src/exec/hdfs-parquet-table-writer.cc@322
PS12, Line 322:plain_encoded_value_size_,
:parent_->dict_mem_tracker()));
> Nit: Indentation in this case should be even with the "D" in new DictEncode
aligned the indentation with D in next line.
http://gerrit.cloudera.org:8080/#/c/8034/12/be/src/util/dict-encoding.h
File be/src/util/dict-encoding.h:
http://gerrit.cloudera.org:8080/#/c/8034/12/be/src/util/dict-encoding.h@63
PS12, Line 63: void Close() {
: ReleaseBytes();
> I think Close() should do ClearIndices().
implemented the logic of Close() inside ClearIndices()
--
To view, visit http://gerrit.cloudera.org:8080/8034
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02a3b54f6c107d19b62ad9e1c49df94175964299
Gerrit-Change-Number: 8034
Gerrit-PatchSet: 12
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 01 Nov 2017 01:07:09 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction
Csaba Ringhofer has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8436 Change subject: IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction .. IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction During dictionary constructon, most types are copied from the parquet dictionary page, but StringValues keep pointers to it. In this case, the dictionary page must be kept and attached to the last row batch that references it. In case of other types, it is safe do delete the dictionary page after the construction of the dictionary. This patch contains two optimizations: - dictionary pages are deleted as soon as possible for non string types - in non-compressed and non-string case, an unnecessary copy is avoided Change-Id: I4d9d5f4da1028d961155dafdac0028a1c3641004 --- M be/src/exec/parquet-column-readers.cc M be/src/exec/parquet-column-readers.h 2 files changed, 42 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/8436/1 -- To view, visit http://gerrit.cloudera.org:8080/8436 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4d9d5f4da1028d961155dafdac0028a1c3641004 Gerrit-Change-Number: 8436 Gerrit-PatchSet: 1 Gerrit-Owner: Csaba Ringhofer
[Impala-ASF-CR] IMPALA-5307: Part 2: copy out strings in uncompressed Avro
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8146 ) Change subject: IMPALA-5307: Part 2: copy out strings in uncompressed Avro .. Patch Set 18: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If1fc78790d778c874f5aafa5958c3c045a88d233 Gerrit-Change-Number: 8146 Gerrit-PatchSet: 18 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Nov 2017 00:37:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5307: Part 2: copy out strings in uncompressed Avro
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8146 ) Change subject: IMPALA-5307: Part 2: copy out strings in uncompressed Avro .. Patch Set 17: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1421/ -- To view, visit http://gerrit.cloudera.org:8080/8146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If1fc78790d778c874f5aafa5958c3c045a88d233 Gerrit-Change-Number: 8146 Gerrit-PatchSet: 17 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Nov 2017 00:37:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5307: Part 2: copy out strings in uncompressed Avro
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8146 ) Change subject: IMPALA-5307: Part 2: copy out strings in uncompressed Avro .. Patch Set 18: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1422/ -- To view, visit http://gerrit.cloudera.org:8080/8146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If1fc78790d778c874f5aafa5958c3c045a88d233 Gerrit-Change-Number: 8146 Gerrit-PatchSet: 18 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Nov 2017 00:37:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5307: Part 2: copy out strings in uncompressed Avro
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8146 ) Change subject: IMPALA-5307: Part 2: copy out strings in uncompressed Avro .. Patch Set 17: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1421/ -- To view, visit http://gerrit.cloudera.org:8080/8146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If1fc78790d778c874f5aafa5958c3c045a88d233 Gerrit-Change-Number: 8146 Gerrit-PatchSet: 17 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Nov 2017 00:37:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4252: Min-max runtime filters for Kudu
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/7793 ) Change subject: IMPALA-4252: Min-max runtime filters for Kudu .. Patch Set 8: My +1 was for the backend, I can upgrade to a +2 for backend if nobody else wants to take a pass. I don't feel confident +1ing the frontend part. -- To view, visit http://gerrit.cloudera.org:8080/7793 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02bad890f5b5f78388a3041bf38f89369b5e2f1c Gerrit-Change-Number: 7793 Gerrit-PatchSet: 8 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Anonymous Coward #345 Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Nov 2017 00:36:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4252: Min-max runtime filters for Kudu
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/7793 ) Change subject: IMPALA-4252: Min-max runtime filters for Kudu .. Patch Set 8: Code-Review+1 (5 comments) http://gerrit.cloudera.org:8080/#/c/7793/8/be/src/runtime/timestamp-value.h File be/src/runtime/timestamp-value.h: http://gerrit.cloudera.org:8080/#/c/7793/8/be/src/runtime/timestamp-value.h@164 PS8, Line 164: uint8_min I'm confused by this variable name. http://gerrit.cloudera.org:8080/#/c/7793/8/be/src/util/min-max-filter-test.cc File be/src/util/min-max-filter-test.cc: http://gerrit.cloudera.org:8080/#/c/7793/8/be/src/util/min-max-filter-test.cc@27 PS8, Line 27: Can you maybe briefly describe what is tested for each filter type? There seems to be a pattern to the tests but a comment might make it easier to understand what coverage this provides. http://gerrit.cloudera.org:8080/#/c/7793/8/be/src/util/min-max-filter-test.cc@252 PS8, Line 252: ExecEnv* env = new ExecEnv(); Would TestEnv work? That is the semi-standard way to create an ExecEnv for tests. http://gerrit.cloudera.org:8080/#/c/7793/8/be/src/util/min-max-filter-test.cc@353 PS8, Line 353: //IMPALA_TEST_MAIN(); Remove? http://gerrit.cloudera.org:8080/#/c/7793/8/be/src/util/min-max-filter.cc File be/src/util/min-max-filter.cc: http://gerrit.cloudera.org:8080/#/c/7793/8/be/src/util/min-max-filter.cc@246 PS8, Line 246: NULL nullptr? -- To view, visit http://gerrit.cloudera.org:8080/7793 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02bad890f5b5f78388a3041bf38f89369b5e2f1c Gerrit-Change-Number: 7793 Gerrit-PatchSet: 8 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Anonymous Coward #345 Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Nov 2017 00:29:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2494: Support for byte array encoded decimals in Parquet scanner
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/7822 ) Change subject: IMPALA-2494: Support for byte array encoded decimals in Parquet scanner .. Patch Set 7: Code-Review+1 Looks good to me and it looks like you addressed Dan's comments. Dan, did you want to take another look? -- To view, visit http://gerrit.cloudera.org:8080/7822 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2c0e881045109f337fecba53fec21f9cfb9e619e Gerrit-Change-Number: 7822 Gerrit-PatchSet: 7 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Nov 2017 00:12:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5607: Add additional units to EXTRACT, DATE PART, TRUNC
Hello Greg Rahn, Jim Apple, Zach Amsden, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8311 to look at the new patch set (#6). Change subject: IMPALA-5607: Add additional units to EXTRACT, DATE_PART, TRUNC .. IMPALA-5607: Add additional units to EXTRACT, DATE_PART, TRUNC Return type of EXTRACT/DATE_PART is changed from INT to BIGINT because INT data type cannot cover NANOSECOND's value. * Add the following fields to EXTRACT and DATE_PART: WEEK DOW DOY SECOND/SECONDS MICROSECOND/MICROSECONDS NANOSECOND/NANOSECONDS * Add the following field to TRUNC: SS * Testing: Extend unit tests in expr-test Change-Id: If23ea310ddcc5c1c806b2b1ac24d24a4bd6aa4d9 --- M be/src/exprs/expr-test.cc M be/src/exprs/udf-builtins-ir.cc M be/src/exprs/udf-builtins.cc M be/src/exprs/udf-builtins.h M common/function-registry/impala_functions.py M common/thrift/Exprs.thrift 6 files changed, 211 insertions(+), 84 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/8311/6 -- To view, visit http://gerrit.cloudera.org:8080/8311 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If23ea310ddcc5c1c806b2b1ac24d24a4bd6aa4d9 Gerrit-Change-Number: 8311 Gerrit-PatchSet: 6 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-5564: Release lock during planning. (wip)
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/8434 ) Change subject: IMPALA-5564: Release lock during planning. (wip) .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/8434/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8434/1//COMMIT_MSG@17 PS1, Line 17: whereas the web UI would previously block on the lock. After IMPALA-1972, it doesn't block, it just returns an empty profile. http://gerrit.cloudera.org:8080/#/c/8434/1//COMMIT_MSG@26 PS1, Line 26: know the query id : that's necessary to call GetResultSetMetadata() It looks to me like this is against IMPALA-2568. I guess we eventually want to make ExecuteStatement() async, rather than blocking on planning/metadata load. We already hit that problem with clients like Hue timing out while loading metadata for large tables. If we eventually want to move in that direction, I think these changes need to be undone then. -- To view, visit http://gerrit.cloudera.org:8080/8434 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e3fc4c28d7a5ad8546d48bcd22c03fbce502e5b Gerrit-Change-Number: 8434 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 31 Oct 2017 23:42:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4835 (prep only): create io subfolder and namespace
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8424 ) Change subject: IMPALA-4835 (prep only): create io subfolder and namespace .. Patch Set 4: Do you have time to look at this Joe? I know you've done a bit of work in this code recently. There shouldn't be any behaviour changes as a result of this, just things being moved and renamed. -- To view, visit http://gerrit.cloudera.org:8080/8424 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If807f93a47d8027a43e56dd80b1b535d0bb74e1b Gerrit-Change-Number: 8424 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 23:26:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4835 (prep only): create io subfolder and namespace
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8424 to look at the new patch set (#4). Change subject: IMPALA-4835 (prep only): create io subfolder and namespace .. IMPALA-4835 (prep only): create io subfolder and namespace Instead of using the DiskIoMgr class as a namespace, which prevents forward-declaration of inner classes, create an impala::io namespace and unnested the inner class. This is done in anticipation of DiskIoMgr depending on BufferPool. This helps avoid a circular dependency between DiskIoMgr, TmpFileMgr and BufferPool headers that could not be broken with forward declarations. Testing: Ran core tests. Change-Id: If807f93a47d8027a43e56dd80b1b535d0bb74e1b --- M be/CMakeLists.txt M be/src/exec/base-sequence-scanner.cc 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/hdfs-scan-node-mt.h M be/src/exec/hdfs-scan-node.cc M be/src/exec/hdfs-scan-node.h M be/src/exec/hdfs-text-scanner.cc M be/src/exec/kudu-scan-node.cc M be/src/exec/scanner-context.cc M be/src/exec/scanner-context.h M be/src/runtime/CMakeLists.txt D be/src/runtime/disk-io-mgr.h M be/src/runtime/exec-env.cc M be/src/runtime/exec-env.h A be/src/runtime/io/CMakeLists.txt R be/src/runtime/io/disk-io-mgr-internal.h R be/src/runtime/io/disk-io-mgr-stress-test.cc R be/src/runtime/io/disk-io-mgr-stress.cc R be/src/runtime/io/disk-io-mgr-stress.h R be/src/runtime/io/disk-io-mgr-test.cc R be/src/runtime/io/disk-io-mgr.cc A be/src/runtime/io/disk-io-mgr.h R be/src/runtime/io/handle-cache.h R be/src/runtime/io/handle-cache.inline.h R be/src/runtime/io/request-context.cc R be/src/runtime/io/request-context.h A be/src/runtime/io/request-ranges.h R be/src/runtime/io/scan-range.cc M be/src/runtime/row-batch.h M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/runtime/test-env.h M be/src/runtime/tmp-file-mgr-test.cc M be/src/runtime/tmp-file-mgr.cc M be/src/runtime/tmp-file-mgr.h 38 files changed, 1,416 insertions(+), 1,310 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/8424/4 -- To view, visit http://gerrit.cloudera.org:8080/8424 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If807f93a47d8027a43e56dd80b1b535d0bb74e1b Gerrit-Change-Number: 8424 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-4964: Fix Decimal modulo overflow
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8329 ) Change subject: IMPALA-4964: Fix Decimal modulo overflow .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5420201d4440d421e33e443df005cdcc16b8a6cd Gerrit-Change-Number: 8329 Gerrit-PatchSet: 3 Gerrit-Owner: Taras Bobrovytsky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zach Amsden Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Tue, 31 Oct 2017 22:47:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5564: Release lock during planning. (wip)
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8434 ) Change subject: IMPALA-5564: Release lock during planning. (wip) .. Patch Set 1: Hi Dan, This is a draft of the change to let query profiles show up during planning. Feedback appreciated! Thanks! -- Philip -- To view, visit http://gerrit.cloudera.org:8080/8434 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e3fc4c28d7a5ad8546d48bcd22c03fbce502e5b Gerrit-Change-Number: 8434 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 31 Oct 2017 22:46:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5564: Release lock during planning. (wip)
Philip Zeyliger has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8434 Change subject: IMPALA-5564: Release lock during planning. (wip) .. IMPALA-5564: Release lock during planning. (wip) ** I'm looking for feedback on the approach here; if folks think this is the right direction, I'll work on chasing down why Thrift Exception messages aren't propagated and adding tests for any Thrift method that takes a query_id, across Beeswax and HS2 APIs. ** This commit changes the query path to release the client request state lock during planning. As a result, the plan becomes available to the web UI, whereas the web UI would previously block on the lock. This introduces a new boolean, is_planning_, to capture that that we're in planning. This is done largely to be able to assert that result metadata is not queried during planning. The common workflow here is: client, using Thrift, calls ImpalaServer::ExecuteStatement() (in impala-hs2-server.cc), which calls ImpalaServer::Execute(), which calls ImpalaServer::ExecuteInternal(), where the plannig happens. Because the first time the client can cleanly get the query id is the return of ExecuteStatement(), there shouldn't be a way to know the query id that's necessary to call GetResultSetMetadata(). If someone reaches around (e.g., via the web UI), they will get an error message. This happens to addess one of the points in IMPALA-3882, which talks about releasing this very lock. The core tests pass. One exhaustive test had to be modified, and I saw exhaustive failures that are unrelated. I added a new test for one Thrift Beeswax API whose behavior has changed. If folks think this approach is fine, I'll work through the other APIs. Change-Id: I1e3fc4c28d7a5ad8546d48bcd22c03fbce502e5b --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.cc M tests/common/impala_connection.py A tests/custom_cluster/test_profile_during_planning.py M tests/custom_cluster/test_query_concurrency.py 8 files changed, 147 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/8434/1 -- To view, visit http://gerrit.cloudera.org:8080/8434 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1e3fc4c28d7a5ad8546d48bcd22c03fbce502e5b Gerrit-Change-Number: 8434 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger
[Impala-ASF-CR] IMPALA-4252: Min-max runtime filters for Kudu
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/7793 ) Change subject: IMPALA-4252: Min-max runtime filters for Kudu .. Patch Set 8: I need to take another look at this - sorry for the delay. -- To view, visit http://gerrit.cloudera.org:8080/7793 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02bad890f5b5f78388a3041bf38f89369b5e2f1c Gerrit-Change-Number: 7793 Gerrit-PatchSet: 8 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Anonymous Coward #345 Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 31 Oct 2017 22:45:10 + Gerrit-HasComments: No
[Impala-ASF-CR] Correct log line in start-impala-cluster.py.
Philip Zeyliger has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8432 Change subject: Correct log line in start-impala-cluster.py. .. Correct log line in start-impala-cluster.py. Updated logging in start-impala-cluster to accurately specify how many impala nodes were started, and how many of these were coordinators or executors or both. The new logging looks like: Impala Cluster Running with 1 nodes (1 coordinators, 1 executors). Previously, when invoking this script with --cluster_size=1, it would report "1 nodes and 3 coordinators" which was wrong (because there was only 1 coordinator) and confusing (because it seemed like a coordinator was a separate thing from a node). I also removed an unused import. I have run core and exhaustive tests with these change, as part of testing other changes. Nothing untoward happened. Change-Id: I7ceece1c05b9a4ca9f0a08fa30d195f811490c0e --- M bin/start-impala-cluster.py 1 file changed, 8 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/32/8432/1 -- To view, visit http://gerrit.cloudera.org:8080/8432 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7ceece1c05b9a4ca9f0a08fa30d195f811490c0e Gerrit-Change-Number: 8432 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger
[Impala-ASF-CR] Install OpenJDK-dbg for development environments.
Philip Zeyliger has uploaded this change for review. (
http://gerrit.cloudera.org:8080/8431
Change subject: Install OpenJDK-dbg for development environments.
..
Install OpenJDK-dbg for development environments.
Updates the boostrap script to install the OpenJDK debug symbols.
OpenJDK comes with a gdb stack unwinder that errors out if the OpenJDK
debugging symbols aren't installed. This fixes the following error
message in gdb:
Installing openjdk unwinder
Traceback (most recent call last):
File
"/usr/share/gdb/auto-load/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server/libjvm.so-gdb.py",
line 52, in
class Types(object):
File
"/usr/share/gdb/auto-load/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server/libjvm.so-gdb.py",
line 66, in Types
nmethodp_t = gdb.lookup_type('nmethod').pointer()
gdb.error: No type named nmethod.
I tested this by installing the package manually on Ubuntu16.04 and
using gdb a bit.
Change-Id: I9aa3a5e1bbba55a4b0b489e4a4dfa39e35fc0ae0
---
M bin/bootstrap_system.sh
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/8431/1
--
To view, visit http://gerrit.cloudera.org:8080/8431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9aa3a5e1bbba55a4b0b489e4a4dfa39e35fc0ae0
Gerrit-Change-Number: 8431
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger
[Impala-ASF-CR] IMPALA-6127: Fix timeout in TestRuntimeFilter.test wait time
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8427 ) Change subject: IMPALA-6127: Fix timeout in TestRuntimeFilter.test_wait_time .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/8427 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee005bee8e0a535ce59d2e23e56be6004f2eb9de Gerrit-Change-Number: 8427 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 31 Oct 2017 22:02:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6127: Fix timeout in TestRuntimeFilter.test wait time
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8427 ) Change subject: IMPALA-6127: Fix timeout in TestRuntimeFilter.test_wait_time .. IMPALA-6127: Fix timeout in TestRuntimeFilter.test_wait_time test_wait_time has been flaky recently on ASAN due to hitting a timeout. The fix is to increase the timeout for ASAN builds. Change-Id: Iee005bee8e0a535ce59d2e23e56be6004f2eb9de Reviewed-on: http://gerrit.cloudera.org:8080/8427 Reviewed-by: Alex Behm Tested-by: Impala Public Jenkins --- M tests/query_test/test_runtime_filters.py 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Alex Behm: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/8427 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iee005bee8e0a535ce59d2e23e56be6004f2eb9de Gerrit-Change-Number: 8427 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-6136: Part 1: Query duration should not be normally negative.
Zoram Thanga has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8430 Change subject: IMPALA-6136: Part 1: Query duration should not be normally negative. .. IMPALA-6136: Part 1: Query duration should not be normally negative. The second commit for IMPALA-5599, 1640aa97, introduced a regression where calculating the duration of an in-progress query can result in negative values. This can happen for two reasons: 1. The value of ClientRequestState::end_time_us_ is not initialized in the constructor, and may have some random value until ClientRequestState::Done() is called. 2. If ClientRequestState::end_time_us_ happens to have a positive value less than ClientRequestState::start_time_us_, the query duration will be negative. Here, since the query is still in flight, we need to use the local current time as the end time. The fix has been manually verified by executing long-running queries and checking the query profiles to ensure the durations are not some random junks. A new test case will be added to check for sane time values in a follow-on commit. Since we are using Unix time (system clock) for time stamps, it is still possible for end_time_us_ to be less than start_time_us_ if the system clock gets reset while the query is executing. If there are clients that assume that a query duration is never negative, we really should switch to a monotonic clock to time queries. Change-Id: Ib6f971ebd5f0f00f3e38df0495692ffe9d52ef90 --- M be/src/service/client-request-state.cc M be/src/service/impala-http-handler.cc 2 files changed, 7 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/8430/1 -- To view, visit http://gerrit.cloudera.org:8080/8430 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib6f971ebd5f0f00f3e38df0495692ffe9d52ef90 Gerrit-Change-Number: 8430 Gerrit-PatchSet: 1 Gerrit-Owner: Zoram Thanga
[Impala-ASF-CR] IMPALA-5307: Part 4: copy out uncompressed text and seq
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8172 ) Change subject: IMPALA-5307: Part 4: copy out uncompressed text and seq .. Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-scan-node.cc@238 PS8, Line 238: > nit: a lot of unnecessary blank lines. maybe condense a bit so more code fi Done http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-sequence-scanner.cc File be/src/exec/hdfs-sequence-scanner.cc: http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-sequence-scanner.cc@329 PS8, Line 329: add_row = WriteCompleteTuple(row_batch->tuple_data_pool(), field_locations_.data(), > I missed updating this additional code path, we also need to copy out strin Done. The below test was previously failing but now succeeds. We should get better coverage of some of these things once we switch the I/O mgr to the buffer pool and have ASAN poisoning enabled for recycled buffers. ./buildall.sh -asan -skiptests -noclean -ninja -notests && start-impala-cluster.py --impalad_args=--disable_mem_pools=true && impala-py.test -n1 --verbose tests/query_test/test_scanners.py tests/query_test/test_aggregation.py --workload_exploration_strategy=functional-query:exhaustive -k seq http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-text-scanner.cc File be/src/exec/hdfs-text-scanner.cc: http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-text-scanner.cc@704 PS8, Line 704: bool split_delimiter_possible = context_->partition_descriptor()->line_delim() == '\n' > I think there's a latent bug here where we're touching byte_buffer_ *after* I filed IMPALA-6137. I think there are pre-existing bugs here so won't tackle them in this patch (this patch may have to wait for those though since it increase the odds of hitting them). -- To view, visit http://gerrit.cloudera.org:8080/8172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I304fd002b61bfedf41c8b1405cd7eb7b492bb941 Gerrit-Change-Number: 8172 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 21:48:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5307: Part 4: copy out uncompressed text and seq
Hello Michael Ho, Alex Behm, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8172 to look at the new patch set (#9). Change subject: IMPALA-5307: Part 4: copy out uncompressed text and seq .. IMPALA-5307: Part 4: copy out uncompressed text and seq This is the final patch for IMPALA-5307. Text and Seq scanners are converted to use the same approach as Avro. contains_tuple_data is now false so a bunch of dead code in ScannerContext can be removed. We also no longer attach I/O buffers to row batches so that logic can be removed. Testing: Ran core ASAN tests. Perf: I reran the same benchmarks as in Part 2. There was no measurable difference before and after - for both text and seq processing time is dominated by text parsing. Change-Id: I304fd002b61bfedf41c8b1405cd7eb7b492bb941 --- M be/src/codegen/gen_ir_descriptions.py M be/src/exec/base-sequence-scanner.cc M be/src/exec/exec-node.cc M be/src/exec/exec-node.h M be/src/exec/hdfs-avro-scanner-ir.cc M be/src/exec/hdfs-avro-scanner.cc M be/src/exec/hdfs-avro-scanner.h M be/src/exec/hdfs-parquet-scanner.cc M be/src/exec/hdfs-rcfile-scanner.cc M be/src/exec/hdfs-scan-node-base.h M be/src/exec/hdfs-scan-node-mt.cc M be/src/exec/hdfs-scan-node.cc M be/src/exec/hdfs-scanner-ir.cc M be/src/exec/hdfs-scanner.cc M be/src/exec/hdfs-scanner.h M be/src/exec/hdfs-sequence-scanner.cc M be/src/exec/hdfs-text-scanner.cc M be/src/exec/parquet-column-readers.cc M be/src/exec/scanner-context.cc M be/src/exec/scanner-context.h M be/src/runtime/row-batch.cc M be/src/runtime/row-batch.h 22 files changed, 131 insertions(+), 209 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/8172/9 -- To view, visit http://gerrit.cloudera.org:8080/8172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I304fd002b61bfedf41c8b1405cd7eb7b492bb941 Gerrit-Change-Number: 8172 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5307: Part 4: copy out uncompressed text and seq
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8172 ) Change subject: IMPALA-5307: Part 4: copy out uncompressed text and seq .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-sequence-scanner.cc File be/src/exec/hdfs-sequence-scanner.cc: http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-sequence-scanner.cc@329 PS8, Line 329: add_row = WriteCompleteTuple(row_batch->tuple_data_pool(), field_locations_.data(), I missed updating this additional code path, we also need to copy out strings here. -- To view, visit http://gerrit.cloudera.org:8080/8172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I304fd002b61bfedf41c8b1405cd7eb7b492bb941 Gerrit-Change-Number: 8172 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 21:40:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5638: [DOCS] Add known issue for Impala-Kudu-Sentry issue
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8421 ) Change subject: IMPALA-5638: [DOCS] Add known issue for Impala-Kudu-Sentry issue .. IMPALA-5638: [DOCS] Add known issue for Impala-Kudu-Sentry issue Change-Id: I93e99aec2fcbc12f94678e60ebb9d150e72fc77d Reviewed-on: http://gerrit.cloudera.org:8080/8421 Reviewed-by: Bharath Vissapragada Tested-by: Impala Public Jenkins --- M docs/topics/impala_known_issues.xml 1 file changed, 21 insertions(+), 0 deletions(-) Approvals: Bharath Vissapragada: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/8421 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I93e99aec2fcbc12f94678e60ebb9d150e72fc77d Gerrit-Change-Number: 8421 Gerrit-PatchSet: 2 Gerrit-Owner: John Russell Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: John Russell
[Impala-ASF-CR] IMPALA-5638: [DOCS] Add known issue for Impala-Kudu-Sentry issue
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8421 ) Change subject: IMPALA-5638: [DOCS] Add known issue for Impala-Kudu-Sentry issue .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/8421 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I93e99aec2fcbc12f94678e60ebb9d150e72fc77d Gerrit-Change-Number: 8421 Gerrit-PatchSet: 1 Gerrit-Owner: John Russell Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: John Russell Gerrit-Comment-Date: Tue, 31 Oct 2017 21:17:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3887: Use dfs.namenode.replication.min=3
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8426 ) Change subject: IMPALA-3887: Use dfs.namenode.replication.min=3 .. Patch Set 3: I am running into MAPREDUCE-6441 with this patch precisely during data load. -- To view, visit http://gerrit.cloudera.org:8080/8426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie09efdd9512cc4f0e338919af42bcdcede2cfb93 Gerrit-Change-Number: 8426 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 31 Oct 2017 21:15:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8412 ) Change subject: IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/8412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2cd434757de2cd384def5b360a479e51812a Gerrit-Change-Number: 8412 Gerrit-PatchSet: 5 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 21:14:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8412 ) Change subject: IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test .. IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test A recent patch for IMPALA-5129 introduced a use-after-free bug in thrift-server-test. It is fixed in this patch. Change-Id: I2cd434757de2cd384def5b360a479e51812a Reviewed-on: http://gerrit.cloudera.org:8080/8412 Reviewed-by: Sailesh Mukil Tested-by: Impala Public Jenkins --- M be/src/rpc/auth-provider.h M be/src/rpc/thrift-server-test.cc 2 files changed, 19 insertions(+), 3 deletions(-) Approvals: Sailesh Mukil: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/8412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I2cd434757de2cd384def5b360a479e51812a Gerrit-Change-Number: 8412 Gerrit-PatchSet: 6 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6080: clean up table descriptor handling
Tianyi Wang has posted comments on this change. (
http://gerrit.cloudera.org:8080/8330 )
Change subject: IMPALA-6080: clean up table descriptor handling
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8330/2/be/src/runtime/descriptors.h
File be/src/runtime/descriptors.h:
http://gerrit.cloudera.org:8080/#/c/8330/2/be/src/runtime/descriptors.h@79
PS2, Line 79: struct LlvmTupleStruct {
Is this struct used?
--
To view, visit http://gerrit.cloudera.org:8080/8330
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id427dab0c196b556bd8b2d64ec618403d5cbd4d6
Gerrit-Change-Number: 8330
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Tianyi Wang
Gerrit-Comment-Date: Tue, 31 Oct 2017 21:13:29 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5638: [DOCS] Add known issue for Impala-Kudu-Sentry issue
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8421 ) Change subject: IMPALA-5638: [DOCS] Add known issue for Impala-Kudu-Sentry issue .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-docs-submit/170/ -- To view, visit http://gerrit.cloudera.org:8080/8421 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I93e99aec2fcbc12f94678e60ebb9d150e72fc77d Gerrit-Change-Number: 8421 Gerrit-PatchSet: 1 Gerrit-Owner: John Russell Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: John Russell Gerrit-Comment-Date: Tue, 31 Oct 2017 21:10:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3887: Use dfs.namenode.replication.min=3
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8426 ) Change subject: IMPALA-3887: Use dfs.namenode.replication.min=3 .. Patch Set 3: > Patch Set 3: > > I'm still testing this change. With this change we seem to be more prone to > hitting the Hive/mapred local executor race (MAPREDUCE-6441 MAPREDUCE-6992). > Will sync with PhilZ to see how to proceed. https://gerrit.cloudera.org/c/8405/ is ready to commit. We were holding off because Lars said to hold off until the build stabilized. I don't know where you're running into MAPREDUCE-6441. My change only addresses it for data load, since it's an option you have to pass to beeline. c/8405 is splittable into two parts (beeline change for the option; parallelization), but I don't see much benefit into splitting it. -- To view, visit http://gerrit.cloudera.org:8080/8426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie09efdd9512cc4f0e338919af42bcdcede2cfb93 Gerrit-Change-Number: 8426 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 31 Oct 2017 20:38:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4856: Port data stream service to KRPC
Dan Hecht has posted comments on this change. (
http://gerrit.cloudera.org:8080/8023 )
Change subject: IMPALA-4856: Port data stream service to KRPC
..
Patch Set 6:
(37 comments)
Next batch.
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-mgr.h
File be/src/runtime/krpc-data-stream-mgr.h:
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-mgr.h@103
PS6, Line 103: processed
deserialized?
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-mgr.h@104
PS6, Line 104: respectively
what is this "respectively" referring to?
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-mgr.h@249
PS6, Line 249: proto batch
unclear what that means, maybe stale comment? And this should say something
about the row batch being contained in 'request'.
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-mgr.h@258
PS6, Line 258: the actual row batch
isn't that part of "memory pointed to by 'request'"? If so and you want to
explicitly mention row batch, maybe say "including the serialized row batch"?
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-mgr.h@294
PS6, Line 294: typedef std::unique_ptr DeserializeWorkItem;
How about getting rid of this typedef? The code seems easier to understand if
the unique_ptr is visible in the fn decls. it's a bit harder than necessary to
reasonable about DeserializeWorkItem& and &&, given that this is now directly a
unique_ptr.
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-mgr.h@428
PS6, Line 428: senders
a sender
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-recvr.h
File be/src/runtime/krpc-data-stream-recvr.h:
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-recvr.h@119
PS6, Line 119: 'row_batch'
row batch is deserialized
('row_batch' isn't a variable in this context)
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-sender.h
File be/src/runtime/krpc-data-stream-sender.h:
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-sender.h@57
PS6, Line 57: per_channel_buffer_size' is the buffer size in bytes allocated to
each channel's
: /// accumulated row batch.
still not clear what that means. This isn't really the size of a buffer, is it?
How about something like:
... is a soft limit on the buffering, in bytes, into the channel's accumulating
row batch before it will be sent.
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-sender.h@111
PS6, Line 111: cached protobuf
serialized
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-sender.h@130
PS6, Line 130: cached proto
outbound row
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-sender.h@133
PS6, Line 133: Two OutboundRowBatch reused across RPCs
Maybe say:
The outbound row batches are double-buffered.
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-sender.cc
File be/src/runtime/krpc-data-stream-sender.cc:
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-sender.cc@67
PS6, Line 67: can
it looks like there's a third interface now: SerializeAndSendBatch() that takes
an Impala rowbatch.
But now that we have the outbound batch on the sender, why not just use that
for the RANDOM case and do SerializeBatch() then TransmitData(), so that we can
simplify the abstraction?
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-sender.cc@81
PS6, Line 81: TearDown
Teardown()
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-sender.cc@82
PS6, Line 82: TearDown
Teardown()
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-sender.cc@126
PS6, Line 126: .
or if the preceding RPC failed.
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-sender.cc@141
PS6, Line 141: // Flushes any buffered row batches and sends the EOS RPC to
close the channel.
Returns error status if...
Also indicate that it blocks until the EOS RPC is complete.
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-sender.cc@157
PS6, Line 157: below
delete.
If that's all we need it for, maybe just remember the capacity?
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-sender.cc@166
PS6, Line 166: RuntimeState* runtime_state_ = nullptr;
is that actually used?
http://gerrit.cloudera.org:8080/#/c/8023/6/be/src/runtime/krpc-data-stream-sender.cc@176
PS6, Line 176: current_outbound_batch_
the name of this is confusing because it's so similar to current_batch_idx_,
but it means something different (and often the opposite). How about calling
this:
rpc_outbound_batch_ or rpc_in_flight_batch_?
You could even get rid of the r
[Impala-ASF-CR] IMPALA-4177,IMPALA-6039: batched bit reading and rle decoding
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8267 ) Change subject: IMPALA-4177,IMPALA-6039: batched bit reading and rle decoding .. Patch Set 12: > Dan, did you want to have a look or should I find someone else to > +2? I plan to at least look at the new class abstractions, but maybe not all the implementation details. -- To view, visit http://gerrit.cloudera.org:8080/8267 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35de0cf80c86f501c4a39270afc8fb8111552ac6 Gerrit-Change-Number: 8267 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 20:19:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4177,IMPALA-6039: batched bit reading and rle decoding
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8267 ) Change subject: IMPALA-4177,IMPALA-6039: batched bit reading and rle decoding .. Patch Set 12: Dan, did you want to have a look or should I find someone else to +2? -- To view, visit http://gerrit.cloudera.org:8080/8267 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35de0cf80c86f501c4a39270afc8fb8111552ac6 Gerrit-Change-Number: 8267 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 20:13:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8202 ) Change subject: IMPALA-4704: Turns on client connections when local catalog initialized. .. Patch Set 19: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/8202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6 Gerrit-Change-Number: 8202 Gerrit-PatchSet: 19 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Juan Yu Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 31 Oct 2017 19:06:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8202 ) Change subject: IMPALA-4704: Turns on client connections when local catalog initialized. .. Patch Set 16: (1 comment) http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1014 PS16, Line 1014: /// Flag that records if backend and/or client services have been started. > they're on back-to-back lines now in impala_server.cc so I think we're fine Ahh right, I wrote this comment before seeing the other file :). lgtm -- To view, visit http://gerrit.cloudera.org:8080/8202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6 Gerrit-Change-Number: 8202 Gerrit-PatchSet: 16 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Juan Yu Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 31 Oct 2017 19:06:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
Hello Juan Yu, Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris Tsirogiannis, Alex Behm, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8202 to look at the new patch set (#19). Change subject: IMPALA-4704: Turns on client connections when local catalog initialized. .. IMPALA-4704: Turns on client connections when local catalog initialized. Currently, impalad starts beeswax and hs2 servers even if the catalog has not yet been initialized. As a result, client connections see an error message stating that the impalad is not yet ready. This patch changes the impalad startup sequence to wait until the catalog is received before opening beeswax and hs2 ports and starting their servers. Testing: - python e2e tests that start a cluster without a catalog and check that client connections are rejected as expected. Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6 --- M be/src/benchmarks/expr-benchmark.cc M be/src/common/global-flags.cc M be/src/service/frontend.cc M be/src/service/frontend.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/impalad-main.cc M be/src/testutil/in-process-servers.cc M be/src/testutil/in-process-servers.h M bin/start-impala-cluster.py M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/service/FrontendTest.java M tests/common/custom_cluster_test_suite.py M tests/common/impala_cluster.py M tests/common/impala_service.py A tests/custom_cluster/test_catalog_wait.py M tests/custom_cluster/test_coordinators.py 18 files changed, 282 insertions(+), 144 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/19 -- To view, visit http://gerrit.cloudera.org:8080/8202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6 Gerrit-Change-Number: 8202 Gerrit-PatchSet: 19 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Juan Yu Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/8202 ) Change subject: IMPALA-4704: Turns on client connections when local catalog initialized. .. Patch Set 18: (2 comments) http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1014 PS16, Line 1014: /// an accurate expiration time, and this structure guarantees that we will always > Works for me. To make the relationship between 'services_started_' and 'is_ they're on back-to-back lines now in impala_server.cc so I think we're fine. if they drift to different locations, makes sense to add the dcheck. http://gerrit.cloudera.org:8080/#/c/8202/18/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/8202/18/be/src/service/impala-server.cc@2050 PS18, Line 2050: I > just set to true? Done -- To view, visit http://gerrit.cloudera.org:8080/8202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6 Gerrit-Change-Number: 8202 Gerrit-PatchSet: 18 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Juan Yu Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 31 Oct 2017 19:03:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8202 ) Change subject: IMPALA-4704: Turns on client connections when local catalog initialized. .. Patch Set 18: Code-Review+1 (3 comments) I'm happy with this change, but would be great to get another pair of eyes on it. Dan, can you take a look? http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@98 PS17, Line 98: ///- Start ImpalaInternalService API > currently, the main flips this bit-- its the one orchestrating the sequence sounds good http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1014 PS16, Line 1014: /// an accurate expiration time, and this structure guarantees that we will always > I think the thing I'm objecting to here is that the internal state will dep Works for me. To make the relationship between 'services_started_' and 'is_ready' clear, I think we should add a DCHECK right before setting 'is_ready' that asserts 'services_started_' must be true. http://gerrit.cloudera.org:8080/#/c/8202/18/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/8202/18/be/src/service/impala-server.cc@2050 PS18, Line 2050: I just set to true? -- To view, visit http://gerrit.cloudera.org:8080/8202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6 Gerrit-Change-Number: 8202 Gerrit-PatchSet: 18 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Juan Yu Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 31 Oct 2017 18:51:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3887: Use dfs.namenode.replication.min=3
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8426 ) Change subject: IMPALA-3887: Use dfs.namenode.replication.min=3 .. Patch Set 3: I'm still testing this change. With this change we seem to be more prone to hitting the Hive/mapred local executor race (MAPREDUCE-6441 MAPREDUCE-6992). Will sync with PhilZ to see how to proceed. -- To view, visit http://gerrit.cloudera.org:8080/8426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie09efdd9512cc4f0e338919af42bcdcede2cfb93 Gerrit-Change-Number: 8426 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Comment-Date: Tue, 31 Oct 2017 18:29:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3887: Use dfs.namenode.replication.min=3
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8426 ) Change subject: IMPALA-3887: Use dfs.namenode.replication.min=3 .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/8426/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8426/2//COMMIT_MSG@9 PS2, Line 9: and HBase configuration of our > Update? Done -- To view, visit http://gerrit.cloudera.org:8080/8426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie09efdd9512cc4f0e338919af42bcdcede2cfb93 Gerrit-Change-Number: 8426 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Comment-Date: Tue, 31 Oct 2017 18:26:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3887: Use dfs.namenode.replication.min=3
Hello Bharath Vissapragada, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8426 to look at the new patch set (#3). Change subject: IMPALA-3887: Use dfs.namenode.replication.min=3 .. IMPALA-3887: Use dfs.namenode.replication.min=3 Changes the HDFS config of our mini-cluster to use: dfs.namenode.replication.min=3 Several of our tests including planner tests expect HDFS blocks to have exactly three replicas. With the default HDFS configuration of dfs.namenode.replication.min=1, HDFS acks writes as soon as the min replication is satisfied. This can lead to test failures because HDFS replication is racing to replicate blocks while our tests are executing and expecting to see all replicas. A more detailed description is in the JIRA. Testing: - A core/hdfs ran passed - A core/hdfs run with data loading passed Change-Id: Ie09efdd9512cc4f0e338919af42bcdcede2cfb93 --- M fe/src/test/resources/hbase-site.xml.template M testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl 2 files changed, 21 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/8426/3 -- To view, visit http://gerrit.cloudera.org:8080/8426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie09efdd9512cc4f0e338919af42bcdcede2cfb93 Gerrit-Change-Number: 8426 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada
[Impala-ASF-CR] IMPALA-4964: Fix Decimal modulo overflow
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8329 ) Change subject: IMPALA-4964: Fix Decimal modulo overflow .. Patch Set 3: I'll do another pass with fresh eyes. I didn't have any specific concerns. -- To view, visit http://gerrit.cloudera.org:8080/8329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5420201d4440d421e33e443df005cdcc16b8a6cd Gerrit-Change-Number: 8329 Gerrit-PatchSet: 3 Gerrit-Owner: Taras Bobrovytsky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zach Amsden Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Tue, 31 Oct 2017 18:24:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4964: Fix Decimal modulo overflow
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8329 ) Change subject: IMPALA-4964: Fix Decimal modulo overflow .. Patch Set 3: Tim, do you want a second pair of eyes on this? If not, could you do the +2 review for this? -- To view, visit http://gerrit.cloudera.org:8080/8329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5420201d4440d421e33e443df005cdcc16b8a6cd Gerrit-Change-Number: 8329 Gerrit-PatchSet: 3 Gerrit-Owner: Taras Bobrovytsky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zach Amsden Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Tue, 31 Oct 2017 18:23:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5307: Part 4: copy out uncompressed text and seq
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8172 ) Change subject: IMPALA-5307: Part 4: copy out uncompressed text and seq .. Patch Set 8: (3 comments) Michael, can you do the +2 for this one too? http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-scan-node.cc@238 PS8, Line 238: nit: a lot of unnecessary blank lines. maybe condense a bit so more code fits on a screen. http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/scanner-context.h File be/src/exec/scanner-context.h: http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/scanner-context.h@a184 PS8, Line 184: nice to see this get simplified http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/runtime/row-batch.h File be/src/runtime/row-batch.h: http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/runtime/row-batch.h@a215 PS8, Line 215: great to see this go -- To view, visit http://gerrit.cloudera.org:8080/8172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I304fd002b61bfedf41c8b1405cd7eb7b492bb941 Gerrit-Change-Number: 8172 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 18:21:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5307: Part 4: copy out uncompressed text and seq
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8172 ) Change subject: IMPALA-5307: Part 4: copy out uncompressed text and seq .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-text-scanner.cc File be/src/exec/hdfs-text-scanner.cc: http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-text-scanner.cc@704 PS8, Line 704: bool split_delimiter_possible = context_->partition_descriptor()->line_delim() == '\n' I think there's a latent bug here where we're touching byte_buffer_ *after* calling CommitRows(). (I hit some ASAN issues in a follow-on patch). This patch increases the chances of hitting this since memory is recycled earlier. -- To view, visit http://gerrit.cloudera.org:8080/8172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I304fd002b61bfedf41c8b1405cd7eb7b492bb941 Gerrit-Change-Number: 8172 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 18:20:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6134: Update code base to use impala::ConditionVariable
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/8428 )
Change subject: IMPALA-6134: Update code base to use impala::ConditionVariable
..
Patch Set 1:
(4 comments)
No serious concerns, nice to make our code more consistent.
http://gerrit.cloudera.org:8080/#/c/8428/1/be/src/util/condition-variable.h
File be/src/util/condition-variable.h:
http://gerrit.cloudera.org:8080/#/c/8428/1/be/src/util/condition-variable.h@38
PS1, Line 38: void Wait(boost::unique_lock& lock) {
> I'm not yet familiar with this part of C++; why is 'inline' getting removed
"inline" is implied by the function being defined within the class body:
https://isocpp.org/wiki/faq/inline-functions#inline-member-fns-more . So the
inline specifier has no effect. I think in general we have a lot of unnecessary
inline specifiers in the code.
http://gerrit.cloudera.org:8080/#/c/8428/1/be/src/util/condition-variable.h@48
PS1, Line 48: const timespec* timeout
I feel like this should be a const&, since it has to be non-null. That would
let us eliminate one overload.
http://gerrit.cloudera.org:8080/#/c/8428/1/be/src/util/condition-variable.h@64
PS1, Line 64: template
It looks like this is only ever used with microseconds - maybe we should narrow
the scope of the API to use only that type so it's a bit more obvious what is
valid input. (We should definitely add a comment regardless documenting the
behaviour).
http://gerrit.cloudera.org:8080/#/c/8428/1/be/src/util/condition-variable.h@65
PS1, Line 65: bool TimedWait(boost::unique_lock& lock,
This looks like it only has one callsite. There's another callsite in
BlockingQueue::BlockingPutWithTimeout() that does the addition in the caller.
We should either consistently use this API or consistently do the calculation
in the caller. I don't feel strongly either way.
--
To view, visit http://gerrit.cloudera.org:8080/8428
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3085c6dcb42350b61244df6e7f091a1e7db356c9
Gerrit-Change-Number: 8428
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Tue, 31 Oct 2017 18:12:33 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6127: Fix timeout in TestRuntimeFilter.test wait time
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8427 ) Change subject: IMPALA-6127: Fix timeout in TestRuntimeFilter.test_wait_time .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1420/ -- To view, visit http://gerrit.cloudera.org:8080/8427 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee005bee8e0a535ce59d2e23e56be6004f2eb9de Gerrit-Change-Number: 8427 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 31 Oct 2017 18:07:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5599: Clean up references to TimestampValue in be/src.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/8305 ) Change subject: IMPALA-5599: Clean up references to TimestampValue in be/src. .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/8305/11/be/src/service/client-request-state.h File be/src/service/client-request-state.h: http://gerrit.cloudera.org:8080/#/c/8305/11/be/src/service/client-request-state.h@342 PS11, Line 342: start_time_us_, end_time_us_; These two may need to be initialized. -- To view, visit http://gerrit.cloudera.org:8080/8305 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I642a1d713597826bb7c15cd2ecb6638cb813a02c Gerrit-Change-Number: 8305 Gerrit-PatchSet: 11 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 31 Oct 2017 18:01:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/8202 ) Change subject: IMPALA-4704: Turns on client connections when local catalog initialized. .. Patch Set 18: (1 comment) http://gerrit.cloudera.org:8080/#/c/8202/18/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/8202/18/fe/src/main/java/org/apache/impala/service/Frontend.java@859 PS18, Line 859: Waits indefinitely > Will this impact shutting down? at least with the start-impala-cluster script, it had no issue. that script relies on killing the process. -- To view, visit http://gerrit.cloudera.org:8080/8202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6 Gerrit-Change-Number: 8202 Gerrit-PatchSet: 18 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Juan Yu Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 31 Oct 2017 17:52:57 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5599: Clean up references to TimestampValue in be/src.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/8305 ) Change subject: IMPALA-5599: Clean up references to TimestampValue in be/src. .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/8305/11/be/src/service/impala-http-handler.cc File be/src/service/impala-http-handler.cc: http://gerrit.cloudera.org:8080/#/c/8305/11/be/src/service/impala-http-handler.cc@303 PS11, Line 303: record.end_time_us - record.start_time_us; This may be broken if end_time_us is not set yet. The old code will use the current local time if end_time is not set yet but the new code may leave us a negative value. Sorry for missing that during review. -- To view, visit http://gerrit.cloudera.org:8080/8305 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I642a1d713597826bb7c15cd2ecb6638cb813a02c Gerrit-Change-Number: 8305 Gerrit-PatchSet: 11 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 31 Oct 2017 17:49:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6121: remove I/O mgr request context cache
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8408 ) Change subject: IMPALA-6121: remove I/O mgr request context cache .. Patch Set 3: This touches a lot of lines but is just simplification/cleanup - no expected behavioural changes. -- To view, visit http://gerrit.cloudera.org:8080/8408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91414eceaa4938fccd74686fe6bebede6ef36108 Gerrit-Change-Number: 8408 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 17:48:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6121: remove I/O mgr request context cache
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8408 ) Change subject: IMPALA-6121: remove I/O mgr request context cache .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/8408/3/be/src/runtime/disk-io-mgr-reader-context.h File be/src/runtime/disk-io-mgr-reader-context.h: PS3: I should rename this to disk-io-mgr-request-context.h to match the class name. -- To view, visit http://gerrit.cloudera.org:8080/8408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91414eceaa4938fccd74686fe6bebede6ef36108 Gerrit-Change-Number: 8408 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 17:47:34 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5307: Part 4: copy out uncompressed text and seq
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8172 ) Change subject: IMPALA-5307: Part 4: copy out uncompressed text and seq .. Patch Set 8: This is a follow-on to the last patch you reviewed. Hopefully should be easier to review since it's repeating the previous pattern and deleting dead code only. -- To view, visit http://gerrit.cloudera.org:8080/8172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I304fd002b61bfedf41c8b1405cd7eb7b492bb941 Gerrit-Change-Number: 8172 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 17:43:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
Juan Yu has posted comments on this change. ( http://gerrit.cloudera.org:8080/8202 ) Change subject: IMPALA-4704: Turns on client connections when local catalog initialized. .. Patch Set 18: (1 comment) http://gerrit.cloudera.org:8080/#/c/8202/18/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/8202/18/fe/src/main/java/org/apache/impala/service/Frontend.java@859 PS18, Line 859: Waits indefinitely Will this impact shutting down? If after wait for 30 seconds I decide to restart the whole service, could the Impalad be shutdown or restart quickly? -- To view, visit http://gerrit.cloudera.org:8080/8202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6 Gerrit-Change-Number: 8202 Gerrit-PatchSet: 18 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Juan Yu Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 31 Oct 2017 17:43:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4978 / IMPALA-5631: [DOCS] Add FQDN known issue
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/7388 ) Change subject: IMPALA-4978 / IMPALA-5631: [DOCS] Add FQDN known issue .. Patch Set 3: (1 comment) Sorry, just saw this today. http://gerrit.cloudera.org:8080/#/c/7388/3/docs/topics/impala_known_issues.xml File docs/topics/impala_known_issues.xml: http://gerrit.cloudera.org:8080/#/c/7388/3/docs/topics/impala_known_issues.xml@163 PS3, Line 163: Depends on the resolution of IMPALA-4978 I've punted on IMPALA-4978 because I was afraid that there may be some edge cases that we may regress on and never got back to it. But it's worth adding here that even if IMPALA-4978 is fixed, there will be cases where it's not possible to get the FQDN via getaddrinfo(). So, fixing IMPALA-4978 will of course alleviate this problem, but the problem may still show up in certain cases. In those cases, users still need to fall back to this workaround. I think this is something we want to add. -- To view, visit http://gerrit.cloudera.org:8080/7388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib039d0102878f1c05470371f581cb258287b9bc0 Gerrit-Change-Number: 7388 Gerrit-PatchSet: 3 Gerrit-Owner: John Russell Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: John Russell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Tue, 31 Oct 2017 17:34:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
Hello Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris Tsirogiannis, Alex Behm, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8202 to look at the new patch set (#18). Change subject: IMPALA-4704: Turns on client connections when local catalog initialized. .. IMPALA-4704: Turns on client connections when local catalog initialized. Currently, impalad starts beeswax and hs2 servers even if the catalog has not yet been initialized. As a result, client connections see an error message stating that the impalad is not yet ready. This patch changes the impalad startup sequence to wait until the catalog is received before opening beeswax and hs2 ports and starting their servers. Testing: - python e2e tests that start a cluster without a catalog and check that client connections are rejected as expected. Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6 --- M be/src/benchmarks/expr-benchmark.cc M be/src/common/global-flags.cc M be/src/service/frontend.cc M be/src/service/frontend.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/impalad-main.cc M be/src/testutil/in-process-servers.cc M be/src/testutil/in-process-servers.h M bin/start-impala-cluster.py M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/service/FrontendTest.java M tests/common/custom_cluster_test_suite.py M tests/common/impala_cluster.py M tests/common/impala_service.py A tests/custom_cluster/test_catalog_wait.py M tests/custom_cluster/test_coordinators.py 18 files changed, 282 insertions(+), 144 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/18 -- To view, visit http://gerrit.cloudera.org:8080/8202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6 Gerrit-Change-Number: 8202 Gerrit-PatchSet: 18 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/8202 ) Change subject: IMPALA-4704: Turns on client connections when local catalog initialized. .. Patch Set 17: (7 comments) http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@81 PS17, Line 81: /// > whitespace Done http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@85 PS17, Line 85: /// by clients at the same time. > Might want to weave in something like this to motivate the specific startup Done http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@87 PS17, Line 87: /// 1. Init > Init() Done http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@89 PS17, Line 89: ///- Wait (indefinitelt) for local catalog to be initialized from statestore (if coordinator) > typo: indefinitely Done http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@93 PS17, Line 93: /// 2. Start > Start() Done http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@98 PS17, Line 98: /// Membership callback thread: > When is the 'is_ready' metric set? currently, the main flips this bit-- its the one orchestrating the sequence so it knows when its ready. since we've now equated services_started_ and the ready flag, I'll consolidate them and move them to the server. perhaps there were other things the main wanted to do before announcing itself as ready, but for now, they're 1:1. http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1014 PS16, Line 1014: ExpirationQueue queries_by_timestamp_; > Fair point. I think the thing I'm objecting to here is that the internal state will depend on an external view. I'd prefer the arrows to be oriented only from internal to external (external depends on internal), even at the expense of storing this value twice. By using the metric, its the same as accessing a global. I don't see any example where we use metric values for internal state/logic-- afaict, they're used for sanity checks or logging (in addition to their intended purpose). I think its preferable to keep it that way. -- To view, visit http://gerrit.cloudera.org:8080/8202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6 Gerrit-Change-Number: 8202 Gerrit-PatchSet: 17 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 31 Oct 2017 17:30:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6134: Update code base to use impala::ConditionVariable
Philip Zeyliger has posted comments on this change. (
http://gerrit.cloudera.org:8080/8428 )
Change subject: IMPALA-6134: Update code base to use impala::ConditionVariable
..
Patch Set 1:
(1 comment)
Most of this looks mechanical, and it looked fine. You changed some 1-line if's
into 3-line ifs, which may be against house style.
We don't seem to have any tests for condition-variable.h. How did you test this?
http://gerrit.cloudera.org:8080/#/c/8428/1/be/src/util/condition-variable.h
File be/src/util/condition-variable.h:
http://gerrit.cloudera.org:8080/#/c/8428/1/be/src/util/condition-variable.h@38
PS1, Line 38: void Wait(boost::unique_lock& lock) {
I'm not yet familiar with this part of C++; why is 'inline' getting removed
here? And 'struct' in line 47?
--
To view, visit http://gerrit.cloudera.org:8080/8428
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3085c6dcb42350b61244df6e7f091a1e7db356c9
Gerrit-Change-Number: 8428
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Tue, 31 Oct 2017 17:23:37 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8412 ) Change subject: IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1419/ -- To view, visit http://gerrit.cloudera.org:8080/8412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2cd434757de2cd384def5b360a479e51812a Gerrit-Change-Number: 8412 Gerrit-PatchSet: 5 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 17:23:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/8412 ) Change subject: IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test .. Patch Set 5: Code-Review+2 Thanks for the review Tim. I added a function header comment for InitAuth() documenting this. Rebase. Carry +2. -- To view, visit http://gerrit.cloudera.org:8080/8412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2cd434757de2cd384def5b360a479e51812a Gerrit-Change-Number: 8412 Gerrit-PatchSet: 5 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 17:21:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test
Hello Lars Volker, Tim Armstrong, Alex Behm, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8412 to look at the new patch set (#4). Change subject: IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test .. IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test A recent patch for IMPALA-5129 introduced a use-after-free bug in thrift-server-test. It is fixed in this patch. Change-Id: I2cd434757de2cd384def5b360a479e51812a --- M be/src/rpc/auth-provider.h M be/src/rpc/thrift-server-test.cc 2 files changed, 19 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/8412/4 -- To view, visit http://gerrit.cloudera.org:8080/8412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2cd434757de2cd384def5b360a479e51812a Gerrit-Change-Number: 8412 Gerrit-PatchSet: 4 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6134: Update code base to use impala::ConditionVariable
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8428 ) Change subject: IMPALA-6134: Update code base to use impala::ConditionVariable .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8428/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8428/1//COMMIT_MSG@14 PS1, Line 14: This commit substitues every boost::condtion_variable in nit: condtion_variable -> condition_variable. -- To view, visit http://gerrit.cloudera.org:8080/8428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3085c6dcb42350b61244df6e7f091a1e7db356c9 Gerrit-Change-Number: 8428 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 17:15:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8412 ) Change subject: IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2cd434757de2cd384def5b360a479e51812a Gerrit-Change-Number: 8412 Gerrit-PatchSet: 3 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 17:13:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8412 ) Change subject: IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/8412/3/be/src/rpc/thrift-server-test.cc File be/src/rpc/thrift-server-test.cc: http://gerrit.cloudera.org:8080/#/c/8412/3/be/src/rpc/thrift-server-test.cc@145 PS3, Line 145: InitAuth Can we document this InitAuth() behaviour in the comment? I guess it only matters for tests that reinitialise it but good to have some documentation. -- To view, visit http://gerrit.cloudera.org:8080/8412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2cd434757de2cd384def5b360a479e51812a Gerrit-Change-Number: 8412 Gerrit-PatchSet: 3 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 17:13:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/8412 ) Change subject: IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test .. Patch Set 3: Looks like I found the real bug. The SASL library takes the string and holds a reference to it instead of copying it in sasl_server_init(). However, when we reinitialize the SASL library, it doesn't take in the new string because it detects that it was already previously initialized: https://github.com/cyrusimap/cyrus-sasl/blob/master/lib/server.c#L841 And we end up discarding the string that was held by it. So the fix is to get the string once and make sure it lives as long as the process does. -- To view, visit http://gerrit.cloudera.org:8080/8412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2cd434757de2cd384def5b360a479e51812a Gerrit-Change-Number: 8412 Gerrit-PatchSet: 3 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 17:00:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6134: Update code base to use impala::ConditionVariable
Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8428 Change subject: IMPALA-6134: Update code base to use impala::ConditionVariable .. IMPALA-6134: Update code base to use impala::ConditionVariable boost::condtion_variable supports thread interruption which has some overhead. In some places we already use impala::ConditionVariable which is a very thin layer around pthread, therefore it has less overhead. This commit substitues every boost::condtion_variable in the codebase (except under kudu/) to impala::ConditionVariable. It also extends impala::ConditionVariable class to support TimedWait based on boost::system_time and boost::time_duration. Change-Id: I3085c6dcb42350b61244df6e7f091a1e7db356c9 --- M be/src/catalog/catalog-server.cc M be/src/catalog/catalog-server.h M be/src/exec/plan-root-sink.cc M be/src/exec/plan-root-sink.h M be/src/rpc/thrift-server.cc M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/data-stream-mgr.h M be/src/runtime/data-stream-recvr.cc M be/src/runtime/data-stream-sender.cc M be/src/runtime/disk-io-mgr-internal.h M be/src/runtime/disk-io-mgr-reader-context.cc M be/src/runtime/disk-io-mgr-scan-range.cc M be/src/runtime/disk-io-mgr-stress.cc M be/src/runtime/disk-io-mgr-stress.h M be/src/runtime/disk-io-mgr-test.cc M be/src/runtime/disk-io-mgr.cc M be/src/runtime/disk-io-mgr.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/statestore/statestore.h M be/src/util/blocking-queue.h M be/src/util/condition-variable.h M be/src/util/promise.h M be/src/util/thread-pool.h 31 files changed, 134 insertions(+), 114 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/8428/1 -- To view, visit http://gerrit.cloudera.org:8080/8428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3085c6dcb42350b61244df6e7f091a1e7db356c9 Gerrit-Change-Number: 8428 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test
Hello Lars Volker, Tim Armstrong, Alex Behm, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8412 to look at the new patch set (#3). Change subject: IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test .. IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test A recent patch for IMPALA-5129 introduced a use-after-free bug in thrift-server-test. It is fixed in this patch. Change-Id: I2cd434757de2cd384def5b360a479e51812a --- M be/src/rpc/thrift-server-test.cc 1 file changed, 15 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/8412/3 -- To view, visit http://gerrit.cloudera.org:8080/8412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2cd434757de2cd384def5b360a479e51812a Gerrit-Change-Number: 8412 Gerrit-PatchSet: 3 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-3887: Use dfs.namenode.replication.min=3
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/8426 ) Change subject: IMPALA-3887: Use dfs.namenode.replication.min=3 .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/8426/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8426/2//COMMIT_MSG@9 PS2, Line 9: and HBase configuration of our Update? -- To view, visit http://gerrit.cloudera.org:8080/8426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie09efdd9512cc4f0e338919af42bcdcede2cfb93 Gerrit-Change-Number: 8426 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Comment-Date: Tue, 31 Oct 2017 16:52:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8412 ) Change subject: IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1418/ -- To view, visit http://gerrit.cloudera.org:8080/8412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2cd434757de2cd384def5b360a479e51812a Gerrit-Change-Number: 8412 Gerrit-PatchSet: 2 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 16:51:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8412 ) Change subject: IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2cd434757de2cd384def5b360a479e51812a Gerrit-Change-Number: 8412 Gerrit-PatchSet: 2 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 16:49:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8412 ) Change subject: IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test .. Patch Set 2: Code-Review+1 I'm ok with this to unblock things, but we should make sure to get to the root cause of the problem. -- To view, visit http://gerrit.cloudera.org:8080/8412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2cd434757de2cd384def5b360a479e51812a Gerrit-Change-Number: 8412 Gerrit-PatchSet: 2 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 16:41:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6127: Fix timeout in TestRuntimeFilter.test wait time
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8427 ) Change subject: IMPALA-6127: Fix timeout in TestRuntimeFilter.test_wait_time .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8427 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee005bee8e0a535ce59d2e23e56be6004f2eb9de Gerrit-Change-Number: 8427 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Alex Behm Gerrit-Comment-Date: Tue, 31 Oct 2017 16:22:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8202 ) Change subject: IMPALA-4704: Turns on client connections when local catalog initialized. .. Patch Set 17: (7 comments) http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@81 PS17, Line 81: /// whitespace http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@85 PS17, Line 85: /// by clients at the same time. Might want to weave in something like this to motivate the specific startup sequence: The Impala server is considered 'ready' iff it can successfully process requests in all of its roles. The goal is make the state of the service easy to reason about. http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@87 PS17, Line 87: /// 1. Init Init() http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@89 PS17, Line 89: ///- Wait (indefinitelt) for local catalog to be initialized from statestore (if coordinator) typo: indefinitely http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@93 PS17, Line 93: /// 2. Start Start() http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@98 PS17, Line 98: /// Membership callback thread: When is the 'is_ready' metric set? http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1014 PS16, Line 1014: ExpirationQueue queries_by_timestamp_; > clarified the comment. Fair point. My preference would be to use 'is_ready' to avoid redundant state. If metrics are seen as a view on the internal state (which I agree with!), then 'is_ready' should report the value of this 'services_started_' flag and not store a separate value. -- To view, visit http://gerrit.cloudera.org:8080/8202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6 Gerrit-Change-Number: 8202 Gerrit-PatchSet: 17 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 31 Oct 2017 16:21:23 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8412 ) Change subject: IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test .. Patch Set 2: I wonder if the bug is elsewhere and the copy-on-write refcounted string was somehow covering it up - I've seen that before when getting a const char* from a std::string - the memory remains valid until the last reference is destroyed. -- To view, visit http://gerrit.cloudera.org:8080/8412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2cd434757de2cd384def5b360a479e51812a Gerrit-Change-Number: 8412 Gerrit-PatchSet: 2 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Oct 2017 16:18:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3887: Use dfs.namenode.replication.min=3
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8426 ) Change subject: IMPALA-3887: Use dfs.namenode.replication.min=3 .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8426/1/fe/src/test/resources/hbase-site.xml.template File fe/src/test/resources/hbase-site.xml.template: http://gerrit.cloudera.org:8080/#/c/8426/1/fe/src/test/resources/hbase-site.xml.template@38 PS1, Line 38: : : dfs.namenode.replication.min : 3 : > Good question, let me give it a try. I tried without the min replication here and it works fine. Removed it. -- To view, visit http://gerrit.cloudera.org:8080/8426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie09efdd9512cc4f0e338919af42bcdcede2cfb93 Gerrit-Change-Number: 8426 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Comment-Date: Tue, 31 Oct 2017 15:57:16 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3887: Use dfs.namenode.replication.min=3
Hello Bharath Vissapragada, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8426 to look at the new patch set (#2). Change subject: IMPALA-3887: Use dfs.namenode.replication.min=3 .. IMPALA-3887: Use dfs.namenode.replication.min=3 Changes the HDFS and HBase configuration of our mini-cluster to use: dfs.namenode.replication.min=3 Several of our tests including planner tests expect HDFS blocks to have exactly three replicas. With the default HDFS configuration of dfs.namenode.replication.min=1, HDFS acks writes as soon as the min replication is satisfied. This can lead to test failures because HDFS replication is racing to replicate blocks while our tests are executing and expecting to see all replicas. A more detailed description is in the JIRA. Testing: - A core/hdfs ran passed - A core/hdfs run with data loading passed Change-Id: Ie09efdd9512cc4f0e338919af42bcdcede2cfb93 --- M fe/src/test/resources/hbase-site.xml.template M testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl 2 files changed, 21 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/8426/2 -- To view, visit http://gerrit.cloudera.org:8080/8426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie09efdd9512cc4f0e338919af42bcdcede2cfb93 Gerrit-Change-Number: 8426 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada
[Impala-ASF-CR] IMPALA-6127: Fix timeout in TestRuntimeFilter.test wait time
Thomas Tauber-Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8427 Change subject: IMPALA-6127: Fix timeout in TestRuntimeFilter.test_wait_time .. IMPALA-6127: Fix timeout in TestRuntimeFilter.test_wait_time test_wait_time has been flaky recently on ASAN due to hitting a timeout. The fix is to increase the timeout for ASAN builds. Change-Id: Iee005bee8e0a535ce59d2e23e56be6004f2eb9de --- M tests/query_test/test_runtime_filters.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/8427/1 -- To view, visit http://gerrit.cloudera.org:8080/8427 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iee005bee8e0a535ce59d2e23e56be6004f2eb9de Gerrit-Change-Number: 8427 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-5607: Add additional units to EXTRACT, DATE PART, TRUNC
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/8311 ) Change subject: IMPALA-5607: Add additional units to EXTRACT, DATE_PART, TRUNC .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/8311/4/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/8311/4/be/src/exprs/expr-test.cc@6037 PS4, Line 6037: TYPE_BIGINT, 28123); > This is a compatibility breaking change; we need to document the issue. I would suggest discussing this on dev@, since some other compat-breaking changes have been pushed to 3.0 and some other compat-breaking changes have been hidden behind a feature flag. -- To view, visit http://gerrit.cloudera.org:8080/8311 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If23ea310ddcc5c1c806b2b1ac24d24a4bd6aa4d9 Gerrit-Change-Number: 8311 Gerrit-PatchSet: 4 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Zach Amsden Gerrit-Comment-Date: Tue, 31 Oct 2017 14:41:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
Hello Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris Tsirogiannis, Alex Behm, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8202 to look at the new patch set (#17). Change subject: IMPALA-4704: Turns on client connections when local catalog initialized. .. IMPALA-4704: Turns on client connections when local catalog initialized. Currently, impalad starts beeswax and hs2 servers even if the catalog has not yet been initialized. As a result, client connections see an error message stating that the impalad is not yet ready. This patch changes the impalad startup sequence to wait until the catalog is received before opening beeswax and hs2 ports and starting their servers. Testing: - python e2e tests that start a cluster without a catalog and check that client connections are rejected as expected. Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6 --- M be/src/benchmarks/expr-benchmark.cc M be/src/common/global-flags.cc M be/src/service/frontend.cc M be/src/service/frontend.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/testutil/in-process-servers.cc M be/src/testutil/in-process-servers.h M bin/start-impala-cluster.py M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/service/FrontendTest.java M tests/common/custom_cluster_test_suite.py M tests/common/impala_cluster.py M tests/common/impala_service.py A tests/custom_cluster/test_catalog_wait.py M tests/custom_cluster/test_coordinators.py 17 files changed, 275 insertions(+), 141 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/17 -- To view, visit http://gerrit.cloudera.org:8080/8202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6 Gerrit-Change-Number: 8202 Gerrit-PatchSet: 17 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
Vuk Ercegovac has posted comments on this change. (
http://gerrit.cloudera.org:8080/8202 )
Change subject: IMPALA-4704: Turns on client connections when local catalog
initialized.
..
Patch Set 16:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h
File be/src/service/impala-server.h:
http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@105
PS16, Line 105: /// TODO: The state of a running query is currently not cleaned
up if the
> Let's document the startup procedure and the reasoning behind it here
Done
http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1014
PS16, Line 1014: /// Flag that records if backend and/or client services have
been started.
> Clarify the meaning of "and/or" here. The distinction seems important. Will
clarified the comment.
we can use "is_ready", but I think of metrics as a view on internal state, so
I'd prefer to not have internal state depend on them. any preferences on the
topic?
http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1017
PS16, Line 1017: boost::mutex services_started_lock_;
> std::atomic_bool instead of this lock?
Done
http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.cc@1626
PS16, Line 1626: // Register this backend only if services have been started.
> Feels clearer to me to check this at the caller. Can be hard to reason abou
had it at the caller before-- agreed that its clearer there.
http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.cc@1933
PS16, Line 1933: Status ImpalaServer::Init(int32_t thrift_be_port, int32_t
beeswax_port,
> Why reformat fn args? Seemed ok the way it was.
Done
http://gerrit.cloudera.org:8080/#/c/8202/16/tests/custom_cluster/test_catalog_wait.py
File tests/custom_cluster/test_catalog_wait.py:
http://gerrit.cloudera.org:8080/#/c/8202/16/tests/custom_cluster/test_catalog_wait.py@24
PS16, Line 24: """Impalad must wait for the catalog prior to opening up
client ports.
> Specify the waiting condition more precisely. An impalad must wait for the
Done
http://gerrit.cloudera.org:8080/#/c/8202/16/tests/custom_cluster/test_catalog_wait.py@54
PS16, Line 54: self.expect_connection(self.cluster.impalads[0])
> Ideally we'd also check the internal service
added a comment when issuing a query. a query will catch two cases: (1) impalad
registered prematurely is caught by query fragment metrics and (2) query
failure if the impalad registered as a backend but could not run a fragment.
http://gerrit.cloudera.org:8080/#/c/8202/16/tests/custom_cluster/test_catalog_wait.py@59
PS16, Line 59: def test_query_subset(self):
> Can we combine this test wit the one above? They seem very similar
Done
http://gerrit.cloudera.org:8080/#/c/8202/16/tests/custom_cluster/test_catalog_wait.py@71
PS16, Line 71:
self.cluster.impalads[0].service.get_metric_value('impala-server.ready', 1);
> I'm wondering whether this can be flaky. We often use self.wait_for_metric_
Done
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 16
Gerrit-Owner: Vuk Ercegovac
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Balazs Jeszenszky
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Michael Brown
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Tue, 31 Oct 2017 07:19:57 +
Gerrit-HasComments: Yes
