[Impala-ASF-CR] IMPALA-10364: Set the real location for external Iceberg tables stored in HadoopCatalog
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16795 ) Change subject: IMPALA-10364: Set the real location for external Iceberg tables stored in HadoopCatalog .. Patch Set 1: Code-Review+1 (1 comment) Thanks for this improvement, LGTM http://gerrit.cloudera.org:8080/#/c/16795/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/16795/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2742 PS1, Line 2742: newTable.getSd().setLocation(IcebergUtil.loadTable( : TIcebergCatalog.HADOOP_CATALOG, identifier, : IcebergUtil.getIcebergCatalogLocation(newTable)).location()); I think it's better to add comment here to describe the multiple namespaces situation. -- To view, visit http://gerrit.cloudera.org:8080/16795 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I04b75d219e095ce00b4c48f40b8dee872ba57b78 Gerrit-Change-Number: 16795 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Tue, 01 Dec 2020 07:50:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10366: skip test runtime profile aggregated for EC
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16799 ) Change subject: IMPALA-10366: skip test_runtime_profile_aggregated for EC .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16799 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2bb47d89f6d6c59242f2632c481f26d93e28e33e Gerrit-Change-Number: 16799 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 01 Dec 2020 06:39:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10366: skip test runtime profile aggregated for EC
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16799 ) Change subject: IMPALA-10366: skip test_runtime_profile_aggregated for EC .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7755/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/16799 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2bb47d89f6d6c59242f2632c481f26d93e28e33e Gerrit-Change-Number: 16799 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 01 Dec 2020 04:17:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10366: skip test runtime profile aggregated for EC
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16799 ) Change subject: IMPALA-10366: skip test_runtime_profile_aggregated for EC .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6720/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/16799 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2bb47d89f6d6c59242f2632c481f26d93e28e33e Gerrit-Change-Number: 16799 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 01 Dec 2020 03:55:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10366: skip test runtime profile aggregated for EC
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16799 Change subject: IMPALA-10366: skip test_runtime_profile_aggregated for EC .. IMPALA-10366: skip test_runtime_profile_aggregated for EC The schedule for erasure coded data results in 3 instead of 4 instances of the fragment with the scan. Skip the test - we don't need special coverage for erasure coding. Change-Id: I2bb47d89f6d6c59242f2632c481f26d93e28e33e --- M tests/common/skip.py M tests/custom_cluster/test_runtime_profile.py 2 files changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/16799/1 -- To view, visit http://gerrit.cloudera.org:8080/16799 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I2bb47d89f6d6c59242f2632c481f26d93e28e33e Gerrit-Change-Number: 16799 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-9930 (part 2): Introduce new admission control rpc service
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16412 ) Change subject: IMPALA-9930 (part 2): Introduce new admission control rpc service .. Patch Set 10: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I594fc593a27b24b6952e381a9bc1a9a5c6b757ae Gerrit-Change-Number: 16412 Gerrit-PatchSet: 10 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 01 Dec 2020 01:33:53 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16720 ) Change subject: [WIP] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate .. Patch Set 22: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7754/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/16720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I379405ee75b14929df7d6b5d20dabc6f51375691 Gerrit-Change-Number: 16720 Gerrit-PatchSet: 22 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 01 Dec 2020 01:10:21 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate
Qifan Chen has uploaded a new patch set (#22). ( http://gerrit.cloudera.org:8080/16720 ) Change subject: [WIP] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate .. [WIP] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate This patch adds the logic to utilize min/max stats for Parquet row groups or pages to skip these entities when they don't qualify an equi-join predicate. A new class of predicates called overlap predicates is introduced to aid in the determination of whether a Parquet row group or a page overlap with a range computed from the hash join. If not, then the entire Parquet row group or the page are skipped. The new class of predicates co-exist with the existing min/max conjuncts that are introduced based on the local or transitive scan predicates. Both classes of predicates can work individually or together with each other. The overlap predicates are evaluated after the existing min/max conjuncts. TBD: 1. Unit/performance testing; 2. Core testing. Change-Id: I379405ee75b14929df7d6b5d20dabc6f51375691 --- M be/src/exec/exec-node.h M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/hdfs-scan-node-base.h M be/src/exec/parquet/hdfs-parquet-scanner.cc M be/src/exec/parquet/hdfs-parquet-scanner.h M be/src/exec/parquet/parquet-column-stats.cc M be/src/exec/parquet/parquet-column-stats.h M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/scan-node.cc M be/src/runtime/coordinator.cc M be/src/util/min-max-filter.cc M be/src/util/min-max-filter.h M common/thrift/PlanNodes.thrift M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java 16 files changed, 688 insertions(+), 118 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/16720/22 -- To view, visit http://gerrit.cloudera.org:8080/16720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I379405ee75b14929df7d6b5d20dabc6f51375691 Gerrit-Change-Number: 16720 Gerrit-PatchSet: 22 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-9355: TestExchangeMemUsage.test exchange mem usage scaling doesn't hit the memory limit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16791 ) Change subject: IMPALA-9355: TestExchangeMemUsage.test_exchange_mem_usage_scaling doesn't hit the memory limit .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/16791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id945d7e37fac07beb7808e6ccf8530e667cbaad4 Gerrit-Change-Number: 16791 Gerrit-PatchSet: 1 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Nov 2020 22:33:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9355: TestExchangeMemUsage.test exchange mem usage scaling doesn't hit the memory limit
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16791 ) Change subject: IMPALA-9355: TestExchangeMemUsage.test_exchange_mem_usage_scaling doesn't hit the memory limit .. IMPALA-9355: TestExchangeMemUsage.test_exchange_mem_usage_scaling doesn't hit the memory limit This patch reduces the memory limit for the following query in test_exchange_mem_usage_scaling test from 170MB to 164MB to reduce the chance of not detecting a memory allocation failure. set mem_limit= set num_scanner_threads=1; select * from tpch_parquet.lineitem l1 join tpch_parquet.lineitem l2 on l1.l_orderkey = l2.l_orderkey and l1.l_partkey = l2.l_partkey and l1.l_suppkey = l2.l_suppkey and l1.l_linenumber = l2.l_linenumber order by l1.l_orderkey desc, l1.l_partkey, l1.l_suppkey, l1.l_linenumber limit 5; In a test with 500 executions of the above query with the memory limit set to 164MB, there were 500 memory allocation failures in total (one in each execution), and a total of 266 of them from Exchange Node #4. Testing: Ran the query in question individually; Ran TestExchangeMemUsage.test_exchange_mem_usage_scaling test; Ran core tests. Change-Id: Id945d7e37fac07beb7808e6ccf8530e667cbaad4 Reviewed-on: http://gerrit.cloudera.org:8080/16791 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M testdata/workloads/functional-query/queries/QueryTest/exchange-mem-scaling.test 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/16791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id945d7e37fac07beb7808e6ccf8530e667cbaad4 Gerrit-Change-Number: 16791 Gerrit-PatchSet: 2 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-10337: Consider MAX ROW SIZE when computing max reservation
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/16765 ) Change subject: IMPALA-10337: Consider MAX_ROW_SIZE when computing max reservation .. Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/16765/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16765/3//COMMIT_MSG@26 PS3, Line 26: > A better test is using SET_DENY_RESERVATION_PROBABILITY to verify the Buffe Added debug action SET_DENY_RESERVATION_PROBABILITY@1.0 in TestResultSpoolingMaxReservation. test_spilling_large_rows already exercise SET_DENY_RESERVATION_PROBABILITY with mt_dop=1. http://gerrit.cloudera.org:8080/#/c/16765/5/be/src/runtime/spillable-row-batch-queue.cc File be/src/runtime/spillable-row-batch-queue.cc: http://gerrit.cloudera.org:8080/#/c/16765/5/be/src/runtime/spillable-row-batch-queue.cc@97 PS5, Line 97: // have been set. > It'd be helpful to log the row size and batch_queue_->DebugString() here. Added row size and batch_queue_->DebugString() info in the log. However, this reminds me of IMPALA-9851. BufferedTupleStream::DebugString() iterate std::list that can potentially grow very large. While most references to this method are behind DCHEK logging (thus stripped in release build), one of them is used to build Status message through call to GroupingAggregator::Partition::DebugString() at this line: https://github.com/apache/impala/blob/5530b62/be/src/exec/grouping-aggregator-partition.cc#L155 IMPALA-9851 already cap error message to 128kb at most, but that does not eliminate the cost to iterate page list in BufferedTupleStream::DebugString(). Should I file this as separate Jira? http://gerrit.cloudera.org:8080/#/c/16765/3/fe/src/main/java/org/apache/impala/planner/PlanRootSink.java File fe/src/main/java/org/apache/impala/planner/PlanRootSink.java: http://gerrit.cloudera.org:8080/#/c/16765/3/fe/src/main/java/org/apache/impala/planner/PlanRootSink.java@90 PS3, Line 90: long minMemReservationBytes = 2 * maxRowBufferSize; > We should be able to work using the minimal reservation. So I think the pro Make sense. Bump up minMemReservation instead of maxMemReservationBytes in patch set #6. http://gerrit.cloudera.org:8080/#/c/16765/3/tests/query_test/test_result_spooling.py File tests/query_test/test_result_spooling.py: http://gerrit.cloudera.org:8080/#/c/16765/3/tests/query_test/test_result_spooling.py@68 PS3, Line 68: exec_options['max_row_size'] = 16 * 1024 > test_spilling and test_query_retries need to force spill by lowering spill Done http://gerrit.cloudera.org:8080/#/c/16765/3/tests/query_test/test_result_spooling.py@106 PS3, Line 106: assert re.search(plan_root_sink_reservation_limit, profile) > Thanks for the context above, makes sense to keep this too. Done -- To view, visit http://gerrit.cloudera.org:8080/16765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id7138e1e034ea5d1cd15cf8de399690e52a9d726 Gerrit-Change-Number: 16765 Gerrit-PatchSet: 6 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 30 Nov 2020 22:32:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10337: Consider MAX ROW SIZE when computing max reservation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16765 ) Change subject: IMPALA-10337: Consider MAX_ROW_SIZE when computing max reservation .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7753/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/16765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id7138e1e034ea5d1cd15cf8de399690e52a9d726 Gerrit-Change-Number: 16765 Gerrit-PatchSet: 6 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 30 Nov 2020 22:29:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10337: Consider MAX ROW SIZE when computing max reservation
Hello Quanlong Huang, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16765 to look at the new patch set (#6). Change subject: IMPALA-10337: Consider MAX_ROW_SIZE when computing max reservation .. IMPALA-10337: Consider MAX_ROW_SIZE when computing max reservation PlanRootSink can fail silently if result spooling is enabled and maxMemReservationBytes is less than 2 * MAX_ROW_SIZE. This happens because results are spilled using a SpillableRowBatchQueue which needs 2 buffer (read and write) with at least MAX_ROW_SIZE bytes per buffer. This patch fixes this by setting the PlanRootSink's minMemReservationBytes as 2 * maxRowBufferSize. Testing: - Pass exhaustive tests. - Add e2e TestResultSpoolingMaxReservation. - Lower MAX_ROW_SIZE on tests where MAX_RESULT_SPOOLING_MEM is set to extremely low value. Also verify that PLAN_ROOT_SINK's ReservationLimit remain unchanged after lowering the MAX_ROW_SIZE. Change-Id: Id7138e1e034ea5d1cd15cf8de399690e52a9d726 --- M be/src/runtime/buffered-tuple-stream.h M be/src/runtime/spillable-row-batch-queue.cc M fe/src/main/java/org/apache/impala/planner/PlanRootSink.java M tests/custom_cluster/test_query_retries.py M tests/query_test/test_result_spooling.py 5 files changed, 115 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/16765/6 -- To view, visit http://gerrit.cloudera.org:8080/16765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id7138e1e034ea5d1cd15cf8de399690e52a9d726 Gerrit-Change-Number: 16765 Gerrit-PatchSet: 6 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-10337: Consider MAX ROW SIZE when computing max reservation
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/16765 ) Change subject: IMPALA-10337: Consider MAX_ROW_SIZE when computing max reservation .. Patch Set 5: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/16765/3/tests/query_test/test_result_spooling.py File tests/query_test/test_result_spooling.py: http://gerrit.cloudera.org:8080/#/c/16765/3/tests/query_test/test_result_spooling.py@106 PS3, Line 106: assert re.search(plan_root_sink_reservation_limit, profile) > This assertion, however, is OK to delete, because it is not the focus of th Thanks for the context above, makes sense to keep this too. -- To view, visit http://gerrit.cloudera.org:8080/16765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id7138e1e034ea5d1cd15cf8de399690e52a9d726 Gerrit-Change-Number: 16765 Gerrit-PatchSet: 5 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 30 Nov 2020 19:52:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9930 (part 2): Introduce new admission control rpc service
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16412 ) Change subject: IMPALA-9930 (part 2): Introduce new admission control rpc service .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7752/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/16412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I594fc593a27b24b6952e381a9bc1a9a5c6b757ae Gerrit-Change-Number: 16412 Gerrit-PatchSet: 10 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Nov 2020 19:27:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9930 (part 2): Introduce new admission control rpc service
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/16412 ) Change subject: IMPALA-9930 (part 2): Introduce new admission control rpc service .. Patch Set 10: (3 comments) http://gerrit.cloudera.org:8080/#/c/16412/9/be/src/scheduling/admission-control-service.cc File be/src/scheduling/admission-control-service.cc: http://gerrit.cloudera.org:8080/#/c/16412/9/be/src/scheduling/admission-control-service.cc@159 PS9, Line 159: admission_state->admission_done = true; > Can we file a JIRA to rework this so that this doesn't block the RPC thread IMPALA-10359 http://gerrit.cloudera.org:8080/#/c/16412/9/be/src/scheduling/admission-control-service.cc@236 PS9, Line 236: } > When would this error happen? Is this the appropriate log level? Currently, this shouldn't ever happen. I think logging as an ERROR instead of eg. at FATAL or DCHECK-ing or similar seems like the most defensive choice. http://gerrit.cloudera.org:8080/#/c/16412/9/be/src/scheduling/remote-admission-control-client.cc File be/src/scheduling/remote-admission-control-client.cc: http://gerrit.cloudera.org:8080/#/c/16412/9/be/src/scheduling/remote-admission-control-client.cc@189 PS9, Line 189: RPC_TIMEOUT_MS, RPC_BACKOFF_TIME_MS, "REMOTE_AC_RELEASE_BACKENDS"); > Should we move these constants into a shared place? Done -- To view, visit http://gerrit.cloudera.org:8080/16412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I594fc593a27b24b6952e381a9bc1a9a5c6b757ae Gerrit-Change-Number: 16412 Gerrit-PatchSet: 10 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Nov 2020 19:06:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9930 (part 2): Introduce new admission control rpc service
Hello Sahil Takiar, Joe McDonnell, Tim Armstrong, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16412 to look at the new patch set (#10). Change subject: IMPALA-9930 (part 2): Introduce new admission control rpc service .. IMPALA-9930 (part 2): Introduce new admission control rpc service This patch introduces a new krpc service, AdmissionControlService, which coordinators can use to submit queries for admission. This patch adds some simple configuration flags that make it possible to have coordinators use this service to submit their queries for admission to other coordinators. These flags are only to make this patch testable and will be replaced when the separate admission control daemon is introduced in IMPALA-9975. The interface consists of the following RPCs: - AdmitQuery: takes a TQueryExecRequest and a TQueryOptions (serialized into sidecars), places the request on a queue to be processed by a thread pool and then immediately returns. - GetQueryStatus: takes a query id and returns the current admission status, including the QuerySchedulePB if admission has completed successfully but the query has not been released yet. - ReleaseQueryBackends: called when individual backends complete but the overall query is still running to release resources incrementally. This RPC will be called at most O(log(# backends)) per query due to BackendResourceState, which batches backends to release together. - ReleaseQuery: called when the query has completely finished. Releases all remaining resources. - CancelAdmission: called if a query is cancelled before an admission decision has been made to indicate that it should no longer be considered for admission. The majority of the patch consists of two classes: - AdmissionControlClient: used to abstract whether admission is being performed locally or remotely. In the local case, it is basically just a wrapper around AdmissionController. In the remote case, it handles serializing/deserializing of RPC params, polling GetQueryStatus() until a decision has been made, etc. - AdmissionControlService: exports the RPC interface and acts as a wrapper around AdmissionController. Some notable changes involved: - AdmissionController::SubmitForAdmission() no longer blocks while a query is queued. Instead, a new function WaitOnQueued() can be used to monitor the admission status of a queued query. - Adding events to the query timeline is moved out of AdmissionController and into the AdmissionControlClient classes, so that it always happens on the coordinator. - When a cluster is run in the new admission control service mode, only the impalad that is performing admission control exposes the /admission http endpoint. Observability will be cleaned up in a subsequent patch. Testing: - Modified existing admission control tests to run both with and without the admission control service enabled, including both the functional and stress tests. The 'num_queries' param in the stress test is modified to only use a single value to reduce the number of tests that are run and keep the running time reasonable. - Ran tpch10 on a local minicluster and observed no significant regressions. Change-Id: I594fc593a27b24b6952e381a9bc1a9a5c6b757ae --- M be/src/runtime/exec-env.cc M be/src/runtime/exec-env.h M be/src/scheduling/CMakeLists.txt M be/src/scheduling/admission-control-client.cc M be/src/scheduling/admission-control-client.h A be/src/scheduling/admission-control-service.cc A be/src/scheduling/admission-control-service.h M be/src/scheduling/admission-controller-test.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M be/src/scheduling/local-admission-control-client.cc M be/src/scheduling/local-admission-control-client.h A be/src/scheduling/remote-admission-control-client.cc A be/src/scheduling/remote-admission-control-client.h M be/src/scheduling/schedule-state.cc M be/src/scheduling/schedule-state.h M be/src/service/client-request-state.cc M be/src/service/impala-http-handler.cc M be/src/util/sharded-query-map-util.cc M common/protobuf/admission_control_service.proto M tests/common/resource_pool_config.py M tests/custom_cluster/test_admission_controller.py M tests/hs2/hs2_test_suite.py M tests/util/web_pages_util.py 24 files changed, 1,268 insertions(+), 171 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/16412/10 -- To view, visit http://gerrit.cloudera.org:8080/16412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I594fc593a27b24b6952e381a9bc1a9a5c6b757ae Gerrit-Change-Number: 16412 Gerrit-PatchSet: 10 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala
[Impala-ASF-CR] [WIP] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16720 ) Change subject: [WIP] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/16720/12//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16720/12//COMMIT_MSG@9 PS12, Line 9: This patch adds the logic to utilize min/max stats > On the scope of the work. I think getting the row group/page filtering working (with all data types, etc) is a good end-point for this patch. My understanding is that the row/partition filtering will get enabled automatically once the filters are generated and I just want to understand the implications of that: * It looks like the min-max filters are ordered after the bloom filters for evaluation purposes. * It looks like the min-max filters don't count towards the MAX_NUM_RUNTIME_FILTERS limit - https://impala.apache.org/docs/build/html/topics/impala_max_num_runtime_filters.html#max_num_runtime_filters. So this means we will maybe get some new source/destination pairs, which might change the runtime behaviour of some plans. I suspect this is all a net win, since the min-max filters should be relatively cheap to construct and will get automatically disabled if they're ineffective in the scan, but there is a bit of overhead added. So I think we want to do some benchmarks to make sure there's no regressions before changing the default. Probably TPC-DS since it is heavy on the filters. -- To view, visit http://gerrit.cloudera.org:8080/16720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I379405ee75b14929df7d6b5d20dabc6f51375691 Gerrit-Change-Number: 16720 Gerrit-PatchSet: 12 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 30 Nov 2020 18:31:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16720 ) Change subject: [WIP] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate .. Patch Set 21: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7751/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/16720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I379405ee75b14929df7d6b5d20dabc6f51375691 Gerrit-Change-Number: 16720 Gerrit-PatchSet: 21 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 30 Nov 2020 18:20:44 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate
Qifan Chen has uploaded a new patch set (#21). ( http://gerrit.cloudera.org:8080/16720 ) Change subject: [WIP] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate .. [WIP] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate This patch adds the logic to utilize min/max stats for Parquet row groups or pages to skip these entities when they don't qualify an equi-join predicate. A new class of predicates called overlap predicates is introduced to aid in the determination of whether a Parquet row group or a page overlap with a range computed from the hash join. If not, then the entire Parquet row group or the page are skipped. The new class of predicates co-exist with the existing min/max conjuncts that are introduced based on the local scan predicates. Both classes of predicates can work individually or together with each other. The overlap predicates are evaluated after the existing min/max conjuncts. TBD: 1. Unit/performance testing; 2. Core testing. Change-Id: I379405ee75b14929df7d6b5d20dabc6f51375691 --- M be/src/exec/exec-node.h M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/hdfs-scan-node-base.h M be/src/exec/parquet/hdfs-parquet-scanner.cc M be/src/exec/parquet/hdfs-parquet-scanner.h M be/src/exec/parquet/parquet-column-stats.cc M be/src/exec/parquet/parquet-column-stats.h M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/scan-node.cc M be/src/runtime/coordinator.cc M be/src/util/min-max-filter.cc M be/src/util/min-max-filter.h M common/thrift/PlanNodes.thrift M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java 16 files changed, 661 insertions(+), 119 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/16720/21 -- To view, visit http://gerrit.cloudera.org:8080/16720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I379405ee75b14929df7d6b5d20dabc6f51375691 Gerrit-Change-Number: 16720 Gerrit-PatchSet: 21 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-9910: [DOCS] Add fault tolerance docs
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/16610 ) Change subject: IMPALA-9910: [DOCS] Add fault tolerance docs .. Patch Set 3: Code-Review+2 There hasn't been any movement here in awhile. While there are definite further improvements that could be made, I think its already a big improvement as-is, so I'll go ahead and submit this, unless anyone has any objections. -- To view, visit http://gerrit.cloudera.org:8080/16610 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9d178b21a9654bbed8b814ccadca95703ffacb62 Gerrit-Change-Number: 16610 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Shajini Thayasingh Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Mon, 30 Nov 2020 17:56:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9355: TestExchangeMemUsage.test exchange mem usage scaling doesn't hit the memory limit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16791 ) Change subject: IMPALA-9355: TestExchangeMemUsage.test_exchange_mem_usage_scaling doesn't hit the memory limit .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6719/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/16791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id945d7e37fac07beb7808e6ccf8530e667cbaad4 Gerrit-Change-Number: 16791 Gerrit-PatchSet: 1 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Nov 2020 17:10:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9355: TestExchangeMemUsage.test exchange mem usage scaling doesn't hit the memory limit
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16791 ) Change subject: IMPALA-9355: TestExchangeMemUsage.test_exchange_mem_usage_scaling doesn't hit the memory limit .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id945d7e37fac07beb7808e6ccf8530e667cbaad4 Gerrit-Change-Number: 16791 Gerrit-PatchSet: 1 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Nov 2020 17:09:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10364: Set the real location for external Iceberg tables stored in HadoopCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16795 ) Change subject: IMPALA-10364: Set the real location for external Iceberg tables stored in HadoopCatalog .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7750/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/16795 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I04b75d219e095ce00b4c48f40b8dee872ba57b78 Gerrit-Change-Number: 16795 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Mon, 30 Nov 2020 15:58:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10364: Set the real location for external Iceberg tables stored in HadoopCatalog
Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16795 Change subject: IMPALA-10364: Set the real location for external Iceberg tables stored in HadoopCatalog .. IMPALA-10364: Set the real location for external Iceberg tables stored in HadoopCatalog Impala tries to come up with the table location of external Iceberg tables stored in HadoopCatalog. The current method is not correct for tables that are nested under multiple namespaces. With this patch Imapala loads the Iceberg table and retrieves the location from it. Testing: * added e2e test in iceberg-create.test Change-Id: I04b75d219e095ce00b4c48f40b8dee872ba57b78 --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/IcebergUtil.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test 3 files changed, 33 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/16795/1 -- To view, visit http://gerrit.cloudera.org:8080/16795 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I04b75d219e095ce00b4c48f40b8dee872ba57b78 Gerrit-Change-Number: 16795 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-10361: Supported using field id to resolve columns for Iceberg tables
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16788 ) Change subject: IMPALA-10361: Supported using field id to resolve columns for Iceberg tables .. Patch Set 2: (5 comments) Thanks WangSheng for working on this important task! I did a first pass and the change looks good to me. Looking forward for the tests. http://gerrit.cloudera.org:8080/#/c/16788/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16788/2//COMMIT_MSG@7 PS2, Line 7: Supported nit: Use field id to resolve columns for Iceberg tables http://gerrit.cloudera.org:8080/#/c/16788/2//COMMIT_MSG@11 PS2, Line 11: to choose field id : resolving. I think this should be the default for Iceberg tables. http://gerrit.cloudera.org:8080/#/c/16788/2/common/thrift/ImpalaInternalService.thrift File common/thrift/ImpalaInternalService.thrift: http://gerrit.cloudera.org:8080/#/c/16788/2/common/thrift/ImpalaInternalService.thrift@48 PS2, Line 48: FIELDID nit: FIELD_ID http://gerrit.cloudera.org:8080/#/c/16788/2/fe/src/main/java/org/apache/impala/catalog/IcebergStructField.java File fe/src/main/java/org/apache/impala/catalog/IcebergStructField.java: http://gerrit.cloudera.org:8080/#/c/16788/2/fe/src/main/java/org/apache/impala/catalog/IcebergStructField.java@25 PS2, Line 25: nit: add comment http://gerrit.cloudera.org:8080/#/c/16788/2/fe/src/main/java/org/apache/impala/catalog/Table.java File fe/src/main/java/org/apache/impala/catalog/Table.java: http://gerrit.cloudera.org:8080/#/c/16788/2/fe/src/main/java/org/apache/impala/catalog/Table.java@439 PS2, Line 439: By nit: From -- To view, visit http://gerrit.cloudera.org:8080/16788 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I057bdc6ab2859cc4d40de5ed428d0c20028b8435 Gerrit-Change-Number: 16788 Gerrit-PatchSet: 2 Gerrit-Owner: wangsheng Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Mon, 30 Nov 2020 12:38:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] WiP: IMPALA-10237: Support Bucket and Truncate partition transforms as built-in functions
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16741 ) Change subject: WiP: IMPALA-10237: Support Bucket and Truncate partition transforms as built-in functions .. Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/16741/3/be/src/exprs/CMakeLists.txt File be/src/exprs/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/16741/3/be/src/exprs/CMakeLists.txt@90 PS3, Line 90: nit: space http://gerrit.cloudera.org:8080/#/c/16741/3/be/src/exprs/iceberg-functions-ir.cc File be/src/exprs/iceberg-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/16741/3/be/src/exprs/iceberg-functions-ir.cc@26 PS3, Line 26: nit: too much spaces http://gerrit.cloudera.org:8080/#/c/16741/3/be/src/exprs/iceberg-functions-ir.cc@77 PS3, Line 77: return input.val - (((input.val % width.val) + width.val) % width.val); maybe we could have a fast-path when input.val is nonnegative, i.e. in that case the expression could be simply: input.val - (input.val % width.val) http://gerrit.cloudera.org:8080/#/c/16741/3/be/src/exprs/iceberg-functions-ir.cc@84 PS3, Line 84: decimal_val - (decimal_val % width); Is this correct formula if decimal_val is negative? -- To view, visit http://gerrit.cloudera.org:8080/16741 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I485680cf79d96d578dd8cfbfd554bec468fe84bd Gerrit-Change-Number: 16741 Gerrit-PatchSet: 3 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 30 Nov 2020 11:59:04 + Gerrit-HasComments: Yes
