[Impala-ASF-CR] IMPALA-10450: Catalogd crashes due to exception in ThriftDebugString
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17110 ) Change subject: IMPALA-10450: Catalogd crashes due to exception in ThriftDebugString .. Patch Set 8: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/17110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I42cee6186a3d5bacc1117bae5961ac60ac9f7a66 Gerrit-Change-Number: 17110 Gerrit-PatchSet: 8 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 02 Mar 2021 06:52:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10524: Changes to HdfsPartition for third party extensions.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17092 ) Change subject: IMPALA-10524: Changes to HdfsPartition for third party extensions. .. IMPALA-10524: Changes to HdfsPartition for third party extensions. Some changes are needed to HdfsPartition and other related classes to allow for third party extensions. These changes include: - A protected constructor which will allow a subclass to instantiate HdfsPartition using its own Builder. - Various changes of permissions to methods and variables to allow third party extension visibility. - Creation of the getHostIndex() method to allow the subclass to override how the hostIndexes are retrieved. - Added a new default method "getFileSystem()" to FeFsPartition which will allow the third party extension to override how the filesystem is obtained from the partition object. Change-Id: I5a792642f27228118ac8f2e8ef98e8ba7aee4a46 Reviewed-on: http://gerrit.cloudera.org:8080/17092 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/analysis/AnalyticWindow.java M fe/src/main/java/org/apache/impala/analysis/StatementBase.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartitionLocationCompressor.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java M fe/src/main/java/org/apache/impala/planner/CardinalityCheckNode.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java M fe/src/main/java/org/apache/impala/planner/PlanNode.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/SortNode.java M fe/src/main/java/org/apache/impala/planner/SubplanNode.java 14 files changed, 82 insertions(+), 25 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/17092 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I5a792642f27228118ac8f2e8ef98e8ba7aee4a46 Gerrit-Change-Number: 17092 Gerrit-PatchSet: 13 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Steve Carlin
[Impala-ASF-CR] IMPALA-10524: Changes to HdfsPartition for third party extensions.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17092 ) Change subject: IMPALA-10524: Changes to HdfsPartition for third party extensions. .. Patch Set 12: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/17092 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a792642f27228118ac8f2e8ef98e8ba7aee4a46 Gerrit-Change-Number: 17092 Gerrit-PatchSet: 12 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Tue, 02 Mar 2021 05:57:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9856: Enable result spooling by default.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16755 ) Change subject: IMPALA-9856: Enable result spooling by default. .. IMPALA-9856: Enable result spooling by default. Result spooling has been relatively stable since it was introduced, and it has several benefits described in IMPALA-8656. This patch enable result spooling (SPOOL_QUERY_RESULTS) query options by default. Furthermore, some tests need to be adjusted to account for result spooling by default. The following are the adjustment categories and list of tests that fall under such category. Change in assertions: PlannerTest#testAcidTableScans PlannerTest#testBloomFilterAssignment PlannerTest#testConstantFolding PlannerTest#testFkPkJoinDetection PlannerTest#testFkPkJoinDetectionWithHDFSNumRowsEstDisabled PlannerTest#testKuduSelectivity PlannerTest#testMaxRowSize PlannerTest#testMinMaxRuntimeFilters PlannerTest#testMinMaxRuntimeFiltersWithHDFSNumRowsEstDisabled PlannerTest#testMtDopValidation PlannerTest#testParquetFiltering PlannerTest#testParquetFilteringDisabled PlannerTest#testPartitionPruning PlannerTest#testPreaggBytesLimit PlannerTest#testResourceRequirements PlannerTest#testRuntimeFilterQueryOptions PlannerTest#testSortExprMaterialization PlannerTest#testSpillableBufferSizing PlannerTest#testTableSample PlannerTest#testTpch PlannerTest#testKuduTpch PlannerTest#testTpchNested PlannerTest#testUnion TpcdsPlannerTest custom_cluster/test_admission_controller.py::TestAdmissionController::test_dedicated_coordinator_planner_estimates custom_cluster/test_admission_controller.py::TestAdmissionController::test_memory_rejection custom_cluster/test_admission_controller.py::TestAdmissionController::test_pool_mem_limit_configs metadata/test_explain.py::TestExplain::test_explain_level2 metadata/test_explain.py::TestExplain::test_explain_level3 metadata/test_stats_extrapolation.py::TestStatsExtrapolation::test_stats_extrapolation Increase BUFFER_POOL_LIMIT: query_test/test_queries.py::TestQueries::test_analytic_fns query_test/test_runtime_filters.py::TestRuntimeRowFilters::test_row_filter_reservation query_test/test_sort.py::TestQueryFullSort::test_multiple_mem_limits_full_output query_test/test_spilling.py::TestSpillingBroadcastJoins::test_spilling_broadcast_joins query_test/test_spilling.py::TestSpillingDebugActionDimensions::test_spilling_aggs query_test/test_spilling.py::TestSpillingDebugActionDimensions::test_spilling_regression_exhaustive query_test/test_udfs.py::TestUdfExecution::test_mem_limits Increase MEM_LIMIT: query_test/test_mem_usage_scaling.py::TestExchangeMemUsage::test_exchange_mem_usage_scaling query_test/test_mem_usage_scaling.py::TestScanMemLimit::test_hdfs_scanner_thread_mem_scaling Increase MAX_ROW_SIZE: custom_cluster/test_parquet_max_page_header.py::TestParquetMaxPageHeader::test_large_page_header_config query_test/test_insert.py::TestInsertQueries::test_insert_large_string query_test/test_query_mem_limit.py::TestQueryMemLimit::test_mem_limit query_test/test_scanners.py::TestTextSplitDelimiters::test_text_split_across_buffers_delimiter query_test/test_scanners.py::TestWideRow::test_wide_row Disable result spooling to maintain assertion: custom_cluster/test_admission_controller.py::TestAdmissionController::test_set_request_pool custom_cluster/test_admission_controller.py::TestAdmissionController::test_timeout_reason_host_memory custom_cluster/test_admission_controller.py::TestAdmissionController::test_timeout_reason_pool_memory custom_cluster/test_admission_controller.py::TestAdmissionController::test_queue_reasons_memory custom_cluster/test_admission_controller.py::TestAdmissionController::test_pool_config_change_while_queued custom_cluster/test_query_retries.py::TestQueryRetries::test_retry_fetched_rows custom_cluster/test_query_retries.py::TestQueryRetries::test_retry_finished_query custom_cluster/test_scratch_disk.py::TestScratchDir::test_no_dirs custom_cluster/test_scratch_disk.py::TestScratchDir::test_non_existing_dirs custom_cluster/test_scratch_disk.py::TestScratchDir::test_non_writable_dirs query_test/test_insert.py::TestInsertQueries::test_insert_large_string (the last query only) query_test/test_kudu.py::TestKuduMemLimits::test_low_mem_limit_low_selectivity_scan query_test/test_mem_usage_scaling.py::TestScanMemLimit::test_kudu_scan_mem_usage query_test/test_queries.py::TestQueriesParquetTables::test_very_large_strings query_test/test_query_mem_limit.py::TestCodegenMemLimit::test_codegen_mem_limit shell/test_shell_client.py::TestShellClient::test_fetch_size Testing: - Pass exhaustive tests. Change-Id: I9e360c1428676d8f3fab5d95efee18aca085eba4 Reviewed-on: http://gerrit.cloudera.org:8080/16755 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M common/thrift/ImpalaInternalService.thrift M testdata/workloads/functional-planner/queries/PlannerTest/acid-scans.test M testdata
[Impala-ASF-CR] IMPALA-9856: Enable result spooling by default.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16755 ) Change subject: IMPALA-9856: Enable result spooling by default. .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/16755 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e360c1428676d8f3fab5d95efee18aca085eba4 Gerrit-Change-Number: 16755 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: Tue, 02 Mar 2021 04:58:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10550: Add External Frontend service port
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17125 ) Change subject: IMPALA-10550: Add External Frontend service port .. Patch Set 7: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/6927/ -- To view, visit http://gerrit.cloudera.org:8080/17125 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I991b5b05e12e37d8739e18ed1086bbb0228acc40 Gerrit-Change-Number: 17125 Gerrit-PatchSet: 7 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 02 Mar 2021 04:23:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10494: Making use of the min/max column stats to improve min/max filters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17075 ) Change subject: IMPALA-10494: Making use of the min/max column stats to improve min/max filters .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8279/ : 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/17075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I08581b44419bb8da5940cbf98502132acd1c86df Gerrit-Change-Number: 17075 Gerrit-PatchSet: 12 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Comment-Date: Tue, 02 Mar 2021 03:58:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10494: Making use of the min/max column stats to improve min/max filters
Qifan Chen has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/17075 ) Change subject: IMPALA-10494: Making use of the min/max column stats to improve min/max filters .. IMPALA-10494: Making use of the min/max column stats to improve min/max filters This patch adds the functionality to compute the minimal and the maximal value for a column of type integers, float or double for parquet tables, and to make use of the new stats to discard the min/max filters whose coverage are too close to the actual range. Only the min/max values for non-partition columns are stored in HMS. The min/max values for partition columns are computed in coordinator. Two new columns 'Min' and 'Max' are added in the output of the show column stats command as shown below. show column stats tpcds_parquet.store_sales +---+--+-...---+-+-+ | Column| Type | #Falses | Min | Max | +---+--+-...---+-+-+ | ss_sold_time_sk | INT | -1 | 28800 | 75599 | | ss_item_sk| BIGINT | -1 | 1 | 18000 | | ss_customer_sk| INT | -1 | 1 | 10 | | ss_cdemo_sk | INT | -1 | 15 | 1920797 | | ss_hdemo_sk | INT | -1 | 1 | 7200| | ss_addr_sk| INT | -1 | 1 | 5 | | ss_store_sk | INT | -1 | 1 | 10 | | ss_promo_sk | INT | -1 | 1 | 300 | | ss_ticket_number | BIGINT | -1 | 1 | 24 | | ss_quantity | INT | -1 | 1 | 100 | | ss_wholesale_cost | DECIMAL(7,2) | -1 | -1 | -1 | | ss_list_price | DECIMAL(7,2) | -1 | -1 | -1 | | ss_sales_price| DECIMAL(7,2) | -1 | -1 | -1 | | ss_ext_discount_amt | DECIMAL(7,2) | -1 | -1 | -1 | | ss_ext_sales_price| DECIMAL(7,2) | -1 | -1 | -1 | | ss_ext_wholesale_cost | DECIMAL(7,2) | -1 | -1 | -1 | | ss_ext_list_price | DECIMAL(7,2) | -1 | -1 | -1 | | ss_ext_tax| DECIMAL(7,2) | -1 | -1 | -1 | | ss_coupon_amt | DECIMAL(7,2) | -1 | -1 | -1 | | ss_net_paid | DECIMAL(7,2) | -1 | -1 | -1 | | ss_net_paid_inc_tax | DECIMAL(7,2) | -1 | -1 | -1 | | ss_net_profit | DECIMAL(7,2) | -1 | -1 | -1 | | ss_sold_date_sk | INT | -1 | 2450816 | 2452642 | +---+--+-...---+-+-+ Testing: - Added TestLowAndHighValueShort and TestLowAndHighValueInt to IncrStatsUtilTest TODO: 1. Test compute stats for timestamp and date columns; 2. Test filters being disabled at the scan node; 3. Add logic to disable min/max filters inside HJ builder via the column stats. Change-Id: I08581b44419bb8da5940cbf98502132acd1c86df --- M be/src/exec/catalog-op-executor.cc M be/src/exec/filter-context.cc M be/src/exec/filter-context.h M be/src/exec/hdfs-scanner.h M be/src/exec/incr-stats-util-test.cc M be/src/exec/incr-stats-util.cc M be/src/exec/incr-stats-util.h M be/src/exec/parquet/hdfs-parquet-scanner.cc M be/src/service/hs2-util.cc M be/src/service/hs2-util.h M be/src/service/query-options.cc M be/src/service/query-options.h M be/src/util/min-max-filter.h M common/thrift/CatalogObjects.thrift M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M common/thrift/PlanNodes.thrift M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/catalog/ColumnStats.java M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M testdata/workloads/functional-query/queries/QueryTest/overlap_min_max_filters.test 26 files changed, 847 insertions(+), 68 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/17075/12 -- To view, visit http://gerrit.cloudera.org:8080/17075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I08581b44419bb8da5940cbf98502132acd1c86df Gerrit-Change-Number: 17075 Gerrit-PatchSet: 12 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen
[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 ) Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8278/ : 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/17136 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53 Gerrit-Change-Number: 17136 Gerrit-PatchSet: 3 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Tue, 02 Mar 2021 02:36:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 ) Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/17136/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17136/2//COMMIT_MSG@23 PS2, Line 23: Set the default fs as a s3 path, reran > Could you add an unit-test? Done http://gerrit.cloudera.org:8080/#/c/17136/2/be/src/runtime/tmp-file-mgr.cc File be/src/runtime/tmp-file-mgr.cc: http://gerrit.cloudera.org:8080/#/c/17136/2/be/src/runtime/tmp-file-mgr.cc@663 PS2, Line 663: -1, true, false > When you have long parameter lists with constants where its not clear what Done -- To view, visit http://gerrit.cloudera.org:8080/17136 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53 Gerrit-Change-Number: 17136 Gerrit-PatchSet: 2 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Tue, 02 Mar 2021 02:18:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
Yida Wu has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/17136 ) Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build .. IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build For start option “scratch_dirs”, it only considers local filesystem as the default filesystem, regardless of the setting of DefaultFS(for a remote scratch dir, it needs to explicitly set it with the remote fs prefix). However, the function AssignQueue() would assign the queue based on not only the path string but also the default filesystem setting. For example, if scratch_dirs is set as "/tmp", the scratch dir is supposed to be in the local filesystem, but the AssignQueue() would consider it as "s3a://xxx/tmp" if a s3 path is set as the default fs. To fix this, the solution is to add a bool variable to AssignQueue() to decide whether or not to check the default fs setting when parsing the file path. For all of the scratch dirs, AssignQueue() won’t check the default fs. Tests: Added a unit testcase: TmpFileMgrTest::TestSpillingWithRemoteDefaultFS. Ran and Passed TmpFileMgrTest. Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53 --- M be/src/runtime/io/disk-io-mgr.cc M be/src/runtime/io/disk-io-mgr.h M be/src/runtime/io/scan-range.cc M be/src/runtime/test-env.h M be/src/runtime/tmp-file-mgr-test.cc M be/src/runtime/tmp-file-mgr.cc 6 files changed, 56 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/17136/3 -- To view, visit http://gerrit.cloudera.org:8080/17136 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53 Gerrit-Change-Number: 17136 Gerrit-PatchSet: 3 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-10535: Add interface to ImpalaServer for execution of externally compiled statements
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17104 ) Change subject: IMPALA-10535: Add interface to ImpalaServer for execution of externally compiled statements .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8277/ : 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/17104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iace716dd67290f08441857dc02d2428b0e335eaa Gerrit-Change-Number: 17104 Gerrit-PatchSet: 8 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 02 Mar 2021 01:54:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10550: Add External Frontend service port
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17125 ) Change subject: IMPALA-10550: Add External Frontend service port .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8276/ : 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/17125 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I991b5b05e12e37d8739e18ed1086bbb0228acc40 Gerrit-Change-Number: 17125 Gerrit-PatchSet: 8 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 02 Mar 2021 01:53:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Kurt Deschler has posted comments on this change. (
http://gerrit.cloudera.org:8080/17115 )
Change subject: IMPALA-10522: Support external use of frontend libraries
..
Patch Set 8:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/runtime/exec-env.h
File be/src/runtime/exec-env.h:
http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/runtime/exec-env.h@82
PS8, Line 82: class ExecEnv {
> Might be nice to flesh out the class comment here a little about what it me
Done
http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:
http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/runtime/exec-env.cc@253
PS8, Line 253: frontend_(external_fe ? nullptr : new Frontend()),
> Its a bit tricky to trace through and prove that the variables here that ar
All of the local references were inside null checks. I added DCHECK to the
frontend() method to protect external callers.
http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/service/fe-support.cc
File be/src/service/fe-support.cc:
http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/service/fe-support.cc@83
PS8, Line 83: InitForFeTests
> This function name is unfortunate now. Not sure what a better options is -
Done
http://gerrit.cloudera.org:8080/#/c/17115/8/fe/src/main/java/org/apache/impala/service/FeSupport.java
File fe/src/main/java/org/apache/impala/service/FeSupport.java:
http://gerrit.cloudera.org:8080/#/c/17115/8/fe/src/main/java/org/apache/impala/service/FeSupport.java@489
PS8, Line 489: public static void loadLibrary() {
> I'm a little confused by this function and the 'externalFE_' variable - I g
Done
--
To view, visit http://gerrit.cloudera.org:8080/17115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
Gerrit-Change-Number: 17115
Gerrit-PatchSet: 8
Gerrit-Owner: Kurt Deschler
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: John Sherman
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Tue, 02 Mar 2021 01:53:09 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10535: Add interface to ImpalaServer for execution of externally compiled statements
Hello Thomas Tauber-Marshall, Joe McDonnell, John Sherman, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17104 to look at the new patch set (#8). Change subject: IMPALA-10535: Add interface to ImpalaServer for execution of externally compiled statements .. IMPALA-10535: Add interface to ImpalaServer for execution of externally compiled statements The ExecutePlannedStatement interface allows an externally supplied TExecRequest to be executed by impalad. The TExecRequest must be fully populated and will be sent directly to the backend for execution. The following fields in the TExecRequest are updated by the coordinator: - Hostname - KRPC address - Local Timezone In order to add the interface to ImpalaInternalService.thrift, several of the thrift classes were moved to Query.thrift to avoid a circular dependency with Frontend.thrift. Added functionality to format and dump TExecRequest structures to path specified in debug flag dump_exec_request_path. A start timestamp field has been added to TExecRequest to represent the interval in the query profile between when the request was sent by the external frontend and handled by the backend. A local timestamp field has been added to the Ping result struct to return the current backend timestamp. This is used by the external to frontend to populate the start timestamp. Also included is a change to avoid generating silent AnalysisExceptions during table resolution. Tested with TExecRequest structures populated by external frontend. Local timezone change tested withe INT64 TIMESTAMP datatype Reviewed-by: John Sherman Change-Id: Iace716dd67290f08441857dc02d2428b0e335eaa --- M be/generated-sources/gen-cpp/CMakeLists.txt M be/src/rpc/hs2-http-test.cc M be/src/runtime/debug-options.h M be/src/runtime/query-driver.cc M be/src/runtime/query-driver.h 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 be/src/service/impala-server.h M common/thrift/CMakeLists.txt M common/thrift/Frontend.thrift M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift A common/thrift/Query.thrift M fe/pom.xml M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 21 files changed, 986 insertions(+), 755 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/17104/8 -- To view, visit http://gerrit.cloudera.org:8080/17104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iace716dd67290f08441857dc02d2428b0e335eaa Gerrit-Change-Number: 17104 Gerrit-PatchSet: 8 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-10550: Add External Frontend service port
Kurt Deschler has uploaded a new patch set (#8) to the change originally created by John Sherman. ( http://gerrit.cloudera.org:8080/17125 ) Change subject: IMPALA-10550: Add External Frontend service port .. IMPALA-10550: Add External Frontend service port - If external_fe_port flag is >0, spins up a new HS2 compatible service port - Added enable_external_fe_support option to start-impala-cluster.py - which when detected will start impala clusters with external_fe_port on 21150-21152 - Modify impalad_coordinator Dockerfile to expose external frontend port at 21150 - The intent of this commit is to separate external frontend connections from normal hs2 connections - This allows different security policy to be applied to each type of connection. The external_fe_port should be considered a privileged service and should only be exposed to an external frontend that does user authentication and does authorization checks on generated plans Change-Id: I991b5b05e12e37d8739e18ed1086bbb0228acc40 Reviewed-by: Aman Sinha --- M be/src/rpc/authentication.cc M be/src/rpc/authentication.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 bin/start-impala-cluster.py M common/thrift/metrics.json M docker/impalad_coordinator/Dockerfile M tests/common/impala_cluster.py 10 files changed, 136 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/17125/8 -- To view, visit http://gerrit.cloudera.org:8080/17125 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I991b5b05e12e37d8739e18ed1086bbb0228acc40 Gerrit-Change-Number: 17125 Gerrit-PatchSet: 8 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-10450: Catalogd crashes due to exception in ThriftDebugString
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17110 ) Change subject: IMPALA-10450: Catalogd crashes due to exception in ThriftDebugString .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8275/ : 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/17110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I42cee6186a3d5bacc1117bae5961ac60ac9f7a66 Gerrit-Change-Number: 17110 Gerrit-PatchSet: 8 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 02 Mar 2021 01:20:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10450: Catalogd crashes due to exception in ThriftDebugString
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17110 ) Change subject: IMPALA-10450: Catalogd crashes due to exception in ThriftDebugString .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6930/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/17110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I42cee6186a3d5bacc1117bae5961ac60ac9f7a66 Gerrit-Change-Number: 17110 Gerrit-PatchSet: 8 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 02 Mar 2021 01:01:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 ) Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build .. Patch Set 2: > Patch Set 2: > > (1 comment) Good to see this! I hit the same issue in GCS when working on https://gerrit.cloudera.org/c/17121 I reproduce it using a test in test_queries.py::TestQueries::test_analytic_fns SET default_spillable_buffer_size=8m; SET buffer_pool_limit=47m; SELECT lag(-180, 13) over (ORDER BY t1.int_col ASC, t2.int_col ASC) AS int_col FROM functional_parquet.alltypes t1 CROSS JOIN functional_parquet.alltypes t2 LIMIT 10; This patch fixes my issue. Maybe you can add a test similar to this and only run on S3 builds. -- To view, visit http://gerrit.cloudera.org:8080/17136 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53 Gerrit-Change-Number: 17136 Gerrit-PatchSet: 2 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 02 Mar 2021 01:01:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10450: Catalogd crashes due to exception in ThriftDebugString
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17110 ) Change subject: IMPALA-10450: Catalogd crashes due to exception in ThriftDebugString .. Patch Set 8: Code-Review+2 (1 comment) Carrying forward previous +2 from Joe and Quanlong. http://gerrit.cloudera.org:8080/#/c/17110/7/tests/custom_cluster/test_thrift_debug_string_exception.py File tests/custom_cluster/test_thrift_debug_string_exception.py: http://gerrit.cloudera.org:8080/#/c/17110/7/tests/custom_cluster/test_thrift_debug_string_exception.py@42 PS7, Line 42: est_thrift_debug_str(self): : """Sanity test which executes API call to get a catalog object and make sure th > Nit: Could you note in this comment that this is a positive test where it s Thanks for the comment. I update the documentation for the test methods as suggested. -- To view, visit http://gerrit.cloudera.org:8080/17110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I42cee6186a3d5bacc1117bae5961ac60ac9f7a66 Gerrit-Change-Number: 17110 Gerrit-PatchSet: 8 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 02 Mar 2021 01:00:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10450: Catalogd crashes due to exception in ThriftDebugString
Vihang Karajgaonkar has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/17110 ) Change subject: IMPALA-10450: Catalogd crashes due to exception in ThriftDebugString .. IMPALA-10450: Catalogd crashes due to exception in ThriftDebugString This patch adds a wrapper around ThriftDebugString method provided in the Thrift library. The thrift's method can throw exceptions like (bad_alloc or TProtocolException) when the object cannot be serialized into a string representation. This exception is not caught on the catalogd side and it crashes the catalogd. The error was specifically seen in the catalogd's debug UI which provides a way to display a Table object. An exception thrown when rendering the table on the UI would have crashed the catalogd before the patch. In order to simulate this crash a new debug action called EXCEPTION was added. A new custom cluster test was added which simulates a exception thrown in this method and makes sure that fetching the table from catalogd's debug UI does not crash the catalogd. Tests: 1. Added a new custom cluster test which reproduces the crash. Change-Id: I42cee6186a3d5bacc1117bae5961ac60ac9f7a66 --- M be/src/catalog/catalog-server.cc M be/src/util/debug-util.cc M be/src/util/debug-util.h A be/src/util/thrift-debug-util.h A tests/custom_cluster/test_thrift_debug_string_exception.py 5 files changed, 121 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/17110/8 -- To view, visit http://gerrit.cloudera.org:8080/17110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I42cee6186a3d5bacc1117bae5961ac60ac9f7a66 Gerrit-Change-Number: 17110 Gerrit-PatchSet: 8 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-10450: Catalogd crashes due to exception in ThriftDebugString
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/17110 ) Change subject: IMPALA-10450: Catalogd crashes due to exception in ThriftDebugString .. Patch Set 7: Code-Review+2 (1 comment) This makes sense to me. One small nit on a comment. Since Quanlong has already +1'd, I'm going to take this to +2. http://gerrit.cloudera.org:8080/#/c/17110/7/tests/custom_cluster/test_thrift_debug_string_exception.py File tests/custom_cluster/test_thrift_debug_string_exception.py: http://gerrit.cloudera.org:8080/#/c/17110/7/tests/custom_cluster/test_thrift_debug_string_exception.py@42 PS7, Line 42: The test executes a API call to get a catalog object from the debug UI and makes : sure that catalogd does not crash if the ThriftDebugString throws an exception. Nit: Could you note in this comment that this is a positive test where it should not throw an exception? -- To view, visit http://gerrit.cloudera.org:8080/17110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I42cee6186a3d5bacc1117bae5961ac60ac9f7a66 Gerrit-Change-Number: 17110 Gerrit-PatchSet: 7 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 02 Mar 2021 00:53:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10551: Add result sink support for external frontends
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17144 ) Change subject: IMPALA-10551: Add result sink support for external frontends .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8274/ : 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/17144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I024bf41d77bb81f1ab0debdbd31ec3687c83f072 Gerrit-Change-Number: 17144 Gerrit-PatchSet: 2 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 02 Mar 2021 00:39:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10555: Fix Hit DCHECK in TmpFileGroup::RecoverWriteError
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17140 ) Change subject: IMPALA-10555: Fix Hit DCHECK in TmpFileGroup::RecoverWriteError .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/17140 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifd9aea4bf2fff634ea9a30bf6e87987be4e1c611 Gerrit-Change-Number: 17140 Gerrit-PatchSet: 2 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 02 Mar 2021 00:34:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10551: Add result sink support for external frontends
John Sherman has posted comments on this change. (
http://gerrit.cloudera.org:8080/17144 )
Change subject: IMPALA-10551: Add result sink support for external frontends
..
Patch Set 2:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/17144/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17144/1//COMMIT_MSG@14
PS1, Line 14: - External frontends are responsible for managing the files after
the
: query is completed.
> I wonder how this would interact with the "retry_failed_queries" query opti
I think the newest version of the patch doesn't try to retry failed queries
when there is a result sink.
http://gerrit.cloudera.org:8080/#/c/17144/1/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:
http://gerrit.cloudera.org:8080/#/c/17144/1/be/src/runtime/coordinator.cc@798
PS1, Line 798: Coordinator::FinalizeResultSink()
> I might be missing something, but where is this called?
Great catch - I think some code got lost in a rebase. Will show up in next
patch (in Wait()).
http://gerrit.cloudera.org:8080/#/c/17144/1/be/src/runtime/coordinator.cc@907
PS1, Line 907: } else if (stmt_type_ == TStmtType::QUERY) {
: DCHECK(coord_instance_ != nullptr);
> I'm thinking through the implication of calling this after WaitForBackends(
I think a delay in reporting an error is okay for now (As long as that is the
only negative in thee way I am doing this)
I will also take some time to think it through a bit since I haven't thought
about this code for awhile.
http://gerrit.cloudera.org:8080/#/c/17144/2/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:
http://gerrit.cloudera.org:8080/#/c/17144/2/be/src/runtime/coordinator.cc@897
PS2, Line 897: // than streaming them to a client directly. For these types
of queries we need to wait
> line too long (91 > 90)
I'll catch in next round of fixes.
http://gerrit.cloudera.org:8080/#/c/17144/1/be/src/runtime/fragment-instance-state.h
File be/src/runtime/fragment-instance-state.h:
http://gerrit.cloudera.org:8080/#/c/17144/1/be/src/runtime/fragment-instance-state.h@121
PS1, Line 121: JoinBuilder* GetJoinBuildSink() const;
> Add CR
Done
http://gerrit.cloudera.org:8080/#/c/17144/1/fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
File fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java:
http://gerrit.cloudera.org:8080/#/c/17144/1/fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java@245
PS1, Line 245: exprs.addAll(outputExprs_.subList(0,
> line too long (91 > 90)
Done
--
To view, visit http://gerrit.cloudera.org:8080/17144
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I024bf41d77bb81f1ab0debdbd31ec3687c83f072
Gerrit-Change-Number: 17144
Gerrit-PatchSet: 2
Gerrit-Owner: John Sherman
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: John Sherman
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Tue, 02 Mar 2021 00:23:26 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 ) Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/17136/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17136/2//COMMIT_MSG@23 PS2, Line 23: Set the default fs as a s3 path, reran Could you add an unit-test? -- To view, visit http://gerrit.cloudera.org:8080/17136 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53 Gerrit-Change-Number: 17136 Gerrit-PatchSet: 2 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 02 Mar 2021 00:22:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10551: Add result sink support for external frontends
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17144 ) Change subject: IMPALA-10551: Add result sink support for external frontends .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/17144/2/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/17144/2/be/src/runtime/coordinator.cc@897 PS2, Line 897: // than streaming them to a client directly. For these types of queries we need to wait line too long (91 > 90) -- To view, visit http://gerrit.cloudera.org:8080/17144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I024bf41d77bb81f1ab0debdbd31ec3687c83f072 Gerrit-Change-Number: 17144 Gerrit-PatchSet: 2 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 02 Mar 2021 00:19:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10551: Add result sink support for external frontends
Hello Aman Sinha, Thomas Tauber-Marshall, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17144 to look at the new patch set (#2). Change subject: IMPALA-10551: Add result sink support for external frontends .. IMPALA-10551: Add result sink support for external frontends - The intended purpose of these changes is to allow external frontends to receive query results via files rather than streaming the results through the thrift interface. - External frontends are expected to provide an FeFsTable implementation that describes the desired location to store results. - External frontends are responsible for managing the files after the query is completed. - Testing has been manual and through an implementation of an external frontend. Change-Id: I024bf41d77bb81f1ab0debdbd31ec3687c83f072 Reviewed-by: Aman Sinha --- M be/src/exec/hdfs-table-sink.cc M be/src/exec/hdfs-table-sink.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M common/thrift/DataSinks.thrift M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/TableSink.java 9 files changed, 111 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/17144/2 -- To view, visit http://gerrit.cloudera.org:8080/17144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I024bf41d77bb81f1ab0debdbd31ec3687c83f072 Gerrit-Change-Number: 17144 Gerrit-PatchSet: 2 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 ) Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/17136/2/be/src/runtime/tmp-file-mgr.cc File be/src/runtime/tmp-file-mgr.cc: http://gerrit.cloudera.org:8080/#/c/17136/2/be/src/runtime/tmp-file-mgr.cc@663 PS2, Line 663: -1, true, false When you have long parameter lists with constants where its not clear what they correspond to, you can make it more readable by adding the parameter names in /* */, i.e. this line would become: return file_group_->io_mgr_->AssignQueue(local_buffer_path_.c_str(), /* disk_id */ -1, /* expected_local */ true, /* check_default_fs */ false); and you can do the same for the other lines, here and in scan-range.cc, where you're passing constants in as parameters -- To view, visit http://gerrit.cloudera.org:8080/17136 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53 Gerrit-Change-Number: 17136 Gerrit-PatchSet: 2 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 02 Mar 2021 00:14:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10524: Changes to HdfsPartition for third party extensions.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17092 ) Change subject: IMPALA-10524: Changes to HdfsPartition for third party extensions. .. Patch Set 12: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/17092 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a792642f27228118ac8f2e8ef98e8ba7aee4a46 Gerrit-Change-Number: 17092 Gerrit-PatchSet: 12 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Tue, 02 Mar 2021 00:11:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10524: Changes to HdfsPartition for third party extensions.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17092 ) Change subject: IMPALA-10524: Changes to HdfsPartition for third party extensions. .. Patch Set 12: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6929/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/17092 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a792642f27228118ac8f2e8ef98e8ba7aee4a46 Gerrit-Change-Number: 17092 Gerrit-PatchSet: 12 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Tue, 02 Mar 2021 00:11:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10524: Changes to HdfsPartition for third party extensions.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17092 ) Change subject: IMPALA-10524: Changes to HdfsPartition for third party extensions. .. Patch Set 11: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/17092 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a792642f27228118ac8f2e8ef98e8ba7aee4a46 Gerrit-Change-Number: 17092 Gerrit-PatchSet: 11 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Tue, 02 Mar 2021 00:10:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10535: Add interface to ImpalaServer for execution of externally compiled statements
Thomas Tauber-Marshall has posted comments on this change. (
http://gerrit.cloudera.org:8080/17104 )
Change subject: IMPALA-10535: Add interface to ImpalaServer for execution of
externally compiled statements
..
Patch Set 7:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/service/impala-hs2-server.cc@458
PS7, Line 458: execRequest
nit: exec_request (here and several other places in this patch)
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/service/impala-server.h
File be/src/service/impala-server.h:
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/service/impala-server.h@678
PS7, Line 678: /// have been checked out.
comment might benefit from a brief mention of what 'exec_request' is
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/service/impala-server.cc@319
PS7, Line 319:
"TExecRequest-{internal|external}.{query_id.hi}-{query_id.lo}");
might be worth explicitly saying this is for debugging
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/service/impala-server.cc@1114
PS7, Line 1114: if (FLAGS_dump_exec_request_path.empty())
: return;
nit: this can go on one line (and if it couldn't, Impala always uses braces
around blocks)
Some of these formatting issues can be avoided if you check out
clang-format-diff, see:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=65868536
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/service/impala-server.cc@1202
PS7, Line 1202: // Update the externally provided exec_request with Impala's
query_id()
: (*query_handle)->SetExecRequestQueryId(query_id);
: // Update coordinator related internal addresses
: (*query_handle)->SetExecRequestHostname(
:
ExecEnv::GetInstance()->configured_backend_address().hostname);
:
(*query_handle)->SetExecRequestKrpcAddress(ExecEnv::GetInstance()->krpc_address());
: // Update the field of 'local_time_zone' of 'query_ctx'.
:
(*query_handle)->SetExecRequestLocalTimeZone(query_ctx.local_time_zone);
might be nice to move this into QueryDriver::SetExternalPlan()
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/service/impala-server.cc@1213
PS7, Line 1213:
RETURN_IF_ERROR((*query_handle)->UpdateQueryStatus(exec_status));
What's the point of this call? I think it will always be called with OK and not
do anything
--
To view, visit http://gerrit.cloudera.org:8080/17104
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iace716dd67290f08441857dc02d2428b0e335eaa
Gerrit-Change-Number: 17104
Gerrit-PatchSet: 7
Gerrit-Owner: Kurt Deschler
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: John Sherman
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Mon, 01 Mar 2021 23:56:42 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9856: Enable result spooling by default.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16755 ) Change subject: IMPALA-9856: Enable result spooling by default. .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6928/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/16755 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e360c1428676d8f3fab5d95efee18aca085eba4 Gerrit-Change-Number: 16755 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, 01 Mar 2021 23:13:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9856: Enable result spooling by default.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16755 ) Change subject: IMPALA-9856: Enable result spooling by default. .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16755 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e360c1428676d8f3fab5d95efee18aca085eba4 Gerrit-Change-Number: 16755 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, 01 Mar 2021 23:13:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9856: Enable result spooling by default.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/16755 ) Change subject: IMPALA-9856: Enable result spooling by default. .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16755 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e360c1428676d8f3fab5d95efee18aca085eba4 Gerrit-Change-Number: 16755 Gerrit-PatchSet: 4 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, 01 Mar 2021 23:12:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10551: Add result sink support for external frontends
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/17144 ) Change subject: IMPALA-10551: Add result sink support for external frontends .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/17144/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17144/1//COMMIT_MSG@14 PS1, Line 14: - External frontends are responsible for managing the files after the : query is completed. I wonder how this would interact with the "retry_failed_queries" query option. That can reexecute a failed statement if no rows were fetched. Depending on how files are cleaned up, this could be a problem. We may want to throw an error if using this result sink when retry_failed_queries = True until we can test it. http://gerrit.cloudera.org:8080/#/c/17144/1/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/17144/1/be/src/runtime/coordinator.cc@798 PS1, Line 798: Coordinator::FinalizeResultSink() I might be missing something, but where is this called? http://gerrit.cloudera.org:8080/#/c/17144/1/be/src/runtime/coordinator.cc@907 PS1, Line 907: RETURN_IF_ERROR(UpdateExecState(coord_instance_->WaitForOpen(), : &coord_instance_->runtime_state()->fragment_instance_id(), FLAGS_hostname)); I'm thinking through the implication of calling this after WaitForBackends() for QUERY statements. I think it would delay the detection of certain types of errors, but I'm not sure if there are other problems. -- To view, visit http://gerrit.cloudera.org:8080/17144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I024bf41d77bb81f1ab0debdbd31ec3687c83f072 Gerrit-Change-Number: 17144 Gerrit-PatchSet: 1 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 23:01:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17146 ) Change subject: IMPALA-10522: Support external use of frontend libraries .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8273/ : 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/17146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I51775ef13acbdca02531ddb37296890675a0a2b9 Gerrit-Change-Number: 17146 Gerrit-PatchSet: 1 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 01 Mar 2021 22:52:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10550: Add External Frontend service port
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17125 ) Change subject: IMPALA-10550: Add External Frontend service port .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8272/ : 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/17125 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I991b5b05e12e37d8739e18ed1086bbb0228acc40 Gerrit-Change-Number: 17125 Gerrit-PatchSet: 6 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 22:47:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10550: Add External Frontend service port
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17125 ) Change subject: IMPALA-10550: Add External Frontend service port .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/17125 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I991b5b05e12e37d8739e18ed1086bbb0228acc40 Gerrit-Change-Number: 17125 Gerrit-PatchSet: 7 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 22:40:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10550: Add External Frontend service port
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17125 ) Change subject: IMPALA-10550: Add External Frontend service port .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6927/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/17125 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I991b5b05e12e37d8739e18ed1086bbb0228acc40 Gerrit-Change-Number: 17125 Gerrit-PatchSet: 7 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 22:40:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10550: Add External Frontend service port
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/17125 ) Change subject: IMPALA-10550: Add External Frontend service port .. Patch Set 6: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/17125/5/be/src/rpc/authentication.h File be/src/rpc/authentication.h: http://gerrit.cloudera.org:8080/#/c/17125/5/be/src/rpc/authentication.h@67 PS5, Line 67: : AuthProvider* GetEx > I totally did - usually I self review a little better. Thanks for catching. No worries -- To view, visit http://gerrit.cloudera.org:8080/17125 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I991b5b05e12e37d8739e18ed1086bbb0228acc40 Gerrit-Change-Number: 17125 Gerrit-PatchSet: 6 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 22:37:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Kurt Deschler has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17146 Change subject: IMPALA-10522: Support external use of frontend libraries .. IMPALA-10522: Support external use of frontend libraries This patch enables the Impala frontend jar and dependent library libfesupport.so to be used by an external Java frontend. Calling FeSupport.setExternalFE() will cause external frontend initialization mode to be used during FeSupport.loadLibrary(). This mode builds upon logic that is used to initialize the frontend jar for unit tests. Initialization in external frontend mode differs as follows: - Skip instantiating Frontend object and it's dependents - Skip loading libhdfs - Skip starting JVM Pause monitor - Disable Minidumper - Initialize TimezoneDatabase for external frontends - Disable redirect of stderr/stdout to libfesupport.so glog - Log messages from libfesupport.so to stderr - Use libfesupport.so for JNI symbol look up Null check were added in places where objects were assumed to be instantiated but are now skipped during initialization. Additional change: 1) Add libfesupport.lib path to JAVA_LIBRARY_PATH in test driver Testing: - Initialized frontend jar from external frontend - Verified that frontend Java objects can be used externally without issues - Verified that exceptions thrown from Impala Java or libfesupport can be caught or propagated correctly by the external frontend - Manual verification of minicluster logs - Ran queries with external frontend Co-authored-by: John Sherman Co-authored-by: Aman Sinha Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9 temp Change-Id: I51775ef13acbdca02531ddb37296890675a0a2b9 --- M be/src/benchmarks/expr-benchmark.cc M be/src/common/init.cc M be/src/common/init.h M be/src/runtime/data-stream-test.cc M be/src/runtime/exec-env.cc M be/src/runtime/exec-env.h M be/src/runtime/lib-cache.cc M be/src/runtime/lib-cache.h M be/src/service/fe-support.cc M be/src/util/jni-util.cc M fe/src/main/java/org/apache/impala/service/FeSupport.java M testdata/bin/run-hive-server.sh 12 files changed, 99 insertions(+), 46 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/17146/1 -- To view, visit http://gerrit.cloudera.org:8080/17146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I51775ef13acbdca02531ddb37296890675a0a2b9 Gerrit-Change-Number: 17146 Gerrit-PatchSet: 1 Gerrit-Owner: Kurt Deschler
[Impala-ASF-CR] IMPALA-10550: Add External Frontend service port
John Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/17125 ) Change subject: IMPALA-10550: Add External Frontend service port .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/17125/5/be/src/rpc/authentication.h File be/src/rpc/authentication.h: http://gerrit.cloudera.org:8080/#/c/17125/5/be/src/rpc/authentication.h@67 PS5, Line 67: Currently this is either null if external_fe_port <= 0 or : /// NoAuthProvider. > I think you put this above the wrong function? I totally did - usually I self review a little better. Thanks for catching. http://gerrit.cloudera.org:8080/#/c/17125/5/bin/start-impala-cluster.py File bin/start-impala-cluster.py: http://gerrit.cloudera.org:8080/#/c/17125/5/bin/start-impala-cluster.py@235 PS5, Line 235: 'external_fe_port': DEFAULT_EXTERNAL_FE_PORT + instance_num, > What would you think about putting this behind a flag for now, eg. "start-i Done I went with the approach of excluding it in build_impalad_port_args since that seemed like a nice clean way of excluding it even if the map contains the port. -- To view, visit http://gerrit.cloudera.org:8080/17125 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I991b5b05e12e37d8739e18ed1086bbb0228acc40 Gerrit-Change-Number: 17125 Gerrit-PatchSet: 5 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 22:26:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10550: Add External Frontend service port
Hello Aman Sinha, Thomas Tauber-Marshall, Kurt Deschler, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17125 to look at the new patch set (#6). Change subject: IMPALA-10550: Add External Frontend service port .. IMPALA-10550: Add External Frontend service port - If external_fe_port flag is >0, spins up a new HS2 compatible service port - Added enable_external_fe_support option to start-impala-cluster.py - which when detected will start impala clusters with external_fe_port on 21150-21152 - Modify impalad_coordinator Dockerfile to expose external frontend port at 21150 - The intent of this commit is to separate external frontend connections from normal hs2 connections - This allows different security policy to be applied to each type of connection. The external_fe_port should be considered a privileged service and should only be exposed to an external frontend that does user authentication and does authorization checks on generated plans Change-Id: I991b5b05e12e37d8739e18ed1086bbb0228acc40 Reviewed-by: Aman Sinha --- M be/src/rpc/authentication.cc M be/src/rpc/authentication.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 bin/start-impala-cluster.py M common/thrift/metrics.json M docker/impalad_coordinator/Dockerfile M tests/common/impala_cluster.py 10 files changed, 136 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/17125/6 -- To view, visit http://gerrit.cloudera.org:8080/17125 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I991b5b05e12e37d8739e18ed1086bbb0228acc40 Gerrit-Change-Number: 17125 Gerrit-PatchSet: 6 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-10524: Changes to HdfsPartition for third party extensions.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17092 ) Change subject: IMPALA-10524: Changes to HdfsPartition for third party extensions. .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8271/ : 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/17092 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a792642f27228118ac8f2e8ef98e8ba7aee4a46 Gerrit-Change-Number: 17092 Gerrit-PatchSet: 11 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Mon, 01 Mar 2021 22:21:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10524: Changes to HdfsPartition for third party extensions.
Steve Carlin has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/17092 ) Change subject: IMPALA-10524: Changes to HdfsPartition for third party extensions. .. IMPALA-10524: Changes to HdfsPartition for third party extensions. Some changes are needed to HdfsPartition and other related classes to allow for third party extensions. These changes include: - A protected constructor which will allow a subclass to instantiate HdfsPartition using its own Builder. - Various changes of permissions to methods and variables to allow third party extension visibility. - Creation of the getHostIndex() method to allow the subclass to override how the hostIndexes are retrieved. - Added a new default method "getFileSystem()" to FeFsPartition which will allow the third party extension to override how the filesystem is obtained from the partition object. Change-Id: I5a792642f27228118ac8f2e8ef98e8ba7aee4a46 --- M fe/src/main/java/org/apache/impala/analysis/AnalyticWindow.java M fe/src/main/java/org/apache/impala/analysis/StatementBase.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartitionLocationCompressor.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java M fe/src/main/java/org/apache/impala/planner/CardinalityCheckNode.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java M fe/src/main/java/org/apache/impala/planner/PlanNode.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/SortNode.java M fe/src/main/java/org/apache/impala/planner/SubplanNode.java 14 files changed, 82 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/92/17092/11 -- To view, visit http://gerrit.cloudera.org:8080/17092 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5a792642f27228118ac8f2e8ef98e8ba7aee4a46 Gerrit-Change-Number: 17092 Gerrit-PatchSet: 11 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Steve Carlin
[Impala-ASF-CR] IMPALA-10494: Making use of the min/max column stats to improve min/max filters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17075 ) Change subject: IMPALA-10494: Making use of the min/max column stats to improve min/max filters .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8270/ : 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/17075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I08581b44419bb8da5940cbf98502132acd1c86df Gerrit-Change-Number: 17075 Gerrit-PatchSet: 11 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Comment-Date: Mon, 01 Mar 2021 21:33:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10494: Making use of the min/max column stats to improve min/max filters
Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/17075 ) Change subject: IMPALA-10494: Making use of the min/max column stats to improve min/max filters .. Patch Set 11: Also add the query option compute_column_min_max_stats. Default to 'false'. -- To view, visit http://gerrit.cloudera.org:8080/17075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I08581b44419bb8da5940cbf98502132acd1c86df Gerrit-Change-Number: 17075 Gerrit-PatchSet: 11 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Comment-Date: Mon, 01 Mar 2021 21:14:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10494: Making use of the min/max column stats to improve min/max filters
Qifan Chen has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/17075 ) Change subject: IMPALA-10494: Making use of the min/max column stats to improve min/max filters .. IMPALA-10494: Making use of the min/max column stats to improve min/max filters This patch adds the functionality to compute the minimal and the maximal value for a column of type integer and double for parquet tables, and to make use of the new stats to discard min/max filters whose coverage are too close to the actual range. Only the min/max values for non-partition columns are stored in HMS. The min/max values for partition columns are computed in coordinator. Two new columns 'Min' and 'Max' are added in the output of the show column stats command as shown below. show column stats tpcds_parquet.store_sales +---+--+-...---+-+-+ | Column| Type | #Falses | Min | Max | +---+--+-...---+-+-+ | ss_sold_time_sk | INT | -1 | 28800 | 75599 | | ss_item_sk| BIGINT | -1 | 1 | 18000 | | ss_customer_sk| INT | -1 | 1 | 10 | | ss_cdemo_sk | INT | -1 | 15 | 1920797 | | ss_hdemo_sk | INT | -1 | 1 | 7200| | ss_addr_sk| INT | -1 | 1 | 5 | | ss_store_sk | INT | -1 | 1 | 10 | | ss_promo_sk | INT | -1 | 1 | 300 | | ss_ticket_number | BIGINT | -1 | 1 | 24 | | ss_quantity | INT | -1 | 1 | 100 | | ss_wholesale_cost | DECIMAL(7,2) | -1 | -1 | -1 | | ss_list_price | DECIMAL(7,2) | -1 | -1 | -1 | | ss_sales_price| DECIMAL(7,2) | -1 | -1 | -1 | | ss_ext_discount_amt | DECIMAL(7,2) | -1 | -1 | -1 | | ss_ext_sales_price| DECIMAL(7,2) | -1 | -1 | -1 | | ss_ext_wholesale_cost | DECIMAL(7,2) | -1 | -1 | -1 | | ss_ext_list_price | DECIMAL(7,2) | -1 | -1 | -1 | | ss_ext_tax| DECIMAL(7,2) | -1 | -1 | -1 | | ss_coupon_amt | DECIMAL(7,2) | -1 | -1 | -1 | | ss_net_paid | DECIMAL(7,2) | -1 | -1 | -1 | | ss_net_paid_inc_tax | DECIMAL(7,2) | -1 | -1 | -1 | | ss_net_profit | DECIMAL(7,2) | -1 | -1 | -1 | | ss_sold_date_sk | INT | -1 | 2450816 | 2452642 | +---+--+-...---+-+-+ Testing: - Added TestLowAndHighValueShort and TestLowAndHighValueInt to IncrStatsUtilTest TODO: 1. Test compute stats for timestamp and date columns; 2. Test filters being disabled at the scan node; 3. Add logic to disable min/max filters inside HJ builder via the column stats. Change-Id: I08581b44419bb8da5940cbf98502132acd1c86df --- M be/src/exec/catalog-op-executor.cc M be/src/exec/filter-context.cc M be/src/exec/filter-context.h M be/src/exec/hdfs-scanner.h M be/src/exec/incr-stats-util-test.cc M be/src/exec/incr-stats-util.cc M be/src/exec/incr-stats-util.h M be/src/exec/parquet/hdfs-parquet-scanner.cc M be/src/service/hs2-util.cc M be/src/service/hs2-util.h M be/src/service/query-options.cc M be/src/service/query-options.h M be/src/util/min-max-filter.h M common/thrift/CatalogObjects.thrift M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M common/thrift/PlanNodes.thrift M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/catalog/ColumnStats.java M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java 25 files changed, 827 insertions(+), 51 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/17075/11 -- To view, visit http://gerrit.cloudera.org:8080/17075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I08581b44419bb8da5940cbf98502132acd1c86df Gerrit-Change-Number: 17075 Gerrit-PatchSet: 11 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen
[Impala-ASF-CR] IMPALA-10535: Add interface to ImpalaServer for execution of externally compiled statements
Joe McDonnell has posted comments on this change. (
http://gerrit.cloudera.org:8080/17104 )
Change subject: IMPALA-10535: Add interface to ImpalaServer for execution of
externally compiled statements
..
Patch Set 7:
(7 comments)
Posting comments that I have so far
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/runtime/query-driver.cc
File be/src/runtime/query-driver.cc:
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/runtime/query-driver.cc@67
PS7, Line 67: exec_request
Nit: I think it would be clearer to call this 'external_exec_request'
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/service/impala-server.cc@1118
PS7, Line 1118: FLAGS_dump_exec_request_path + "/TExecRequest-" + dumpType +
"." +
: std::to_string(queryID.hi) + "-" +
std::to_string(queryID.lo)
For constructing the filename, I think it would be cleaner to use the
Substitute() function that takes a template string and args:
https://github.com/apache/impala/blob/master/be/src/gutil/strings/substitute.h#L172
There are several uses in this file to consult.
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/service/impala-server.cc@1140
PS7, Line 1140: exec_request
It might be clearer for this to be 'external_exec_request' to emphasize that
this is ordinarily null.
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/service/impala-server.cc@1191
PS7, Line 1191: exec_request
Nit: From a style point, we prefer explicit checks against nullptr
("exec_request != nullptr") rather than implicit non-zero checks of pointer
values.
http://gerrit.cloudera.org:8080/#/c/17104/7/be/src/service/impala-server.cc@1198
PS7, Line 1198: exec_request
Nit: Same here (use explicit nullptr checks)
http://gerrit.cloudera.org:8080/#/c/17104/7/common/thrift/CMakeLists.txt
File common/thrift/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/17104/7/common/thrift/CMakeLists.txt@70
PS7, Line 70: # Also do not generate ImpalaService.thrift because the
generated code doesn't
: # compile with hive if the thrift version in hive is 0.9.0
Nit: We can remove this part of the comment
http://gerrit.cloudera.org:8080/#/c/17104/7/fe/pom.xml
File fe/pom.xml:
http://gerrit.cloudera.org:8080/#/c/17104/7/fe/pom.xml@434
PS7, Line 434: org.apache.hive
: hive-classification
: ${hive.version}
Does this bring in any dependencies that we need to exclude?
If you haven't already, it's worth checking mvn dependency:tree or diffing the
fe/target/build-classpath.txt to see if this is adding anything.
--
To view, visit http://gerrit.cloudera.org:8080/17104
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iace716dd67290f08441857dc02d2428b0e335eaa
Gerrit-Change-Number: 17104
Gerrit-PatchSet: 7
Gerrit-Owner: Kurt Deschler
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: John Sherman
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Mon, 01 Mar 2021 20:59:42 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10550: Add External Frontend service port
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/17125 ) Change subject: IMPALA-10550: Add External Frontend service port .. Patch Set 5: (2 comments) > > (2 comments) > > > > Are there any existing hs2 methods that it might make sense to > > block for the "external frontend" server? eg. we might want to > > return an error for ExecuteStatement() from it if the external > > frontend will never need to call it. > > > > That would give us a little more safety in case people > accidentally > > expose this port to the outside world (of course, it would still > be > > possible for bad actors to use the ExecutePlannedStatement > > interface, but its probably a lot harder to put together a valid > > TExecRequest to use it than it is to put together a SQL string > like > > ExecuteStatement takes) > > > > Also out of curiosity - what's the long run testing plan here? > Are > > we going to have an actual external FE running in the minicluster > > that can exercise this stuff? > > So the current implementation of external frontend does utilize the > ExecuteStatement functionality (for things like COMPUTE STATS). I > do agree with your assessment that it would be nice to reduce the > surface area in the future. The long term plan would also likely > include enabling similar protections that intra-impalad > communication use between nodes (that prevent people connecting > easily to the backend port and pretending to be a coordinator). > > One option I considered based on your comment was to add a 2nd flag > that would be named something like: external_fe_allow_unsafe which > defaulted to false and disallowed ExecuteStatement via the > external_fe_port. So a user would have to enable external_fe_port > AND set external_fe_allow_unsafe to true to be able to call > ExecuteStatement. But if someone is enabling the external_fe_port - > it is somewhat assumed they know what they are doing so I'm not > 100% convinced this approach is worth it. I am open to suggestions > (or if you like the idea of the 2nd flag). Agreed that sounds unnecessarily complicated. I think its fine as-is for now, just something to keep in mind. > > As for testing - I do believe once the various external FE commits > land we should focus on: > 1) auditing and shoring up what we can build unit tests around > 2) And, yes, it is my understanding that we will eventually be > including an external frontend in the minicluster for more > end-to-end testing. Otherwise, we will need to mock up some sort of > "send pre-made exec request" and "check response" test framework > but I suspect that might be not fun to implement cleanly. Sounds good http://gerrit.cloudera.org:8080/#/c/17125/5/be/src/rpc/authentication.h File be/src/rpc/authentication.h: http://gerrit.cloudera.org:8080/#/c/17125/5/be/src/rpc/authentication.h@67 PS5, Line 67: Currently this is either null if external_fe_port <= 0 or : /// NoAuthProvider. I think you put this above the wrong function? http://gerrit.cloudera.org:8080/#/c/17125/5/bin/start-impala-cluster.py File bin/start-impala-cluster.py: http://gerrit.cloudera.org:8080/#/c/17125/5/bin/start-impala-cluster.py@235 PS5, Line 235: 'external_fe_port': DEFAULT_EXTERNAL_FE_PORT + instance_num, What would you think about putting this behind a flag for now, eg. "start-impala-cluster.py --enable_external_fe_support" or similar? -- To view, visit http://gerrit.cloudera.org:8080/17125 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I991b5b05e12e37d8739e18ed1086bbb0228acc40 Gerrit-Change-Number: 17125 Gerrit-PatchSet: 5 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 20:54:06 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17116 ) Change subject: IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8269/ : 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/17116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I14a3cee29f1fc91f4431b7ea89053bb3fbfa5e69 Gerrit-Change-Number: 17116 Gerrit-PatchSet: 12 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 20:42:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17116 ) Change subject: IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/17116/12/tests/hs2/test_hs2.py File tests/hs2/test_hs2.py: http://gerrit.cloudera.org:8080/#/c/17116/12/tests/hs2/test_hs2.py@743 PS12, Line 743: _ flake8: E501 line too long (96 > 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/17116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I14a3cee29f1fc91f4431b7ea89053bb3fbfa5e69 Gerrit-Change-Number: 17116 Gerrit-PatchSet: 12 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 20:09:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad
Hello Thomas Tauber-Marshall, Joe McDonnell, John Sherman, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17116 to look at the new patch set (#12). Change subject: IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad .. IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad This patch add a new interface ImpalaServer::GetBackendConfig() that returns the current TBackendGflags from impalad. Testing: Called new interface from external frontend. Verified that TBackendGflags were populated correctly. Reviewed-by: John Sherman Change-Id: I14a3cee29f1fc91f4431b7ea89053bb3fbfa5e69 --- M be/src/catalog/catalog.cc M be/src/rpc/hs2-http-test.cc M be/src/service/frontend.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.h M be/src/util/backend-gflag-util.cc M be/src/util/backend-gflag-util.h M be/src/util/logging-support.cc M common/thrift/ImpalaService.thrift M tests/common/impala_test_suite.py M tests/conftest.py M tests/hs2/hs2_test_suite.py M tests/hs2/test_hs2.py 13 files changed, 84 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/17116/12 -- To view, visit http://gerrit.cloudera.org:8080/17116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I14a3cee29f1fc91f4431b7ea89053bb3fbfa5e69 Gerrit-Change-Number: 17116 Gerrit-PatchSet: 12 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Thomas Tauber-Marshall has posted comments on this change. (
http://gerrit.cloudera.org:8080/17115 )
Change subject: IMPALA-10522: Support external use of frontend libraries
..
Patch Set 8:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/runtime/exec-env.h
File be/src/runtime/exec-env.h:
http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/runtime/exec-env.h@82
PS8, Line 82: class ExecEnv {
Might be nice to flesh out the class comment here a little about what it means
to have an ExecEnv that's for an external FE, eg. "Not everything is going to
be initialized, just at least the stuff needed to do the FeSupport functions.
TODO: separate this out better"
http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:
http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/runtime/exec-env.cc@253
PS8, Line 253: frontend_(external_fe ? nullptr : new Frontend()),
Its a bit tricky to trace through and prove that the variables here that aren't
getting initialized anymore (frontend_ and default_fs_ below) aren't actually
used in any of the FeSupport functionality.
You could store a 'external_fe_' variable and DCHECK on it when those things
are accessed, for a little extra safety.
http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/service/fe-support.cc
File be/src/service/fe-support.cc:
http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/service/fe-support.cc@83
PS8, Line 83: InitForFeTests
This function name is unfortunate now. Not sure what a better options is -
InitForFeSupport() maybe?
http://gerrit.cloudera.org:8080/#/c/17115/8/fe/src/main/java/org/apache/impala/service/FeSupport.java
File fe/src/main/java/org/apache/impala/service/FeSupport.java:
http://gerrit.cloudera.org:8080/#/c/17115/8/fe/src/main/java/org/apache/impala/service/FeSupport.java@489
PS8, Line 489: public static void loadLibrary() {
I'm a little confused by this function and the 'externalFE_' variable - I guess
the expected usage is that the external FE could call "loadLibrary(true)"
first, and then subsequent calls to "loadLibrary()", eg. in functions in this
file like CacheJar(), LookupSymbol(), etc. will get the "externalFE" version of
this function (though that won't actually matter, as 'loaded_' would have to be
true in those cases)?
Maybe it would be clearer to have a "setExternalFE()" function that the
external FE can call, which sets 'externalFE_' and which is required to be
called before any calls to loadLibrary(). You could presumably check that
'loaded_' is false in setExternalFE() for a little extra safety.
--
To view, visit http://gerrit.cloudera.org:8080/17115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
Gerrit-Change-Number: 17115
Gerrit-PatchSet: 8
Gerrit-Owner: Kurt Deschler
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: John Sherman
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Mon, 01 Mar 2021 19:50:17 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10552: External Frontend CTAS support
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17145 ) Change subject: IMPALA-10552: External Frontend CTAS support .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8268/ : 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/17145 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iae0ea4a832d8281c563427d0d7da1623bfce437b Gerrit-Change-Number: 17145 Gerrit-PatchSet: 1 Gerrit-Owner: John Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 18:53:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10551: Add result sink support for external frontends
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17144 ) Change subject: IMPALA-10551: Add result sink support for external frontends .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8267/ : 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/17144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I024bf41d77bb81f1ab0debdbd31ec3687c83f072 Gerrit-Change-Number: 17144 Gerrit-PatchSet: 1 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 18:52:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10552: External Frontend CTAS support
John Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/17145 ) Change subject: IMPALA-10552: External Frontend CTAS support .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/17145/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17145/1//COMMIT_MSG@22 PS1, Line 22: Add co-author for Kurt -- To view, visit http://gerrit.cloudera.org:8080/17145 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iae0ea4a832d8281c563427d0d7da1623bfce437b Gerrit-Change-Number: 17145 Gerrit-PatchSet: 1 Gerrit-Owner: John Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 17:12:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10552: External Frontend CTAS support
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/17145 )
Change subject: IMPALA-10552: External Frontend CTAS support
..
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/17145/1/be/src/exec/hdfs-table-sink.cc
File be/src/exec/hdfs-table-sink.cc:
http://gerrit.cloudera.org:8080/#/c/17145/1/be/src/exec/hdfs-table-sink.cc@131
PS1, Line 131: staging_dir_ = Substitute("$0/_impala_insert_staging/$1",
table_desc_->hdfs_base_dir(),
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/17145/1/be/src/exec/hdfs-table-sink.cc@232
PS1, Line 232: void HdfsTableSink::BuildHdfsFileNames(const
HdfsPartitionDescriptor& partition_descriptor,
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/17145/1/be/src/exec/hdfs-table-sink.cc@506
PS1, Line 506: bool external_part = HasExternalStagingDir() && j >=
external_staging_partition_depth_;
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/17145/1/be/src/exec/hdfs-table-sink.cc@549
PS1, Line 549: BuildHdfsFileNames(partition_descriptor, output_partition,
external_partition_name_ss.str());
line too long (95 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/17145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae0ea4a832d8281c563427d0d7da1623bfce437b
Gerrit-Change-Number: 17145
Gerrit-PatchSet: 1
Gerrit-Owner: John Sherman
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: John Sherman
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Mon, 01 Mar 2021 17:10:17 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10551: Add result sink support for external frontends
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17144 ) Change subject: IMPALA-10551: Add result sink support for external frontends .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/17144/1/fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java File fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java: http://gerrit.cloudera.org:8080/#/c/17144/1/fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java@245 PS1, Line 245: exprs.addAll(outputExprs_.subList(0, targetTable_.getNonClusteringColumns().size())); line too long (91 > 90) -- To view, visit http://gerrit.cloudera.org:8080/17144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I024bf41d77bb81f1ab0debdbd31ec3687c83f072 Gerrit-Change-Number: 17144 Gerrit-PatchSet: 1 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 17:09:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10552: External Frontend CTAS support
John Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/17145 ) Change subject: IMPALA-10552: External Frontend CTAS support .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/17145/1/be/src/exec/hdfs-table-sink.h File be/src/exec/hdfs-table-sink.h: http://gerrit.cloudera.org:8080/#/c/17145/1/be/src/exec/hdfs-table-sink.h@249 PS1, Line 249: delete -- To view, visit http://gerrit.cloudera.org:8080/17145 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iae0ea4a832d8281c563427d0d7da1623bfce437b Gerrit-Change-Number: 17145 Gerrit-PatchSet: 1 Gerrit-Owner: John Sherman Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Comment-Date: Mon, 01 Mar 2021 17:09:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10552: External Frontend CTAS support
Hello Kurt Deschler, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/17145 to review the following change. Change subject: IMPALA-10552: External Frontend CTAS support .. IMPALA-10552: External Frontend CTAS support - Adds the concept of an external staging dir to HdfsTableSink - This allows an external to specify the destination of the sink - When this is set, the external frontend is responsible for for moving and managing the results - External frontends may optionally supply a partition depth which acts as a hint to skip a certain number of partitions while creating directories for partitions. This is for when the external frontend has pre-created a certain number of the directories in staging (usually the static portion of a partition specification)/ - Modifies delta/base naming to include 0 prefix padding to match Hive for dynamic partitioning detection Change-Id: Iae0ea4a832d8281c563427d0d7da1623bfce437b Reviewed-by: Kurt Deschler --- M be/src/exec/hdfs-table-sink.cc M be/src/exec/hdfs-table-sink.h M common/thrift/DataSinks.thrift M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java 5 files changed, 124 insertions(+), 22 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/17145/1 -- To view, visit http://gerrit.cloudera.org:8080/17145 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iae0ea4a832d8281c563427d0d7da1623bfce437b Gerrit-Change-Number: 17145 Gerrit-PatchSet: 1 Gerrit-Owner: John Sherman Gerrit-Reviewer: Kurt Deschler
[Impala-ASF-CR] IMPALA-10551: Add result sink support for external frontends
Hello Aman Sinha, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/17144 to review the following change. Change subject: IMPALA-10551: Add result sink support for external frontends .. IMPALA-10551: Add result sink support for external frontends - The intended purpose of these changes is to allow external frontends to receive query results via files rather than streaming the results through the thrift interface. - External frontends are expected to provide an FeFsTable implementation that describes the desired location to store results. - External frontends are responsible for managing the files after the query is completed. - Testing has been manual and through an implementation of an external frontend. Change-Id: I024bf41d77bb81f1ab0debdbd31ec3687c83f072 Reviewed-by: Aman Sinha --- M be/src/exec/hdfs-table-sink.cc M be/src/exec/hdfs-table-sink.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M common/thrift/DataSinks.thrift M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/TableSink.java 9 files changed, 93 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/17144/1 -- To view, visit http://gerrit.cloudera.org:8080/17144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I024bf41d77bb81f1ab0debdbd31ec3687c83f072 Gerrit-Change-Number: 17144 Gerrit-PatchSet: 1 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha
[Impala-ASF-CR] IMPALA-10551: Add result sink support for external frontends
John Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/17144 ) Change subject: IMPALA-10551: Add result sink support for external frontends .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/17144/1/be/src/runtime/fragment-instance-state.h File be/src/runtime/fragment-instance-state.h: http://gerrit.cloudera.org:8080/#/c/17144/1/be/src/runtime/fragment-instance-state.h@121 PS1, Line 121: JoinBuilder* GetJoinBuildSink() const; Add CR -- To view, visit http://gerrit.cloudera.org:8080/17144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I024bf41d77bb81f1ab0debdbd31ec3687c83f072 Gerrit-Change-Number: 17144 Gerrit-PatchSet: 1 Gerrit-Owner: John Sherman Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: John Sherman Gerrit-Comment-Date: Mon, 01 Mar 2021 17:09:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 ) Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8266/ : 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/17136 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53 Gerrit-Change-Number: 17136 Gerrit-PatchSet: 2 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 16:14:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17116 ) Change subject: IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8265/ : 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/17116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I14a3cee29f1fc91f4431b7ea89053bb3fbfa5e69 Gerrit-Change-Number: 17116 Gerrit-PatchSet: 11 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 16:03:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
Yida Wu has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/17136 ) Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build .. IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build For start option “scratch_dirs”, it only considers local filesystem as the default filesystem, regardless of the setting of DefaultFS(for a remote scratch dir, it needs to explicitly set it with the remote fs prefix). However, the function AssignQueue() would assign the queue based on not only the path string but also the default filesystem setting. For example, if scratch_dirs is set as "/tmp", the scratch dir is supposed to be in the local filesystem, but the AssignQueue() would consider it as "s3a://xxx/tmp" if a s3 path is set as the default fs. To fix this, the solution is to add a bool variable to AssignQueue() to decide whether or not to check the default fs setting when parsing the file path. For all of the scratch dirs, AssignQueue() won’t check the default fs. Tests: Set the default fs as a s3 path, reran TestDiskSpillConfigurations::test_disk_spill_encryption_disabled and TestQueryRetries::test_query_retry_reaches_spool_limit. Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53 --- M be/src/runtime/io/disk-io-mgr.cc M be/src/runtime/io/disk-io-mgr.h M be/src/runtime/io/scan-range.cc M be/src/runtime/tmp-file-mgr.cc 4 files changed, 24 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/17136/2 -- To view, visit http://gerrit.cloudera.org:8080/17136 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53 Gerrit-Change-Number: 17136 Gerrit-PatchSet: 2 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17116 ) Change subject: IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad .. Patch Set 11: (4 comments) http://gerrit.cloudera.org:8080/#/c/17116/11/tests/hs2/hs2_test_suite.py File tests/hs2/hs2_test_suite.py: http://gerrit.cloudera.org:8080/#/c/17116/11/tests/hs2/hs2_test_suite.py@123 PS11, Line 123: flake8: E251 unexpected spaces around keyword / parameter equals http://gerrit.cloudera.org:8080/#/c/17116/11/tests/hs2/hs2_test_suite.py@123 PS11, Line 123: flake8: E251 unexpected spaces around keyword / parameter equals http://gerrit.cloudera.org:8080/#/c/17116/11/tests/hs2/test_hs2.py File tests/hs2/test_hs2.py: http://gerrit.cloudera.org:8080/#/c/17116/11/tests/hs2/test_hs2.py@740 PS11, Line 740: " flake8: E501 line too long (114 > 90 characters) http://gerrit.cloudera.org:8080/#/c/17116/11/tests/hs2/test_hs2.py@742 PS11, Line 742: _ flake8: E501 line too long (96 > 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/17116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I14a3cee29f1fc91f4431b7ea89053bb3fbfa5e69 Gerrit-Change-Number: 17116 Gerrit-PatchSet: 11 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 15:37:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/17116 ) Change subject: IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad .. Patch Set 9: (7 comments) http://gerrit.cloudera.org:8080/#/c/17116/9/be/src/rpc/hs2-http-test.cc File be/src/rpc/hs2-http-test.cc: http://gerrit.cloudera.org:8080/#/c/17116/9/be/src/rpc/hs2-http-test.cc@53 PS9, Line 53: return_val > nit: we've used '_return' for all of the other functions here Done http://gerrit.cloudera.org:8080/#/c/17116/9/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/17116/9/be/src/service/impala-hs2-server.cc@1193 PS9, Line 1193: shared_ptr session; > I don't think the session is actually used for anything here, we're basical Done http://gerrit.cloudera.org:8080/#/c/17116/9/be/src/util/backend-gflag-util.h File be/src/util/backend-gflag-util.h: http://gerrit.cloudera.org:8080/#/c/17116/9/be/src/util/backend-gflag-util.h@27 PS9, Line 27: /// Builds the TBackendGflags object to pass to JNI. This is used to pass the gflag : /// configs to the Frontend and the Catalog. > It would be cleaner to put this comment directly above the version of the f Done http://gerrit.cloudera.org:8080/#/c/17116/9/be/src/util/backend-gflag-util.h@29 PS9, Line 29: class TBackendGflags; > Not a big deal, but its pretty standard in Impala to put all the forward de Done http://gerrit.cloudera.org:8080/#/c/17116/9/be/src/util/backend-gflag-util.h@31 PS9, Line 31: GetThriftBackendGflags > Might be nice to rename this, eg. to GetThriftBackendGFlagsForJNI, since th Done http://gerrit.cloudera.org:8080/#/c/17116/9/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/17116/9/common/thrift/ImpalaService.thrift@857 PS9, Line 857: // Returns the current TBackendGflags > Maybe mention that this is only supported for the "external fe" server Done http://gerrit.cloudera.org:8080/#/c/17116/9/tests/hs2/test_hs2.py File tests/hs2/test_hs2.py: http://gerrit.cloudera.org:8080/#/c/17116/9/tests/hs2/test_hs2.py@738 PS9, Line 738: hs2_client > I'm not sure how this test would work, since I would assune that 'hs2_clien Done -- To view, visit http://gerrit.cloudera.org:8080/17116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I14a3cee29f1fc91f4431b7ea89053bb3fbfa5e69 Gerrit-Change-Number: 17116 Gerrit-PatchSet: 9 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 15:37:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad
Hello Thomas Tauber-Marshall, Joe McDonnell, John Sherman, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17116 to look at the new patch set (#11). Change subject: IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad .. IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad This patch add a new interface ImpalaServer::GetBackendConfig() that returns the current TBackendGflags from impalad. Testing: Called new interface from external frontend. Verified that TBackendGflags were populated correctly. Reviewed-by: John Sherman Change-Id: I14a3cee29f1fc91f4431b7ea89053bb3fbfa5e69 --- M be/src/catalog/catalog.cc M be/src/rpc/hs2-http-test.cc M be/src/service/frontend.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.h M be/src/util/backend-gflag-util.cc M be/src/util/backend-gflag-util.h M be/src/util/logging-support.cc M common/thrift/ImpalaService.thrift M tests/common/impala_test_suite.py M tests/conftest.py M tests/hs2/hs2_test_suite.py M tests/hs2/test_hs2.py 13 files changed, 83 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/17116/11 -- To view, visit http://gerrit.cloudera.org:8080/17116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I14a3cee29f1fc91f4431b7ea89053bb3fbfa5e69 Gerrit-Change-Number: 17116 Gerrit-PatchSet: 11 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17116 ) Change subject: IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8264/ : 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/17116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I14a3cee29f1fc91f4431b7ea89053bb3fbfa5e69 Gerrit-Change-Number: 17116 Gerrit-PatchSet: 10 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 01 Mar 2021 14:20:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad
Hello Thomas Tauber-Marshall, Joe McDonnell, John Sherman, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17116 to look at the new patch set (#10). Change subject: IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad .. IMPALA-10546: Add ImpalaServer interface to retrieve BackendConfig from impalad This patch add a new interface ImpalaServer::GetBackendConfig() that returns the current TBackendGflags from impalad. Testing: Called new interface from external frontend. Verified that TBackendGflags were populated correctly. Reviewed-by: John Sherman Change-Id: I14a3cee29f1fc91f4431b7ea89053bb3fbfa5e69 --- M be/src/catalog/catalog.cc M be/src/rpc/hs2-http-test.cc M be/src/service/frontend.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.h M be/src/util/backend-gflag-util.cc M be/src/util/backend-gflag-util.h M be/src/util/logging-support.cc M common/thrift/ImpalaService.thrift M tests/hs2/test_hs2.py 10 files changed, 67 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/17116/10 -- To view, visit http://gerrit.cloudera.org:8080/17116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I14a3cee29f1fc91f4431b7ea89053bb3fbfa5e69 Gerrit-Change-Number: 17116 Gerrit-PatchSet: 10 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-10524: Changes to HdfsPartition for third party extensions.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17092 ) Change subject: IMPALA-10524: Changes to HdfsPartition for third party extensions. .. Patch Set 10: (2 comments) The change in HdfsTable may introduce an issue. Other changes look good to me. http://gerrit.cloudera.org:8080/#/c/17092/10/be/src/rpc/hs2-http-test.cc File be/src/rpc/hs2-http-test.cc: http://gerrit.cloudera.org:8080/#/c/17092/10/be/src/rpc/hs2-http-test.cc@51 PS10, Line 51: virtual void GetBackendConfig(TGetBackendConfigResp& return_val, Is this used somewhere? http://gerrit.cloudera.org:8080/#/c/17092/10/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/17092/10/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1719 PS10, Line 1719: if (isSchemaLoaded_) return; I don't think we can do this. This means the table schema will only be loaded once. However, when executiing REFRESH or some DDL commands, we need to reload these to update the schema. Here's an example: # Create a table in Impala and run any query on it to make its metadata loaded impala> create table my_tbl (id int, name string); impala> desc my_tbl; # Change its schema in Hive beeline> alter table my_tbl add columns (address string); # Refresh in Impala. Impala should reload its metadata and get the latest schema. impala> refresh my_tbl; impala> desc my_tbl; -- To view, visit http://gerrit.cloudera.org:8080/17092 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a792642f27228118ac8f2e8ef98e8ba7aee4a46 Gerrit-Change-Number: 17092 Gerrit-PatchSet: 10 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Mon, 01 Mar 2021 13:29:34 + Gerrit-HasComments: Yes
