[Impala-ASF-CR] IMPALA-12771: Impala catalogd events-skipped may mark the wrong number
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21045 ) Change subject: IMPALA-12771: Impala catalogd events-skipped may mark the wrong number .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15754/ : 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/21045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7aeb04e999b82187eb138c0b643ead259da22f1a Gerrit-Change-Number: 21045 Gerrit-PatchSet: 5 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Tue, 02 Apr 2024 04:18:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12602: Unregister queries on idle timeout
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21074 ) Change subject: IMPALA-12602: Unregister queries on idle timeout .. Patch Set 13: Code-Review+1 (1 comment) Will +2 if there is no additional feedback until tomorrow. http://gerrit.cloudera.org:8080/#/c/21074/9/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/21074/9/be/src/service/impala-server.cc@2879 PS9, Line 2879: > Addressed. Thank you for clarifying. -- To view, visit http://gerrit.cloudera.org:8080/21074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 Gerrit-Change-Number: 21074 Gerrit-PatchSet: 13 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 02 Apr 2024 04:09:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12771: Impala catalogd events-skipped may mark the wrong number
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21045 ) Change subject: IMPALA-12771: Impala catalogd events-skipped may mark the wrong number .. Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/21045/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java: http://gerrit.cloudera.org:8080/#/c/21045/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1067 PS5, Line 1067: metrics_.getCounter(MetastoreEventsProcessor.EVENTS_SKIPPED_METRIC).inc(skippedEvent); line too long (94 > 90) http://gerrit.cloudera.org:8080/#/c/21045/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1071 PS5, Line 1071: metrics_.getCounter(MetastoreEventsProcessor.EVENTS_SKIPPED_METRIC).inc(skippedEvent); line too long (94 > 90) http://gerrit.cloudera.org:8080/#/c/21045/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2349 PS5, Line 2349: metrics_.getCounter(MetastoreEventsProcessor.EVENTS_SKIPPED_METRIC).getCount()); line too long (92 > 90) http://gerrit.cloudera.org:8080/#/c/21045/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2658 PS5, Line 2658: metrics_.getCounter(MetastoreEventsProcessor.EVENTS_SKIPPED_METRIC).inc(eventsToProcess.size() + partitionEventsToForceReload.size()); line too long (144 > 90) http://gerrit.cloudera.org:8080/#/c/21045/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2676 PS5, Line 2676: FileMetadataLoadOpts.FORCE_LOAD, getEventType().toString() + " event", true); line too long (95 > 90) -- To view, visit http://gerrit.cloudera.org:8080/21045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7aeb04e999b82187eb138c0b643ead259da22f1a Gerrit-Change-Number: 21045 Gerrit-PatchSet: 5 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Tue, 02 Apr 2024 03:53:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12771: Impala catalogd events-skipped may mark the wrong number
cclive1...@gmail.com has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/21045 ) Change subject: IMPALA-12771: Impala catalogd events-skipped may mark the wrong number .. IMPALA-12771: Impala catalogd events-skipped may mark the wrong number The description of events-skipped metric is wrong. Some cases in Add partition event ,the metric will also be increased, besides for some other cases like alter partition the event is skipped and the log is printed but the events-skipped metric is not increased. Change-Id: I7aeb04e999b82187eb138c0b643ead259da22f1a --- M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java 4 files changed, 174 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/21045/5 -- To view, visit http://gerrit.cloudera.org:8080/21045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7aeb04e999b82187eb138c0b643ead259da22f1a Gerrit-Change-Number: 21045 Gerrit-PatchSet: 5 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala
[Impala-ASF-CR] IMPALA-12852: Make Kudu service start and stop independent
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21090 ) Change subject: IMPALA-12852: Make Kudu service start and stop independent .. Patch Set 2: Code-Review+2 carry +1 from Kurt -- To view, visit http://gerrit.cloudera.org:8080/21090 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9624aaa61353bb4520e879570e5688d5e3493201 Gerrit-Change-Number: 21090 Gerrit-PatchSet: 2 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yifan Zhang Gerrit-Comment-Date: Tue, 02 Apr 2024 03:22:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12852: Make Kudu service start and stop independent
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21090 ) Change subject: IMPALA-12852: Make Kudu service start and stop independent .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10476/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21090 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9624aaa61353bb4520e879570e5688d5e3493201 Gerrit-Change-Number: 21090 Gerrit-PatchSet: 3 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yifan Zhang Gerrit-Comment-Date: Tue, 02 Apr 2024 03:23:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12852: Make Kudu service start and stop independent
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21090 ) Change subject: IMPALA-12852: Make Kudu service start and stop independent .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21090 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9624aaa61353bb4520e879570e5688d5e3493201 Gerrit-Change-Number: 21090 Gerrit-PatchSet: 3 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yifan Zhang Gerrit-Comment-Date: Tue, 02 Apr 2024 03:23:02 + Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-12362: (part-2/4) Optimize default configurations for packaging module.
Xiang Yang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20928 ) Change subject: WIP IMPALA-12362: (part-2/4) Optimize default configurations for packaging module. .. Patch Set 4: Hi zihao, Can you go ahead and help me review the second part of IMPALA-12362? many thanks:) -- To view, visit http://gerrit.cloudera.org:8080/20928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifda229b779a3d6fca647bb81fe23dd61ad7e5d66 Gerrit-Change-Number: 20928 Gerrit-PatchSet: 4 Gerrit-Owner: Xiang Yang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Xiang Yang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 02 Apr 2024 02:06:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12749: Add query option 'DISABLE CROSS JOIN'.
Xiang Yang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20946 ) Change subject: IMPALA-12749: Add query option 'DISABLE_CROSS_JOIN'. .. Patch Set 5: (7 comments) Hi zihao, Thanks for your careful review! http://gerrit.cloudera.org:8080/#/c/20946/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20946/4//COMMIT_MSG@7 PS4, Line 7: DISABLE_CROSS_JOIN > nit: 'DISABLE_CROSS_JOIN' might be more in line with the style of other sim I think the 'supression-type' naming is counterintuitive, but either way is ok. http://gerrit.cloudera.org:8080/#/c/20946/4//COMMIT_MSG@9 PS4, Line 9: It is well known that the cross join has poor performance, and a SQL : which has cross join can even block the execu > Could you further explain why we need this query option and in what scenari Done http://gerrit.cloudera.org:8080/#/c/20946/4/be/src/service/query-options.h File be/src/service/query-options.h: http://gerrit.cloudera.org:8080/#/c/20946/4/be/src/service/query-options.h@53 PS4, Line 53: > Need to align the '\' Done http://gerrit.cloudera.org:8080/#/c/20946/4/common/thrift/Query.thrift File common/thrift/Query.thrift: http://gerrit.cloudera.org:8080/#/c/20946/4/common/thrift/Query.thrift@700 PS4, Line 700: // See comment in ImpalaService.thrift > Need to add the comment Done http://gerrit.cloudera.org:8080/#/c/20946/4/fe/src/main/java/org/apache/impala/planner/NestedLoopJoinNode.java File fe/src/main/java/org/apache/impala/planner/NestedLoopJoinNode.java: http://gerrit.cloudera.org:8080/#/c/20946/4/fe/src/main/java/org/apache/impala/planner/NestedLoopJoinNode.java@81 PS4, Line 81: Cross Join is disabled > It's better to specify in detail why it was disabled (for example, being di Done http://gerrit.cloudera.org:8080/#/c/20946/5/testdata/workloads/functional-query/queries/QueryTest/cross-joins.test File testdata/workloads/functional-query/queries/QueryTest/cross-joins.test: http://gerrit.cloudera.org:8080/#/c/20946/5/testdata/workloads/functional-query/queries/QueryTest/cross-joins.test@7 PS5, Line 7: I accidentally deleted part of it, I'll add it later. http://gerrit.cloudera.org:8080/#/c/20946/4/tests/query_test/test_join_queries.py File tests/query_test/test_join_queries.py: http://gerrit.cloudera.org:8080/#/c/20946/4/tests/query_test/test_join_queries.py@116 PS4, Line 116: test_cross_joins > 'test_disable_cross_join' seems more appropriate. I keep it unchanged, for we can add more 'cross-join' related test in the future, but not only 'disable-cross-join' testcase. -- To view, visit http://gerrit.cloudera.org:8080/20946 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d973118a6d1e433475161924bd0eeafde21bb37 Gerrit-Change-Number: 20946 Gerrit-PatchSet: 5 Gerrit-Owner: Xiang Yang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Xiang Yang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 02 Apr 2024 02:04:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12749: Add query option 'DISABLE CROSS JOIN'.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20946 ) Change subject: IMPALA-12749: Add query option 'DISABLE_CROSS_JOIN'. .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15753/ : 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/20946 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d973118a6d1e433475161924bd0eeafde21bb37 Gerrit-Change-Number: 20946 Gerrit-PatchSet: 5 Gerrit-Owner: Xiang Yang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Xiang Yang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 02 Apr 2024 01:21:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12852: Make Kudu service start and stop independent
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/21090 ) Change subject: IMPALA-12852: Make Kudu service start and stop independent .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21090 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9624aaa61353bb4520e879570e5688d5e3493201 Gerrit-Change-Number: 21090 Gerrit-PatchSet: 2 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yifan Zhang Gerrit-Comment-Date: Tue, 02 Apr 2024 01:05:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12905: Disk-based tuple caching
Kurt Deschler has posted comments on this change. (
http://gerrit.cloudera.org:8080/21171 )
Change subject: IMPALA-12905: Disk-based tuple caching
..
Patch Set 6:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/21171/6/be/src/exec/tuple-cache-node.h
File be/src/exec/tuple-cache-node.h:
http://gerrit.cloudera.org:8080/#/c/21171/6/be/src/exec/tuple-cache-node.h@27
PS6, Line 27: class TupleFileReader;
Are these forward declarations needed? I though the unique_ptr members below
need the classes declared?
http://gerrit.cloudera.org:8080/#/c/21171/6/be/src/exec/tuple-cache-node.cc
File be/src/exec/tuple-cache-node.cc:
http://gerrit.cloudera.org:8080/#/c/21171/6/be/src/exec/tuple-cache-node.cc@61
PS6, Line 61: SCOPED_TIMER(runtime_profile_->total_time_counter());
runtime_profile() used above.
http://gerrit.cloudera.org:8080/#/c/21171/6/be/src/exec/tuple-cache-node.cc@142
PS6, Line 142: } else if (writer_->BytesWritten() >
tuple_cache_mgr->MaxSize()) {
Should check the size and not write if it is over.
http://gerrit.cloudera.org:8080/#/c/21171/6/be/src/exec/tuple-file-reader.h
File be/src/exec/tuple-file-reader.h:
http://gerrit.cloudera.org:8080/#/c/21171/6/be/src/exec/tuple-file-reader.h@30
PS6, Line 30: class MemTracker;
Is this forward declaration needed?
http://gerrit.cloudera.org:8080/#/c/21171/6/be/src/exec/tuple-file-reader.cc
File be/src/exec/tuple-file-reader.cc:
http://gerrit.cloudera.org:8080/#/c/21171/6/be/src/exec/tuple-file-reader.cc@77
PS6, Line 77: reader_.read(reinterpret_cast(_data_len),
sizeof(tuple_data_len));
Check status after reads using if(!reader_) like below.
http://gerrit.cloudera.org:8080/#/c/21171/6/be/src/exec/tuple-file-reader.cc@79
PS6, Line 79: DCHECK_GE(tuple_data_len, 0);
These may not be initialized if read fails.
http://gerrit.cloudera.org:8080/#/c/21171/6/be/src/exec/tuple-file-writer.cc
File be/src/exec/tuple-file-writer.cc:
http://gerrit.cloudera.org:8080/#/c/21171/6/be/src/exec/tuple-file-writer.cc@89
PS6, Line 89: if (!writer_) {
Is it sufficient to just check the stream status here or can a transient error
leave the write file invalid?
--
To view, visit http://gerrit.cloudera.org:8080/21171
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I13a65c4c0559cad3559d5f714a074dd06e9cc9bf
Gerrit-Change-Number: 21171
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Yida Wu
Gerrit-Comment-Date: Tue, 02 Apr 2024 01:00:04 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21143 ) Change subject: IMPALA-12856: Event processor should ignore processing partition with empty partition values .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15752/ : 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/21143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09 Gerrit-Change-Number: 21143 Gerrit-PatchSet: 7 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Tue, 02 Apr 2024 00:57:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12749: Add query option 'DISABLE CROSS JOIN'.
Hello Zihao Ye, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20946 to look at the new patch set (#5). Change subject: IMPALA-12749: Add query option 'DISABLE_CROSS_JOIN'. .. IMPALA-12749: Add query option 'DISABLE_CROSS_JOIN'. It is well known that the cross join has poor performance, and a SQL which has cross join can even block the execution of other SQL sometimes. For this reason Hive add a configuration "hive.strict. checks.cartesian.product" to control whether allow cross join or not. This patch add a similar query option 'DISABLE_CROSS_JOIN', which defaults to false. Testing: - Add an EE test. Change-Id: I2d973118a6d1e433475161924bd0eeafde21bb37 --- M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaService.thrift M common/thrift/Query.thrift M fe/src/main/java/org/apache/impala/planner/NestedLoopJoinNode.java A testdata/workloads/functional-query/queries/QueryTest/cross-joins.test M tests/query_test/test_join_queries.py 7 files changed, 32 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/20946/5 -- To view, visit http://gerrit.cloudera.org:8080/20946 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2d973118a6d1e433475161924bd0eeafde21bb37 Gerrit-Change-Number: 20946 Gerrit-PatchSet: 5 Gerrit-Owner: Xiang Yang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Xiang Yang Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/21143 ) Change subject: IMPALA-12856: Event processor should ignore processing partition with empty partition values .. Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/21143/6/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/21143/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2625 PS6, Line 2625: values.get(i)); > hmm, how can we make sure all the IllegalStateException are due to HMS erro getPartitionExprFromValue() will not throw any exception currently. But I'll go with your suggestion for now since it makes more sense to me. http://gerrit.cloudera.org:8080/#/c/21143/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2926 PS6, Line 2926: // refetched > If we use "continue" here, how do we throw an exception? Ideally, we don't want to throw an exception here. Consider the scenario where we get an empty partitions list from the event, we have two options 1) we just want to ignore the event without putting the event processor into an error state. Also, I had to wrap the getTypeCompatiblePartValues() with try/catch to catch the IllegalStateException Since I introduced that getTypeCompatiblePartValues() can throw IllegalStateException. 2) Or the other option at our disposal is to remove this "if" condition block and let getTypeCompatiblePartValues() throw an exception and let IMPALA-12832 take care of invalidating the table. But the (1) options doesn't invalidate the table, it just ignores the event. But (2) invalidates the table (which can be costly to load the table especially if it wide table or have hug partition count). I would prefer option (1). That's why I'm keeping if block and try/catch condition. Let me know your thoughts on this one. http://gerrit.cloudera.org:8080/#/c/21143/6/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java File fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java: http://gerrit.cloudera.org:8080/#/c/21143/6/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@211 PS6, Line 211: foundEmptyPartitionVals = true; > we can add "break" here Done http://gerrit.cloudera.org:8080/#/c/21143/6/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@215 PS6, Line 215: > nit: "values" ? Done -- To view, visit http://gerrit.cloudera.org:8080/21143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09 Gerrit-Change-Number: 21143 Gerrit-PatchSet: 7 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Tue, 02 Apr 2024 00:33:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values
Hello Quanlong Huang, Kurt Deschler, k.venureddy2...@gmail.com, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21143 to look at the new patch set (#7). Change subject: IMPALA-12856: Event processor should ignore processing partition with empty partition values .. IMPALA-12856: Event processor should ignore processing partition with empty partition values While processing partition related events, Event Processor (EP) is facing IllegalStateException if the partition fetched from HMS has empty partition values. Even though this is a bug in HMS which returns partitions with empty values, EP should ignore such partitions instead of throwing IllegalStateException. Note: Added a debug option 'mock_empty_partition_values' to add malformed partition objects. Testing: - Manually verified the test provided in jira details in local env. - Added unit test to return empty partition values and verify EP state. Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09 --- M fe/src/main/java/org/apache/impala/catalog/HdfsTable.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/main/java/org/apache/impala/util/DebugUtils.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java 6 files changed, 198 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/21143/7 -- To view, visit http://gerrit.cloudera.org:8080/21143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09 Gerrit-Change-Number: 21143 Gerrit-PatchSet: 7 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala
[Impala-ASF-CR] IMPALA-12852: Make Kudu service start and stop independent
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21090 ) Change subject: IMPALA-12852: Make Kudu service start and stop independent .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21090 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9624aaa61353bb4520e879570e5688d5e3493201 Gerrit-Change-Number: 21090 Gerrit-PatchSet: 2 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yifan Zhang Gerrit-Comment-Date: Tue, 02 Apr 2024 00:12:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12602: Unregister queries on idle timeout
Hello Riza Suminto, Jason Fehr, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21074 to look at the new patch set (#13). Change subject: IMPALA-12602: Unregister queries on idle timeout .. IMPALA-12602: Unregister queries on idle timeout Queries cancelled due to idle_query_timeout/QUERY_TIMEOUT_S are now also Unregistered to free any remaining memory, as you cannot fetch results from a cancelled query. Adds a new structure - idle_query_statuses_ - to retain Status messages for queries closed this way so that we can continue to return a clear error message if the client returns and requests query status or attempts to fetch results. This structure must be global because HS2 server can only identify a session ID from a query handle, and the query handle no longer exists. SessionState tracks queries added to idle_query_statuses_ so they can be cleared when the session is closed. Also ensures MarkInactive is called in ClientRequestState when Wait() completes. Previously WaitInternal would only MarkInactive on success, leaving any failed requests in an active state until explicitly closed or the session ended. The beeswax get_log RPC will not return the preserved error message or any warnings for these queries. It's also possible the summary and profile are rotated out of query log as the query is no longer inflight. This is an acceptable outcome as a client will likely not look for a log/summary/profile after it times out. Testing: updates test_query_expiration to verify number of waiting queries is only non-zero for queries cancelled by EXEC_TIME_LIMIT_S and not yet closed as an idle query. Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 --- M be/src/service/client-request-state.cc M be/src/service/impala-beeswax-server.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M docs/topics/impala_timeouts.xml M tests/custom_cluster/test_query_expiration.py 6 files changed, 136 insertions(+), 50 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/21074/13 -- To view, visit http://gerrit.cloudera.org:8080/21074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 Gerrit-Change-Number: 21074 Gerrit-PatchSet: 13 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-12602: Unregister queries on idle timeout
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21074 ) Change subject: IMPALA-12602: Unregister queries on idle timeout .. Patch Set 12: (15 comments) http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py File tests/custom_cluster/test_query_expiration.py: http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py@99 PS12, Line 99: = flake8: W504 line break after binary operator http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py@116 PS12, Line 116: \ flake8: W605 invalid escape sequence '\d' http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py@116 PS12, Line 116: \ flake8: W605 invalid escape sequence '\d' http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py@116 PS12, Line 116: \ flake8: W605 invalid escape sequence '\d' http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py@116 PS12, Line 116: \ flake8: W605 invalid escape sequence '\d' http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py@116 PS12, Line 116: \ flake8: W605 invalid escape sequence '\d' http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py@116 PS12, Line 116: \ flake8: W605 invalid escape sequence '\d' http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py@116 PS12, Line 116: \ flake8: W605 invalid escape sequence '\d' http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py@116 PS12, Line 116: \ flake8: W605 invalid escape sequence '\d' http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py@116 PS12, Line 116: \ flake8: W605 invalid escape sequence '\d' http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py@116 PS12, Line 116: \ flake8: W605 invalid escape sequence '\d' http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py@116 PS12, Line 116: \ flake8: W605 invalid escape sequence '\d' http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py@116 PS12, Line 116: \ flake8: W605 invalid escape sequence '\d' http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py@116 PS12, Line 116: \ flake8: W605 invalid escape sequence '\d' http://gerrit.cloudera.org:8080/#/c/21074/12/tests/custom_cluster/test_query_expiration.py@116 PS12, Line 116: \ flake8: W605 invalid escape sequence '\d' -- To view, visit http://gerrit.cloudera.org:8080/21074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 Gerrit-Change-Number: 21074 Gerrit-PatchSet: 12 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 01 Apr 2024 23:54:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12602: Unregister queries on idle timeout
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21074 ) Change subject: IMPALA-12602: Unregister queries on idle timeout .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/21074/9/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/21074/9/be/src/service/impala-server.cc@2879 PS9, Line 2879: > Might not have been clear earlier. I think I can write a test for this, and Addressed. -- To view, visit http://gerrit.cloudera.org:8080/21074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 Gerrit-Change-Number: 21074 Gerrit-PatchSet: 12 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 01 Apr 2024 23:53:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12602: Unregister queries on idle timeout
Hello Riza Suminto, Jason Fehr, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21074 to look at the new patch set (#12). Change subject: IMPALA-12602: Unregister queries on idle timeout .. IMPALA-12602: Unregister queries on idle timeout Queries cancelled due to idle_query_timeout/QUERY_TIMEOUT_S are now also Unregistered to free any remaining memory, as you cannot fetch results from a cancelled query. Adds a new structure - idle_query_statuses_ - to retain Status messages for queries closed this way so that we can continue to return a clear error message if the client returns and requests query status or attempts to fetch results. This structure must be global because HS2 server can only identify a session ID from a query handle, and the query handle no longer exists. SessionState tracks queries added to idle_query_statuses_ so they can be cleared when the session is closed. Also ensures MarkInactive is called in ClientRequestState when Wait() completes. Previously WaitInternal would only MarkInactive on success, leaving any failed requests in an active state until explicitly closed or the session ended. The beeswax get_log RPC will not return the preserved error message or any warnings for these queries. It's also possible the summary and profile are rotated out of query log as the query is no longer inflight. This is an acceptable outcome as a client will likely not look for a log/summary/profile after it times out. Testing: updates test_query_expiration to verify number of waiting queries is only non-zero for queries cancelled by EXEC_TIME_LIMIT_S and not yet closed as an idle query. Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 --- M be/src/service/client-request-state.cc M be/src/service/impala-beeswax-server.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M docs/topics/impala_timeouts.xml M tests/custom_cluster/test_query_expiration.py 6 files changed, 132 insertions(+), 47 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/21074/12 -- To view, visit http://gerrit.cloudera.org:8080/21074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 Gerrit-Change-Number: 21074 Gerrit-PatchSet: 12 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-12602: Unregister queries on idle timeout
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21074 ) Change subject: IMPALA-12602: Unregister queries on idle timeout .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/21074/9/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/21074/9/be/src/service/impala-server.cc@2879 PS9, Line 2879: > Ack Might not have been clear earlier. I think I can write a test for this, and it's not working because of another bug that seriously limits the usability of this patch. Any query that ends in an exception and ends up in Waiting because the client hasn't fetched rows will never go inactive, and thus won't be cancelled by the idle timeout. -- To view, visit http://gerrit.cloudera.org:8080/21074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 Gerrit-Change-Number: 21074 Gerrit-PatchSet: 11 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 01 Apr 2024 23:39:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12602: Unregister queries on idle timeout
Michael Smith has removed a vote on this change. Change subject: IMPALA-12602: Unregister queries on idle timeout .. Removed Code-Review+2 by Riza Suminto -- To view, visit http://gerrit.cloudera.org:8080/21074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 Gerrit-Change-Number: 21074 Gerrit-PatchSet: 11 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-12771: Impala catalogd events-skipped may mark the wrong number
Sai Hemanth Gantasala has posted comments on this change. (
http://gerrit.cloudera.org:8080/21045 )
Change subject: IMPALA-12771: Impala catalogd events-skipped may mark the wrong
number
..
Patch Set 3:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/21045/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21045/3//COMMIT_MSG@10
PS3, Line 10: event ,the metric will also be increased, besides for some other
cases like alter
nit: "event, the"
http://gerrit.cloudera.org:8080/#/c/21045/3//COMMIT_MSG@11
PS3, Line 11: partition the event is skipped and the log is printed but the
events-skipped metric
Can you add more details in this commit message about what all events are
addressed in this patch to correctly update the events-skipped metric?
And can you also mention the test added in this patch? under the section
"testing:"
http://gerrit.cloudera.org:8080/#/c/21045/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:
http://gerrit.cloudera.org:8080/#/c/21045/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1055
PS1, Line 1055: } else if (numPartsRefreshed == -1) {
> Hi, the reason that I change the catalogOpExecutor_.reloadPartitionsIfExist
I think hdfsTable.reloadPartitionsFromNames() would return 0 only if the
partitions returned from metastore are zero. (simultaneous drop in metastore
while reloading partitions in impala). So I think it would make sense to me to
return 0 and make condition as "else".
http://gerrit.cloudera.org:8080/#/c/21045/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:
http://gerrit.cloudera.org:8080/#/c/21045/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1089
PS3, Line 1089:
metrics_.getCounter(MetastoreEventsProcessor.EVENTS_SKIPPED_METRIC).inc();
Shouldn't this be:
"metrics_.getCounter(MetastoreEventsProcessor.EVENTS_SKIPPED_METRIC).inc(partitions.size());"
http://gerrit.cloudera.org:8080/#/c/21045/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1094
PS3, Line 1094:
metrics_.getCounter(MetastoreEventsProcessor.EVENTS_SKIPPED_METRIC).inc();
nit: same as L#1089
http://gerrit.cloudera.org:8080/#/c/21045/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1098
PS3, Line 1098:
metrics_.getCounter(MetastoreEventsProcessor.EVENTS_SKIPPED_METRIC).inc();
nit: same as L#1089
http://gerrit.cloudera.org:8080/#/c/21045/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1112
PS3, Line 1112:
metrics_.getCounter(MetastoreEventsProcessor.EVENTS_SKIPPED_METRIC).inc();
nit: ".inc(partitionNames.size())"
http://gerrit.cloudera.org:8080/#/c/21045/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1117
PS3, Line 1117:
metrics_.getCounter(MetastoreEventsProcessor.EVENTS_SKIPPED_METRIC).inc();
nit: same as L#1112
http://gerrit.cloudera.org:8080/#/c/21045/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1121
PS3, Line 1121:
metrics_.getCounter(MetastoreEventsProcessor.EVENTS_SKIPPED_METRIC).inc();
nit: same as L#1121
http://gerrit.cloudera.org:8080/#/c/21045/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1745
PS3, Line 1745:
metrics_.getCounter(MetastoreEventsProcessor.EVENTS_SKIPPED_METRIC).inc();
Instead of incrementing the metric here, we can directly evaluate the boolean
value in the reloadTableFromCatalog() method and then increment the metric in
that method. So that we can avoid duplications in L#2326-2329, L#2538-2541,
L#2654-2657, L#2780-2783, L#3005-3008, L#3138-3141.
http://gerrit.cloudera.org:8080/#/c/21045/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
File
fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java:
http://gerrit.cloudera.org:8080/#/c/21045/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@237
PS3, Line 237: // the catalogd.
Can you update this description to also include Reload event and blacklisted
metadata objects?
Also, we increment this when event processor is not in active state(disabled).
--
To view, visit http://gerrit.cloudera.org:8080/21045
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aeb04e999b82187eb138c0b643ead259da22f1a
Gerrit-Change-Number: 21045
Gerrit-PatchSet: 3
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sai Hemanth Gantasala
[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 ) Change subject: IMPALA-12925: Fix decimal data type for external JDBC table .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15751/ : 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/21218 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b Gerrit-Change-Number: 21218 Gerrit-PatchSet: 5 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: gaurav singh Gerrit-Comment-Date: Mon, 01 Apr 2024 23:24:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12602: Unregister queries on idle timeout
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21074 ) Change subject: IMPALA-12602: Unregister queries on idle timeout .. Patch Set 11: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/21074/9/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/21074/9/be/src/service/impala-server.cc@2879 PS9, Line 2879: > Hmm. So far I can't find a way to cause there to be an exception without un Ack -- To view, visit http://gerrit.cloudera.org:8080/21074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 Gerrit-Change-Number: 21074 Gerrit-PatchSet: 11 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 01 Apr 2024 23:19:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12602: Unregister queries on idle timeout
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21074 ) Change subject: IMPALA-12602: Unregister queries on idle timeout .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/21074/9/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/21074/9/be/src/service/impala-server.cc@2879 PS9, Line 2879: > Hmm. So far I can't find a way to cause there to be an exception without un Ok, pretty sure there's another bug here. Whenever we RETURN_IF_ERROR after calling MarkActive on a ClientRequestState, we don't MarkInactive and hold onto a ref. So the query never goes inactive after encountering an exception. -- To view, visit http://gerrit.cloudera.org:8080/21074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 Gerrit-Change-Number: 21074 Gerrit-PatchSet: 11 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 01 Apr 2024 23:19:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12602: Unregister queries on idle timeout
Hello Riza Suminto, Jason Fehr, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21074 to look at the new patch set (#11). Change subject: IMPALA-12602: Unregister queries on idle timeout .. IMPALA-12602: Unregister queries on idle timeout Queries cancelled due to idle_query_timeout/QUERY_TIMEOUT_S are now also Unregistered to free any remaining memory, as you cannot fetch results from a cancelled query. Adds a new structure - idle_query_statuses_ - to retain Status messages for queries closed this way so that we can continue to return a clear error message if the client returns and requests query status or attempts to fetch results. This structure must be global because HS2 server can only identify a session ID from a query handle, and the query handle no longer exists. SessionState tracks queries added to idle_query_statuses_ so they can be cleared when the session is closed. The beeswax get_log RPC will not return the preserved error message or any warnings for these queries. It's also possible the summary and profile are rotated out of query log as the query is no longer inflight. This is an acceptable outcome as a client will likely not look for a log/summary/profile after it times out. Testing: updates test_query_expiration to verify number of waiting queries is only non-zero for queries cancelled by EXEC_TIME_LIMIT_S and not yet closed as an idle query. Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M docs/topics/impala_timeouts.xml M tests/custom_cluster/test_query_expiration.py 5 files changed, 111 insertions(+), 39 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/21074/11 -- To view, visit http://gerrit.cloudera.org:8080/21074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 Gerrit-Change-Number: 21074 Gerrit-PatchSet: 11 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-12602: Unregister queries on idle timeout
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21074 ) Change subject: IMPALA-12602: Unregister queries on idle timeout .. Patch Set 10: (2 comments) http://gerrit.cloudera.org:8080/#/c/21074/9/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/21074/9/be/src/service/impala-server.cc@2879 PS9, Line 2879: > Is there any test where preserved_status.ok() == false? Maybe test query wi Hmm. So far I can't find a way to cause there to be an exception without unregistering the query. Maybe there's no point preserving these messages because they'll always be "Query ... expired due to client inactivity". http://gerrit.cloudera.org:8080/#/c/21074/9/tests/custom_cluster/test_query_expiration.py File tests/custom_cluster/test_query_expiration.py: http://gerrit.cloudera.org:8080/#/c/21074/9/tests/custom_cluster/test_query_expiration.py@144 PS9, Line 144: assert ' expired due to ' in str(e) > Yes, that's what should be happening. Only expiration due to idle timeout i Done -- To view, visit http://gerrit.cloudera.org:8080/21074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 Gerrit-Change-Number: 21074 Gerrit-PatchSet: 10 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 01 Apr 2024 23:04:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table
Wenzhe Zhou has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/21218 ) Change subject: IMPALA-12925: Fix decimal data type for external JDBC table .. IMPALA-12925: Fix decimal data type for external JDBC table Decimal type is a primitive data type for Impala. Current code returns wrong values for columns with decimal data type in external JDBC tables. This patch fixes wrong values returned from JDBC data source, and supports pushing down decimal type of predicates to remote database and remote Impala. The decimal scales of the columns in external JDBC table must be no less than the decimal scales of the corresponding columns in the table of remote database. Otherwise, exception will be thrown since it may cause truncation of decimal data. Testing: - Added Planner test for pushing down decimal type of predicates. - Added end-to-end unit-tests for tables with decimal type of columns for Postgres, MySQL, and Impala-to-Impala. - Passed core-tests. Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b --- M fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java M fe/src/main/java/org/apache/impala/extdatasource/jdbc/util/QueryConditionUtil.java M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java M testdata/bin/clean-mysql-env.sh M testdata/bin/create-ext-data-source-table.sql M testdata/bin/load-ext-data-sources.sh M testdata/bin/setup-mysql-env.sh M testdata/workloads/functional-planner/queries/PlannerTest/data-source-tables.test M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables-predicates.test M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables.test M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test M testdata/workloads/functional-query/queries/QueryTest/mysql-ext-jdbc-tables.test 13 files changed, 524 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/21218/5 -- To view, visit http://gerrit.cloudera.org:8080/21218 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b Gerrit-Change-Number: 21218 Gerrit-PatchSet: 5 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: gaurav singh
[Impala-ASF-CR] IMPALA-12920: Support ai generate text built-in function for OpenAI's chat completion API
Yida Wu has posted comments on this change. (
http://gerrit.cloudera.org:8080/21168 )
Change subject: IMPALA-12920: Support ai_generate_text built-in function for
OpenAI's chat completion API
..
Patch Set 5: Code-Review+1
(4 comments)
LGTM! Some minor comments.
http://gerrit.cloudera.org:8080/#/c/21168/5/be/src/exprs/ai-functions-ir.cc
File be/src/exprs/ai-functions-ir.cc:
http://gerrit.cloudera.org:8080/#/c/21168/5/be/src/exprs/ai-functions-ir.cc@83
PS5, Line 83: != nullptr);
nit. seems we can save one line here because above line has enough space
http://gerrit.cloudera.org:8080/#/c/21168/5/be/src/exprs/ai-functions-ir.cc@225
PS5, Line 225: kudu::EasyCurl curl;
:
curl.set_timeout(kudu::MonoDelta::FromSeconds(FLAGS_ai_connection_timeout_s));
: curl.set_fail_on_http_error(true);
: kudu::faststring resp;
: kudu::Status status = curl.PostToURL(endpoint_str,
payload_str, , headers);
: VLOG(2) << "AI Generate Text: \noriginal response: " <<
resp.ToString();
: if (!status.ok()) {
: string msg = status.ToString();
: return StringVal::CopyFrom(
: ctx, reinterpret_cast(msg.c_str()),
msg.size());
: }
Can we add a summary comment about what this part of code is doing? I think it
does the real thing to contact with the Ai endpoint, and can be critical to
performance, maybe a comment can make it clearer for others to understand its
importance.
http://gerrit.cloudera.org:8080/#/c/21168/5/be/src/exprs/ai-functions-ir.cc@236
PS5, Line 236: JSON string
nit. how about saying "response JSON string"
http://gerrit.cloudera.org:8080/#/c/21168/5/be/src/exprs/ai-functions-ir.cc@251
PS5, Line 251: response
Thinking about whether we need to worry about the case when the length of
response is very large or unexpected large. How about we print a warning log if
the response is larger than certain size?
--
To view, visit http://gerrit.cloudera.org:8080/21168
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4446957f6030bab1f985fdd69185c3da07d7c4b
Gerrit-Change-Number: 21168
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Rawat
Gerrit-Reviewer: Abhishek Rawat
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Yida Wu
Gerrit-Comment-Date: Mon, 01 Apr 2024 22:19:10 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 ) Change subject: IMPALA-12925: Fix decimal data type for external JDBC table .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15750/ : 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/21218 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b Gerrit-Change-Number: 21218 Gerrit-PatchSet: 4 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: gaurav singh Gerrit-Comment-Date: Mon, 01 Apr 2024 19:20:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table
Wenzhe Zhou has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/21218 ) Change subject: IMPALA-12925: Fix decimal data type for external JDBC table .. IMPALA-12925: Fix decimal data type for external JDBC table Decimal type is a primitive data type for Impala. Current code returns wrong values for columns with decimal data type in external JDBC tables. This patch fixes wrong values returned from JDBC data source, and supports pushing down decimal type of predicates to remote database and remote Impala. The decimal scales of the columns in external JDBC table must be no less than the decimal scales of the corresponding columns in the table of remote database. Otherwise, exception will be thrown since it may cause truncation of decimal data. Testing: - Added Planner test for pushing down decimal type of predicates. - Added end-to-end unit-tests for tables with decimal type of columns for Postgres, MySQL, and Impala-to-Impala. - Passed core-tests. Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b --- M fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java M fe/src/main/java/org/apache/impala/extdatasource/jdbc/util/QueryConditionUtil.java M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java M testdata/bin/clean-mysql-env.sh M testdata/bin/create-ext-data-source-table.sql M testdata/bin/load-ext-data-sources.sh M testdata/bin/setup-mysql-env.sh M testdata/workloads/functional-planner/queries/PlannerTest/data-source-tables.test M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables-predicates.test M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables.test M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test M testdata/workloads/functional-query/queries/QueryTest/mysql-ext-jdbc-tables.test 13 files changed, 490 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/21218/4 -- To view, visit http://gerrit.cloudera.org:8080/21218 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b Gerrit-Change-Number: 21218 Gerrit-PatchSet: 4 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: gaurav singh
[Impala-ASF-CR] IMPALA-12920: Support ai generate text built-in function for OpenAI's chat completion API
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21168 ) Change subject: IMPALA-12920: Support ai_generate_text built-in function for OpenAI's chat completion API .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15749/ : 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/21168 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id4446957f6030bab1f985fdd69185c3da07d7c4b Gerrit-Change-Number: 21168 Gerrit-PatchSet: 5 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Mon, 01 Apr 2024 17:12:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12920: Support ai generate text built-in function for OpenAI's chat completion API
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21168 ) Change subject: IMPALA-12920: Support ai_generate_text built-in function for OpenAI's chat completion API .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15748/ : 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/21168 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id4446957f6030bab1f985fdd69185c3da07d7c4b Gerrit-Change-Number: 21168 Gerrit-PatchSet: 4 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Mon, 01 Apr 2024 17:00:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12920: Support ai generate text built-in function for OpenAI's chat completion API
Abhishek Rawat has uploaded a new patch set (#5). (
http://gerrit.cloudera.org:8080/21168 )
Change subject: IMPALA-12920: Support ai_generate_text built-in function for
OpenAI's chat completion API
..
IMPALA-12920: Support ai_generate_text built-in function for OpenAI's chat
completion API
Added support for following built-in functions:
- ai_generate_text_default(prompt)
- ai_generate_text(ai_endpoint, prompt, ai_model,
ai_api_key_jceks_secret, additional_params)
'ai_endpoint', 'ai_model' and 'ai_api_key_jceks_secret' are flagfile
options. 'ai_generate_text_default(prompt)' syntax expects all these
to be set to proper values. The other syntax, will try to use the
provided input parameter values, but fallback to instance level values
if the inputs are NULL or empty.
Only public OpenAI (api.openai.com) and Azure OpenAI (openai.azure.com)
API endpoints are currently supported.
Exposed these functions in FunctionContext so that they can also be
called from UDFs:
- ai_generate_text_default(context, model)
- ai_generate_text(context, ai_endpoint, prompt, ai_model,
ai_api_key_jceks_secret, additional_params)
Testing:
- Added unit tests for AiGenerateTextInternal function
- Ran manual tests to make sure Impala can talk with OpenAI LLMs using
'ai_generate_text' built-in function. Example sql:
select ai_generate_text("https://api.openai.com/v1/chat/completions;,
"hello", "gpt-3.5-turbo", "open-ai-key",
'{"temperature": 0.9, "model": "gpt-4"}')
- Tested using standalone UDF SDK and made sure that the UDFs can invoke
BuiltInFunctions (ai_generate_text and ai_generate_text_default)
Change-Id: Id4446957f6030bab1f985fdd69185c3da07d7c4b
---
M be/src/exprs/CMakeLists.txt
A be/src/exprs/ai-functions-ir.cc
A be/src/exprs/ai-functions.h
M be/src/exprs/expr-test.cc
M be/src/exprs/scalar-expr-evaluator.cc
M be/src/runtime/exec-env.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/udf/udf-internal.h
M be/src/udf/udf.cc
M be/src/udf/udf.h
M be/src/udf_samples/udf-sample.cc
M be/src/udf_samples/udf-sample.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
15 files changed, 593 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/21168/5
--
To view, visit http://gerrit.cloudera.org:8080/21168
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4446957f6030bab1f985fdd69185c3da07d7c4b
Gerrit-Change-Number: 21168
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Rawat
Gerrit-Reviewer: Abhishek Rawat
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Yida Wu
[Impala-ASF-CR] IMPALA-12920: Support ai generate text built-in function for OpenAI's chat completion API
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/21168 ) Change subject: IMPALA-12920: Support ai_generate_text built-in function for OpenAI's chat completion API .. Patch Set 4: (7 comments) http://gerrit.cloudera.org:8080/#/c/21168/3/be/src/exprs/ai-functions-ir.cc File be/src/exprs/ai-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/21168/3/be/src/exprs/ai-functions-ir.cc@74 PS3, Line 74: > Maybe declare a const string for "https://;, we can also use size of it ins Done http://gerrit.cloudera.org:8080/#/c/21168/3/be/src/exprs/ai-functions-ir.cc@75 PS3, Line 75: st > no need spaces Done http://gerrit.cloudera.org:8080/#/c/21168/3/be/src/exprs/ai-functions-ir.cc@79 PS3, Line 79: onst string& end > Maybe declare a const string for it? Also same to "api.openai.com" Done http://gerrit.cloudera.org:8080/#/c/21168/3/be/src/exprs/ai-functions-ir.cc@85 PS3, Line 85: > Thinking may be good the have const strings for all the json field names, e Sure, I added some constants, I think const strings make sense if we're using these fields at multiple places. However, for code readability string literals might make more sense. http://gerrit.cloudera.org:8080/#/c/21168/3/be/src/exprs/ai-functions-ir.cc@122 PS3, Line 122: > should be two spaces Done http://gerrit.cloudera.org:8080/#/c/21168/3/be/src/exprs/ai-functions-ir.cc@196 PS3, Line 196: > doesn't need the namespace Done http://gerrit.cloudera.org:8080/#/c/21168/3/be/src/exprs/string-functions-ir.cc File be/src/exprs/string-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/21168/3/be/src/exprs/string-functions-ir.cc@1950 PS3, Line 1950: > Seems the change is unnecessary. Done -- To view, visit http://gerrit.cloudera.org:8080/21168 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id4446957f6030bab1f985fdd69185c3da07d7c4b Gerrit-Change-Number: 21168 Gerrit-PatchSet: 4 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Mon, 01 Apr 2024 16:36:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12920: Support ai generate text built-in function for OpenAI's chat completion API
Abhishek Rawat has uploaded a new patch set (#4). (
http://gerrit.cloudera.org:8080/21168 )
Change subject: IMPALA-12920: Support ai_generate_text built-in function for
OpenAI's chat completion API
..
IMPALA-12920: Support ai_generate_text built-in function for OpenAI's chat
completion API
Added support for following built-in functions:
- ai_generate_text_default(prompt)
- ai_generate_text(ai_endpoint, prompt, ai_model,
ai_api_key_jceks_secret, additional_params)
'ai_endpoint', 'ai_model' and 'ai_api_key_jceks_secret' are flagfile
options. 'ai_generate_text_default(prompt)' syntax expects all these
to be set to proper values. The other syntax, will try to use the
provided input parameter values, but fallback to instance level values
if the inputs are NULL or empty.
Only public OpenAI (api.openai.com) and Azure OpenAI (openai.azure.com)
API endpoints are currently supported.
Exposed these functions in FunctionContext so that they can also be
called from UDFs:
- ai_generate_text_default(context, model)
- ai_generate_text(context, ai_endpoint, prompt, ai_model,
ai_api_key_jceks_secret, additional_params)
Testing:
- Added unit tests for AiGenerateTextInternal function
- Ran manual tests to make sure Impala can talk with OpenAI LLMs using
'ai_generate_text' built-in function. Example sql:
select ai_generate_text("https://api.openai.com/v1/chat/completions;,
"hello", "gpt-3.5-turbo", "open-ai-key",
'{"temperature": 0.9, "model": "gpt-4"}')
- Tested using standalone UDF SDK and made sure that the UDFs can invoke
BuiltInFunctions (ai_generate_text and ai_generate_text_default)
Change-Id: Id4446957f6030bab1f985fdd69185c3da07d7c4b
---
M be/src/exprs/CMakeLists.txt
A be/src/exprs/ai-functions-ir.cc
A be/src/exprs/ai-functions.h
M be/src/exprs/expr-test.cc
M be/src/exprs/scalar-expr-evaluator.cc
M be/src/runtime/exec-env.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/udf/udf-internal.h
M be/src/udf/udf.cc
M be/src/udf/udf.h
M be/src/udf_samples/udf-sample.cc
M be/src/udf_samples/udf-sample.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
15 files changed, 593 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/21168/4
--
To view, visit http://gerrit.cloudera.org:8080/21168
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4446957f6030bab1f985fdd69185c3da07d7c4b
Gerrit-Change-Number: 21168
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Yida Wu
[Impala-ASF-CR] IMPALA-12920: Support ai generate text built-in function for OpenAI's chat completion API
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/21168 )
Change subject: IMPALA-12920: Support ai_generate_text built-in function for
OpenAI's chat completion API
..
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21168/4/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
http://gerrit.cloudera.org:8080/#/c/21168/4/be/src/exprs/expr-test.cc@11206
PS4, Line 11206: ctx, StringVal("https://ai.com;), prompt, model,
jceks_secret, json_params, dry_run);
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/21168/4/be/src/exprs/expr-test.cc@11264
PS4, Line 11264: ctx, StringVal::null(), prompt, StringVal::null(),
jceks_secret, json_params, dry_run);
line too long (93 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/21168
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4446957f6030bab1f985fdd69185c3da07d7c4b
Gerrit-Change-Number: 21168
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat
Gerrit-Reviewer: Abhishek Rawat
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Yida Wu
Gerrit-Comment-Date: Mon, 01 Apr 2024 16:37:21 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12152: Add query option to wait for events sync up
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20131 ) Change subject: IMPALA-12152: Add query option to wait for events sync up .. Patch Set 14: (1 comment) http://gerrit.cloudera.org:8080/#/c/20131/12/common/thrift/CatalogService.thrift File common/thrift/CatalogService.thrift: http://gerrit.cloudera.org:8080/#/c/20131/12/common/thrift/CatalogService.thrift@751 PS12, Line 751: > We need the same mechanism as we use to support sync_ddl: A simpler solution is just returning the updated/deleted catalog objects in the RPC response. -- To view, visit http://gerrit.cloudera.org:8080/20131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36ac941bb2c2217b09fcfa2eb567b011b38efa2a Gerrit-Change-Number: 20131 Gerrit-PatchSet: 14 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 01 Apr 2024 13:48:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12609: Implement SHOW METADATA TABLES IN statement to list Iceberg Metadata tables
Daniel Becker has removed a vote on this change. Change subject: IMPALA-12609: Implement SHOW METADATA TABLES IN statement to list Iceberg Metadata tables .. Removed Code-Review+2 by Daniel Becker -- To view, visit http://gerrit.cloudera.org:8080/21026 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ide10ccf10fc0abf5c270119ba7092c67e712ec49 Gerrit-Change-Number: 21026 Gerrit-PatchSet: 15 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12609: Implement SHOW METADATA TABLES IN statement to list Iceberg Metadata tables
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21026 ) Change subject: IMPALA-12609: Implement SHOW METADATA TABLES IN statement to list Iceberg Metadata tables .. Patch Set 15: (1 comment) http://gerrit.cloudera.org:8080/#/c/21026/15/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/21026/15/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@1258 PS15, Line 1258: functional_parquet Removed the table name because "functional_parquet.iceberg_query_metadata" and "functional_parquet.*.*" varied with builds. -- To view, visit http://gerrit.cloudera.org:8080/21026 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ide10ccf10fc0abf5c270119ba7092c67e712ec49 Gerrit-Change-Number: 21026 Gerrit-PatchSet: 15 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 01 Apr 2024 10:47:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12609: Implement SHOW METADATA TABLES IN statement to list Iceberg Metadata tables
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21026 ) Change subject: IMPALA-12609: Implement SHOW METADATA TABLES IN statement to list Iceberg Metadata tables .. Patch Set 15: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21026 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ide10ccf10fc0abf5c270119ba7092c67e712ec49 Gerrit-Change-Number: 21026 Gerrit-PatchSet: 15 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 01 Apr 2024 10:40:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 ) Change subject: IMPALA-12925: Fix decimal data type for external JDBC table .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15747/ : 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/21218 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b Gerrit-Change-Number: 21218 Gerrit-PatchSet: 3 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: gaurav singh Gerrit-Comment-Date: Mon, 01 Apr 2024 06:13:10 + Gerrit-HasComments: No
