[Impala-ASF-CR] IMPALA-12508: Update query option "enabled runtime filter types" comments
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20627 ) Change subject: IMPALA-12508: Update query option "enabled_runtime_filter_types" comments .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/20627/3/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/20627/3/common/thrift/ImpalaService.thrift@540 PS3, Line 540: // Default is [BLOOM, MIN_MAX]. > nit: Based on comment at https://github.com/apache/impala/blob/248060528abd Based on comment at https://github.com/apache/impala/blob/248060528abd7fb5425f519ee1608598ed2d3ac3/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java#L1147 and the code below, IN_LIST is currently only enabled in ORC files. So your meaning may be something like this? - Kudu scan: BLOOM, MIN_MAX - Hdfs scan on Parquet file: BLOOM, MIN_MAX - Hdfs scan on ORC file: BLOOM, IN_LIST - Hdfs scan on other kind of file: BLOOM -- To view, visit http://gerrit.cloudera.org:8080/20627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iecff7c655e273712d851d9ce94ef5713e579aa72 Gerrit-Change-Number: 20627 Gerrit-PatchSet: 3 Gerrit-Owner: Zihao Ye Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 30 Oct 2023 07:50:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12508: Update query option "enabled runtime filter types" comments
Hello Riza Suminto, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20627 to look at the new patch set (#4). Change subject: IMPALA-12508: Update query option "enabled_runtime_filter_types" comments .. IMPALA-12508: Update query option "enabled_runtime_filter_types" comments The query option "enabled_runtime_filter_types" currently allows specifying a comma-separated list of runtime filter types (including BLOOM, MIN_MAX and IN_LIST) to enable or setting it to "all" to use all types. But the comment for this query option does not mention IN_LIST and states that it only applies to Kudu, which is outdated, as it also affects HDFS scans after IMPALA-10325. Therefore, this patch update the relevant comments to avoid any misleading caused by the outdated comments. Change-Id: Iecff7c655e273712d851d9ce94ef5713e579aa72 --- M common/thrift/ImpalaService.thrift M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java 2 files changed, 13 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/20627/4 -- To view, visit http://gerrit.cloudera.org:8080/20627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iecff7c655e273712d851d9ce94ef5713e579aa72 Gerrit-Change-Number: 20627 Gerrit-PatchSet: 4 Gerrit-Owner: Zihao Ye Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-12508: Update query option "enabled runtime filter types" comments
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20627 ) Change subject: IMPALA-12508: Update query option "enabled_runtime_filter_types" comments .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/14283/ : 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/20627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iecff7c655e273712d851d9ce94ef5713e579aa72 Gerrit-Change-Number: 20627 Gerrit-PatchSet: 4 Gerrit-Owner: Zihao Ye Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 30 Oct 2023 13:11:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12364: Display memory, disk and network metrics in webUI's query timeline
Surya Hebbar has uploaded a new patch set (#22). ( http://gerrit.cloudera.org:8080/20355 ) Change subject: IMPALA-12364: Display memory, disk and network metrics in webUI's query timeline .. IMPALA-12364: Display memory, disk and network metrics in webUI's query timeline This patch adds fragment-level metrics to the WebUI query timeline display along with additional disk and network metrics. The fragment's plan nodes are enlarged with an animated transition on hovering over the fragment's row in query timeline's fragment diagram. On clicking the plan nodes, total thread and memory usage of the parent fragment are displayed, after accumulating memory and thread usage of all child nodes. Thread usage is being shown on the additional Y-axis. In this way, memory and thread usage of multiple fragments can be compared alongside. A fragment's usage can be hidden by clicking on any of the child plan nodes again. These counters are available within the profile with following names. - MemoryUsage - ThreadUsage Once a fragment's metrics are displayed, they are updated as they are collected from the profile during a running query. A grid-line is displayed along with a tooltip on hovering over the fragment diagram, containing the instantaneous time at that position. This grid-line also triggers tooltips and gridlines in other charts. A warning is displayed on clicking a fragment with less number of samples available. RESOURCE_TRACE_RATIO query option must be set for providing periodic metrics within the profile. This allows the following time series counters to be displayed on the query timeline. - HostDiskWriteThroughput - HostDiskReadThroughput - HostNetworkRx - HostNetworkTx The additional Y-axis within the utilization chart is used to represent the average of these metrics. The memory units in tooltips and ticks on co-ordinate axes are displayed in human readable form such as KB, MB, GB and PB for convenience. Both of the charts contain controls to close the chart. These charts can also be resized until a maximum and minmum limit by dragging the resize bar's handle. Along with mouse wheel events, the diagrams can be horizontally stretched by the help of buttons with horizontal zoom icons at the top of the page. Timeticks are being autoscaled during fragment diagram's horizontal zoom. In addition to the scrollbar, hovering on edges of the window allows horizontal scrolling. Test cases have been for the additional disk, network and fragment level memory metrics parsing functions. Change-Id: Ifd25e6f0bc9fbd664ec98936daff3f27182dfc7f --- M bin/rat_exclude_files.txt A www/icons/drag_handle.png A www/icons/horizontal_zoomin.png A www/icons/horizontal_zoomout.png M www/query_timeline.tmpl M www/scripts/query_timeline/chart_commons.js M www/scripts/query_timeline/fragment_diagram.js A www/scripts/query_timeline/fragment_metrics_diagram.js M www/scripts/query_timeline/global_members.js M www/scripts/query_timeline/host_utilization_diagram.js A www/scripts/tests/query_timeline/fragment_metrics_diagram.test.js M www/scripts/tests/query_timeline/host_utilization_diagram.test.js 12 files changed, 635 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/20355/22 -- To view, visit http://gerrit.cloudera.org:8080/20355 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifd25e6f0bc9fbd664ec98936daff3f27182dfc7f Gerrit-Change-Number: 20355 Gerrit-PatchSet: 22 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Surya Hebbar Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-12364: Display memory, disk and network metrics in webUI's query timeline
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20355 ) Change subject: IMPALA-12364: Display memory, disk and network metrics in webUI's query timeline .. Patch Set 22: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/14284/ : 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/20355 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifd25e6f0bc9fbd664ec98936daff3f27182dfc7f Gerrit-Change-Number: 20355 Gerrit-PatchSet: 22 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Surya Hebbar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Mon, 30 Oct 2023 14:21:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/20612 )
Change subject: IMPALA-3825: Delegate runtime filter aggregation to some
executors
..
Patch Set 7:
(3 comments)
The implementation looks great in general, just a few comments
http://gerrit.cloudera.org:8080/#/c/20612/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20612/7//COMMIT_MSG@20
PS7, Line 20: Query option MAX_NUM_FILTER_AGGREGATOR is added to control
: this feature. Given N as the number of backend executors
excluding the
: coordinator, the selected number of intermediate aggregators M =
: min(MAX_NUM_FILTER_AGGREGATOR, N / 2). Setting
MAX_NUM_FILTER_AGGREGATOR
: <= 0 will disable the intermediate aggregator feature.
MAX_NUM_FILTER_AGGREGATOR seems ok for me in the short term to test this
feature, but I think that there should be a more sophisticated to tune this on
the long term. My concern is that depending on the number of hosts used in the
query, a static default for MAX_NUM_FILTER_AGGREGATOR can be easily too low
or too large.
I think that ideally we should have an option like
MAX_NUM_FILTERS_AGGREGATED_PER_HOST, so if this value is 10, then a 20 host
query will have 2 pre-aggregators, while a 100 host query will have 10.
http://gerrit.cloudera.org:8080/#/c/20612/7/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:
http://gerrit.cloudera.org:8080/#/c/20612/7/be/src/scheduling/scheduler.cc@282
PS7, Line 282: inline bool KrpcAddressMatch(const NetworkAddressPB& lhs, const
NetworkAddressPB& rhs) {
: return lhs.hostname() == rhs.hostname() && lhs.port() ==
rhs.port();
: }
Can you move this to network-util.h?
I think that there should be a single function to compare addresses, as it is
not completely self-evident (due to uds_ address)
http://gerrit.cloudera.org:8080/#/c/20612/7/be/src/scheduling/scheduler.cc@317
PS7, Line 317: if (num_agg <= 0) return;
Does num_agg = 1 make sense?
--
To view, visit http://gerrit.cloudera.org:8080/20612
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11d38ed0f223d6e5b32a19ebe725af7738ee4ab0
Gerrit-Change-Number: 20612
Gerrit-PatchSet: 7
Gerrit-Owner: Riza Suminto
Gerrit-Reviewer: Abhishek Rawat
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Riza Suminto
Gerrit-Comment-Date: Mon, 30 Oct 2023 14:37:24 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP: port critical geospatial functions to c++
Daniel Becker has posted comments on this change. (
http://gerrit.cloudera.org:8080/20602 )
Change subject: WIP: port critical geospatial functions to c++
..
Patch Set 2:
(172 comments)
I didn't pay much attention to the tests, but this review is already huge.
http://gerrit.cloudera.org:8080/#/c/20602/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20602/2//COMMIT_MSG@1
PS2, Line 1: Parent: fb2d2b27 (IMPALA-12430: Skip compression when sending
row batches within same process)
Many functions return a bool that indicates whether reading/writing was
successful. The meaning of the return value could be documented somewhere,
either at each function or in some readme file.
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/common/global-flags.cc@425
PS2, Line 425: +
Nit: add space after +.
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h
File be/src/exprs/geo/common.h:
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@39
PS2, Line 39: // TODO: This file is included in impala-ir.cc, and these
typedefs will be visible in the
This TODO can probably be deleted now.
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@71
PS2, Line 71: OgcTypeToWktPrefix
For other constants we used UPPER_SNAKE_CASE (see L31-35), maybe we should use
it for this and on L61 as well.
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@71
PS2, Line 71: Ogc
On L61 we used OGC in capitals. Either one is good but it should be consistent.
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@82
PS2, Line 82: T readFromGeom(const StringVal& geom, int offset) {
Could add some explanation for this and writeToGeom().
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@84
PS2, Line 84: T*
Could be 'const T*' to emphasise that we're not modifying it.
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@106
PS2, Line 106: inline OGCType getOGCType(const StringVal& geom) {
Could add a DCHECK that 'OGC_TYPE_OFFSET' is within geom.len.
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@108
PS2, Line 108: char
Optional: we could set the underlying type of the 'OGCType' enum to 'char' or
'unsigned char' and then we could use 'OGCType' as the template argument here.
We do this with 'EsriType' in shape-format.h.
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@128
PS2, Line 128: inline void setOGCType(StringVal& geom, OGCType ogc_type) {
Could add 'static_assert(OGC_TYPE_SIZE == sizeof(char));'
Could add a DCHECK that 'OGC_TYPE_OFFSET' is within geom.len.
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@129
PS2, Line 129: char
See L108.
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@134
PS2, Line 134: double
Optional, not sure if worth it: we could use decltype(point.x()) instead of
'double', and static_assert that point.y() has the same type.
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/formatted-double.h
File be/src/exprs/geo/formatted-double.h:
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/formatted-double.h@30
PS2, Line 30: a
Nit: consistEnt
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/formatted-double.h@47
PS2, Line 47: 2.6665
This value is the same as the one on L43. Could you add a short clarification
why precision 17 is not good?
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/formatted-double.h@78
PS2, Line 78: FormattedDouble& operator/=(const FormattedDouble& rhs) {
Consider adding the remaining modification assignment operators for
completeness:
operator-=()
operator*=()
operator%=()
operator&=()
operator|=()
operator^=()
operator>>=()
operator<<=()
operator++()- pre-increment
operator++(int) - post-increment
operator--()- pre-decrement
operator--(int) - post-decrement
They are not needed now but may be needed in the future.
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geometry-wrapper.h
File be/src/exprs/geo/geometry-wrapper.h:
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geometry-wrapper.h@27
PS2, Line 27: #include "common/names.h"
"common" should come before "exprs".
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geometry-wrapper.h@32
PS2, Line 32:
Nit: unnecessary empty line.
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geometry-wrapper.h@33
PS2, Line 33: class RelationWrapper;
If it's declared as a friend class I don't think forward declaring it here is
necessary.
http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geometry-wrapper.h@35
PS2, Line 35: types
It could be extended a bit, e.g. 'The types are taken from here: ...'
[Impala-ASF-CR] IMPALA-7131: Support external data sources in LocalCatalog mode
Wenzhe Zhou has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20574 ) Change subject: IMPALA-7131: Support external data sources in LocalCatalog mode .. IMPALA-7131: Support external data sources in LocalCatalog mode This patch makes external data source working in LocalCatalog mode: - Add APIs in CatalogdMetaProvider to fetch DataSource from Catalog server through RPC. - Add getDataSources() and getDataSource() in LocalCatalog. - Add LocalDataSourceTable class for loading DataSource table in LocalCatalog. - Handle request for loading DataSource in CatalogServiceCatalog on Catalog server. - Enable tests which are skipped by SkipIfCatalogV2.data_sources_unsupported(). Remove SkipIfCatalogV2.data_sources_unsupported(). - Add end-to-end tests for LocalCatalog mode. Testing: - Passed core tests Change-Id: I40841c9be9064ac67771c4d3f5acbb3b552a2e55 Reviewed-on: http://gerrit.cloudera.org:8080/20574 Tested-by: Impala Public Jenkins Reviewed-by: Wenzhe Zhou --- M common/thrift/CatalogService.thrift M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java A fe/src/main/java/org/apache/impala/catalog/local/LocalDataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M testdata/workloads/functional-query/queries/QueryTest/data-source-tables.test M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test M tests/common/skip.py A tests/custom_cluster/test_ext_data_sources.py M tests/metadata/test_ddl.py M tests/metadata/test_metadata_query_statements.py M tests/query_test/test_ext_data_sources.py 16 files changed, 573 insertions(+), 36 deletions(-) Approvals: Impala Public Jenkins: Verified Wenzhe Zhou: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/20574 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I40841c9be9064ac67771c4d3f5acbb3b552a2e55 Gerrit-Change-Number: 20574 Gerrit-PatchSet: 12 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-7131: Support external data sources in LocalCatalog mode
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20574 ) Change subject: IMPALA-7131: Support external data sources in LocalCatalog mode .. Patch Set 11: Code-Review+2 carry +2 from Quanlong -- To view, visit http://gerrit.cloudera.org:8080/20574 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I40841c9be9064ac67771c4d3f5acbb3b552a2e55 Gerrit-Change-Number: 20574 Gerrit-PatchSet: 11 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Mon, 30 Oct 2023 16:04:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12530: Make Postgres server to accept remote connections from same subnet
Wenzhe Zhou has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20634 Change subject: IMPALA-12530: Make Postgres server to accept remote connections from same subnet .. IMPALA-12530: Make Postgres server to accept remote connections from same subnet Currently Postgres server in Impala mini-cluster only accepts local connections. In dockerised-tests, Impala daemons and Postgres server are running on different hosts so Postgres server is not accessible for Impala coordinators. This patch changes the configurations of Postgres server to make it accept remote connections from hosts in the same sub network. Testing: - Ran dockerised-tests for query_test/test_ext_data_sources.py. Change-Id: I7dfaf38bf9178cb2ec7ef15c79c17a5ab1e1c6dc --- M bin/bootstrap_system.sh M tests/query_test/test_ext_data_sources.py 2 files changed, 11 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/20634/1 -- To view, visit http://gerrit.cloudera.org:8080/20634 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7dfaf38bf9178cb2ec7ef15c79c17a5ab1e1c6dc Gerrit-Change-Number: 20634 Gerrit-PatchSet: 1 Gerrit-Owner: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-12530: Make Postgres server to accept remote connections from same subnet
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20634 ) Change subject: IMPALA-12530: Make Postgres server to accept remote connections from same subnet .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/14285/ : 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/20634 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7dfaf38bf9178cb2ec7ef15c79c17a5ab1e1c6dc Gerrit-Change-Number: 20634 Gerrit-PatchSet: 1 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 30 Oct 2023 20:01:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12530: Make Postgres server to accept remote connections from same subnet
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20634 ) Change subject: IMPALA-12530: Make Postgres server to accept remote connections from same subnet .. Patch Set 1: (1 comment) I don't see any unwarranted security impact from this change. http://gerrit.cloudera.org:8080/#/c/20634/1/bin/bootstrap_system.sh File bin/bootstrap_system.sh: http://gerrit.cloudera.org:8080/#/c/20634/1/bin/bootstrap_system.sh@392 PS1, Line 392: ubuntu sudo sed -ri 's/host +all +all +127.0.0.1\/32/host all all samenet/g' \ This is only going to update the IPv4 line. We could technically use listen_addresses = '0.0.0.0' to match that. I also wonder if 'samehost' would be sufficient, since they're all local containers. -- To view, visit http://gerrit.cloudera.org:8080/20634 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7dfaf38bf9178cb2ec7ef15c79c17a5ab1e1c6dc Gerrit-Change-Number: 20634 Gerrit-PatchSet: 1 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 30 Oct 2023 20:19:19 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12530: Make Postgres server to accept remote connections from same subnet
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20634 ) Change subject: IMPALA-12530: Make Postgres server to accept remote connections from same subnet .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/20634 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7dfaf38bf9178cb2ec7ef15c79c17a5ab1e1c6dc Gerrit-Change-Number: 20634 Gerrit-PatchSet: 1 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 30 Oct 2023 20:19:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12530: Make Postgres server to accept remote connections from same subnet
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20634 ) Change subject: IMPALA-12530: Make Postgres server to accept remote connections from same subnet .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/20634/1/bin/bootstrap_system.sh File bin/bootstrap_system.sh: http://gerrit.cloudera.org:8080/#/c/20634/1/bin/bootstrap_system.sh@392 PS1, Line 392: ubuntu sudo sed -ri 's/host +all +all +127.0.0.1\/32/host all all samenet/g' \ > This is only going to update the IPv4 line. We could technically use 'samehost' does not work since they are running on hosts with different ips. In one dockerised-tests, Postgres server was running on 172.18.0.1, Impalad was running on 172.18.0.6. I will try to run test with listen_addresses = '0.0.0.0'. Thanks for your review. -- To view, visit http://gerrit.cloudera.org:8080/20634 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7dfaf38bf9178cb2ec7ef15c79c17a5ab1e1c6dc Gerrit-Change-Number: 20634 Gerrit-PatchSet: 1 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Mon, 30 Oct 2023 20:36:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12508: Update query option "enabled runtime filter types" comments
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20627 ) Change subject: IMPALA-12508: Update query option "enabled_runtime_filter_types" comments .. Patch Set 4: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/20627/3/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/20627/3/common/thrift/ImpalaService.thrift@540 PS3, Line 540: // ALL - apply all type > Based on comment at Zihao Ye's interpretation matches my reading of assignRuntimeFilters as well. It looks like Kudu accepts IN_LIST too, but doesn't actually support it. -- To view, visit http://gerrit.cloudera.org:8080/20627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iecff7c655e273712d851d9ce94ef5713e579aa72 Gerrit-Change-Number: 20627 Gerrit-PatchSet: 4 Gerrit-Owner: Zihao Ye Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 30 Oct 2023 22:34:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10976: Sync db/table in catalogD to latest HMS event id after the DDL operation for all DDLS from Impala clients
Hello Quanlong Huang, k.venureddy2...@gmail.com, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20367 to look at the new patch set (#5). Change subject: IMPALA-10976: Sync db/table in catalogD to latest HMS event id after the DDL operation for all DDLS from Impala clients .. IMPALA-10976: Sync db/table in catalogD to latest HMS event id after the DDL operation for all DDLS from Impala clients The idea is that when any DDL operation is performed by Impala, it also syncs the db/table to its latest event ID as per HMS. This way updates to a db/table's are applied in the same order as they appear in the Notification log table in HMS which ensures consistency. Currently catalogD applies any updates received from Impala clients in-place. Instead it should perform an HMS operation first and then replay all the HMS events since the last synced event id. Implementation: when the enable_sync_to_latest_event_on_ddls flag is set to true, we do the DDL operation first, i.e., perform HMS operation and then sync the db/table in the catalogD's cache to the latest event in HMS for the corresponding db/table. Currently we fetch all events greater than the db/table's lastSyncEventId and filter them in the events processor to sync only the current db/table events. Once HIVE-27499 is implemented, we can directly fetch the events only for the respective db/table and process them. Currently, there is no efficient way to identify if there are pending events for a db/table. Set 'enable_sync_to_latest_event_on_ddls' to true and 'invalidate_hms_cache_on_ddls' to false to use this feature. Note: We don't modify the cache using MetastoreEventsProcessor for alter table rename operation as this is a complex operation regarding cache modification. Also, we modify cache for a DML operation 'truncate table' using this feature. Testing: 1) Added few tests in the MetaStoreEventProcessorForTest to verify this feature that simulates the metadata sync between HMS and Impala. 2) Added few tests in the CatalogHmsSyncToLatestEventIdTest class to the metadata sync between HMS end point, Catalog Metastore Server and Impala. The HMS end point serves as common interface to metadata changes outside the current Impala service such as Hive, Spark or other Impala service. Also verified the table lastSyncEventId is updated after the events are sync and confirmed that metastore event processor ignored these synced events. Change-Id: Ia250d0a943838086c187e5cb7c60035e5a564bbf --- M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsSyncToLatestEventIdTest.java 5 files changed, 511 insertions(+), 92 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/20367/5 -- To view, visit http://gerrit.cloudera.org:8080/20367 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia250d0a943838086c187e5cb7c60035e5a564bbf Gerrit-Change-Number: 20367 Gerrit-PatchSet: 5 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala
[Impala-ASF-CR] IMPALA-10976: Sync db/table in catalogD to latest HMS event id after the DDL operation for all DDLS from Impala clients
Sai Hemanth Gantasala has posted comments on this change. (
http://gerrit.cloudera.org:8080/20367 )
Change subject: IMPALA-10976: Sync db/table in catalogD to latest HMS event id
after the DDL operation for all DDLS from Impala clients
..
Patch Set 5:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/20367/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20367/4//COMMIT_MSG@8
PS4, Line 8: after the DD
> Can you add to the title whether Impala syncs the events before or after th
Ack
http://gerrit.cloudera.org:8080/#/c/20367/4//COMMIT_MSG@19
PS4, Line 19: n: when the enable_sync_to_latest_event_on_ddls flag is
: set to true, we do t
> This is stale now.
Ack
http://gerrit.cloudera.org:8080/#/c/20367/4//COMMIT_MSG@20
PS4, Line 20: DDL operation first, i.e., perform HMS operation
: and then sync the db/table in the catalogD's cache to the latest
event
: in HMS for the corresponding db/table. Currently we fetch all
events
> Won't there be always a pending event, as the DDL from Impala creates a new
Agreed. But we may also fetch events that are from other databases/tables.
Ideally, we want to fetch only events that only required for the current
metadata object.
http://gerrit.cloudera.org:8080/#/c/20367/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/20367/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1396
PS3, Line 1396: Preconditions.checkNotNull(topicUpdateEntry);
> I think that we also need to hold versionLock_ before calling this, as the
I have to remove this method. This is not required anymore.
http://gerrit.cloudera.org:8080/#/c/20367/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/20367/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1045
PS4, Line 1045: alterTableOrViewRename(tbl,
:
TableName.fromThrift(params.getRename_params().getNew_table_name()),
: newCatalogVersion, wantMinimalResult, response);
: return;
> I don't see this change applied to renames, while the commit message sugges
Rename is a complex operation, it has to change the catalogD's cache
immediately after the HMS operation without waiting for
MetastoreEventsProcessor to sync the metadata object's events. I'll update the
commit message accordingly, regarding operations that don't depend upon the
event processor to update the cache.
http://gerrit.cloudera.org:8080/#/c/20367/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1113
PS4, Line 1113: reloadMetadata = false;
> All the code paths that set 'reloadMetadata' to false have updated the cata
reloadMetadata is set to false for addPartitions() --
https://gerrit.cloudera.org/c/20367/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java#4393
and dropPartitions() --
https://gerrit.cloudera.org/c/20367/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java#5265
I have also added tests CatalogHmsSyncToLatestEventIdTest to verify the same.
Do you see any issues with not modifying the cache in place? Should I change
this back to the original state for addPartitions() and dropPartitions()?
Please let me know.
http://gerrit.cloudera.org:8080/#/c/20367/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1263
PS4, Line 1263: // Make sure we won't forget finalizing the modification.
: if (tbl.hasInProgressModification())
Preconditions.checkState(reloadMetadata);
: if (!trySyncToLatestEventId(tbl) && reloadMetadata) {
: loadTableMetadata(tbl, newCatalogVersion,
reloadFileMetadata,
: reloadTableSchema, null, "ALTER TABLE " + params
> refactor idea: this could be moved to a function like CatalogOpExecutor.try
Ack
http://gerrit.cloudera.org:8080/#/c/20367/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1270
PS4, Line 1270: catalog_.addVersionsForInflightEvents(false, tbl,
newCatalogVersion);
> This seems valid even in the syncToLatestEventId case.
Ack
http://gerrit.cloudera.org:8080/#/c/20367/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1276
PS4, Line 1276: }
> Are we using in flight event lists for anything when enableSyncToLatestEven
This is used to identify if the events are from other Impala services.
http://gerrit.cloudera.org:8080/#/c/20367/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1658
PS4, Line 1658: Id(eventId);
> What kind of guarantees do we have for the state of the event processor whe
I und
[Impala-ASF-CR] IMPALA-10976: Sync db/table in catalogD to latest HMS event id after the DDL operation for all DDLS from Impala clients
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20367 ) Change subject: IMPALA-10976: Sync db/table in catalogD to latest HMS event id after the DDL operation for all DDLS from Impala clients .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/14286/ : 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/20367 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia250d0a943838086c187e5cb7c60035e5a564bbf Gerrit-Change-Number: 20367 Gerrit-PatchSet: 5 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Tue, 31 Oct 2023 00:23:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12530: Make Postgres server to accept remote connections from same subnet
Wenzhe Zhou has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/20634 ) Change subject: IMPALA-12530: Make Postgres server to accept remote connections from same subnet .. IMPALA-12530: Make Postgres server to accept remote connections from same subnet Currently Postgres server in Impala mini-cluster only accepts local connections. In dockerised-tests, Impala daemons and Postgres server are running on different hosts so Postgres server is not accessible for Impala coordinators. This patch changes the configurations of Postgres server to make it accept remote connections from hosts in the same sub network. Enables query_test/test_ext_data_sources.py for dockerised-tests. Testing: - Passed dockerised-tests. - Passed regular core-tests. Change-Id: I7dfaf38bf9178cb2ec7ef15c79c17a5ab1e1c6dc --- M bin/bootstrap_system.sh M tests/query_test/test_ext_data_sources.py 2 files changed, 11 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/20634/2 -- To view, visit http://gerrit.cloudera.org:8080/20634 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7dfaf38bf9178cb2ec7ef15c79c17a5ab1e1c6dc Gerrit-Change-Number: 20634 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-12530: Make Postgres server to accept remote connections from same subnet
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20634 ) Change subject: IMPALA-12530: Make Postgres server to accept remote connections from same subnet .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9860/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/20634 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7dfaf38bf9178cb2ec7ef15c79c17a5ab1e1c6dc Gerrit-Change-Number: 20634 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 31 Oct 2023 01:23:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12530: Make Postgres server to accept remote connections from same subnet
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20634 ) Change subject: IMPALA-12530: Make Postgres server to accept remote connections from same subnet .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/14287/ : 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/20634 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7dfaf38bf9178cb2ec7ef15c79c17a5ab1e1c6dc Gerrit-Change-Number: 20634 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 31 Oct 2023 01:36:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12530: Make Postgres server to accept remote connections from same subnet
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20634 ) Change subject: IMPALA-12530: Make Postgres server to accept remote connections from same subnet .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/20634 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7dfaf38bf9178cb2ec7ef15c79c17a5ab1e1c6dc Gerrit-Change-Number: 20634 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 31 Oct 2023 05:50:28 + Gerrit-HasComments: No
