[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21029 ) Change subject: IMPALA-12543: Detect self-events before finishing DDL .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15402/ : 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/21029 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79 Gerrit-Change-Number: 21029 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Tue, 05 Mar 2024 06:58:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21029 ) Change subject: IMPALA-12543: Detect self-events before finishing DDL .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15401/ : 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/21029 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79 Gerrit-Change-Number: 21029 Gerrit-PatchSet: 8 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Tue, 05 Mar 2024 06:48:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21029 ) Change subject: IMPALA-12543: Detect self-events before finishing DDL .. Patch Set 9: (8 comments) http://gerrit.cloudera.org:8080/#/c/21029/7/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/21029/7/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@909 PS7, Line 909: LOG.info("Received {} events. First event id: {}.", response.getEvents().size(), : (response.getEvents().size() > 0 ? response.getEvents().get(0).getEventId() : : "none")); : if (filter > nit: It'd be more readable to use the message template: Done http://gerrit.cloudera.org:8080/#/c/21029/5/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/21029/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1683 PS5, Line 1683: if (msTbl.getPartitionKeysSize() == > +1. Could you add a comment for this? Done http://gerrit.cloudera.org:8080/#/c/21029/7/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/21029/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1066 PS7, Line 1066: * Register the new version number into table's in-flight events. > Could you add a comment for when should we use this, e.g. before invoking t Done http://gerrit.cloudera.org:8080/#/c/21029/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@ PS7, Line : */ : public void validateInProgressModificationComplete() { > nit: please add error messages for these Done http://gerrit.cloudera.org:8080/#/c/21029/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1366 PS7, Line 1366: } > nit: let's add an error message for Preconditions. Done http://gerrit.cloudera.org:8080/#/c/21029/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4493 PS7, Line 4493: } > Might be an existing issue, but should we still do this when 'addedHmsParti Probably not, but I prefer matching the old-behavior like in old-code, L1147. We can investigate the impact of not doing this (and another seemingly unnecessary code in L5307) in separate patch and see which test will break. http://gerrit.cloudera.org:8080/#/c/21029/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@5819 PS7, Line 5819: KuduCatalogOpExecutor.validateKuduTblExists(msTbl); > Just realized this is invoked in alterTable() instead of alterView().. In a This probably need to be handled by the switch anyway for SET_VIEW_PROPERTIES enum value. http://gerrit.cloudera.org:8080/#/c/21029/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6135 PS7, Line 6135: } > Should we do this before L6106 where the events will be generated, and hand This is one spot where I'm no sure about. What if some alterPartitions went through and the rest is not? Fortunately, I think partition-level events handling still acquire table locks. >From IMPALA-12461 commit message: "Postponing solving this for partition level events as that would be more complex, as both the partition list and the partitions' in-flight event lists would need to be protected from parallel operations that add/remove partitions." So cancelInflightEvent is not required here, just like other alter partition operation (ie., alterTableDropPartition and alterTableAddPartitions). -- To view, visit http://gerrit.cloudera.org:8080/21029 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79 Gerrit-Change-Number: 21029 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Tue, 05 Mar 2024 06:36:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL
Hello Quanlong Huang, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21029 to look at the new patch set (#9). Change subject: IMPALA-12543: Detect self-events before finishing DDL .. IMPALA-12543: Detect self-events before finishing DDL test_iceberg_self_events has been flaky for not having tbls_refreshed_before equal to tbls_refreshed_after in-between query executions. This flakiness happens in ARM, TEST_JDK_VERSION 11, and TEST_JDK_VERSION 17 environments. Further investigation reveals concurrency bug due to db/table level locks is not taken during db/table self-events check (IMPALA-12461 part1). The order of ALTER TABLE operation is as follow: 1. alter table starts in CatalogOpExecutor 2. table level lock is taken 3. HMS RPC starts (CatalogOpExecutor.applyAlterTable()) 4. HMS generates the event 5. HMS RPC returns 6. table is reloaded 7. catalog version is added to inflight event list 8. table level lock is releases Meanwhile the event processor thread fetches the new event after 4 and before 7. Because of IMPALA-12461 (part 1), it can also finish self-events checking before reaching 7. Before IMPALA-12461, self-events would have needed to wait for 8. Note that this issue is only relevant for table level events, as self-events checking for partition level events still takes table lock. This patch fix the issue by adding newCatalogVersion to the table's inflight event list before updating HMS. If HMS update does not complete (ie., an exception is thrown), the new newCatalogVersion that was added is then removed. This patch also fix few smaller issues, including: - Avoid incrementing EVENTS_SKIPPED_METRIC if numFilteredEvents == 0 in MetastoreEventFactory.getFilteredEvents(). - Increment EVENTS_SKIPPED_METRIC in MetastoreTableEvent.reloadTableFromCatalog() if table is already in the middle of reloading (revealed through flaky test_skipping_older_events). - Rephrase misleading log message in MetastoreEventProcessor.getNextMetastoreEvents(). Testing: - Pass exhaustive tests. Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79 --- M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/Table.java 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 5 files changed, 273 insertions(+), 141 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/21029/9 -- To view, visit http://gerrit.cloudera.org:8080/21029 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79 Gerrit-Change-Number: 21029 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Sai Hemanth Gantasala
[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL
Hello Quanlong Huang, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21029 to look at the new patch set (#8). Change subject: IMPALA-12543: Detect self-events before finishing DDL .. IMPALA-12543: Detect self-events before finishing DDL test_iceberg_self_events has been flaky for not having tbls_refreshed_before equal to tbls_refreshed_after in-between query executions. This flakiness happens in ARM, TEST_JDK_VERSION 11, and TEST_JDK_VERSION 17 environments. Further investigation reveals concurrency bug due to db/table level locks is not taken during db/table self-events check (IMPALA-12461 part1). The order of ALTER TABLE operation is as follow: 1. alter table starts in CatalogOpExecutor 2. table level lock is taken 3. HMS RPC starts (CatalogOpExecutor.applyAlterTable()) 4. HMS generates the event 5. HMS RPC returns 6. table is reloaded 7. catalog version is added to inflight event list 8. table level lock is releases Meanwhile the event processor thread fetches the new event after 4 and before 7. Because of IMPALA-12461 (part 1), it can also finish self-events checking before reaching 7. Before IMPALA-12461, self-events would have needed to wait for 8. Note that this issue is only relevant for table level events, as self-events checking for partition level events still takes table lock. This patch fix the issue by adding newCatalogVersion to the table's inflight event list before updating HMS. If HMS update does not complete (ie., an exception is thrown), the new newCatalogVersion that was added is then removed. This patch also fix few smaller issues, including: - Avoid incrementing EVENTS_SKIPPED_METRIC if numFilteredEvents == 0 in MetastoreEventFactory.getFilteredEvents(). - Increment EVENTS_SKIPPED_METRIC in MetastoreTableEvent.reloadTableFromCatalog() if table is already in the middle of reloading (revealed through flaky test_skipping_older_events). - Rephrase misleading log message in MetastoreEventProcessor.getNextMetastoreEvents(). Testing: - Pass exhaustive tests. Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79 --- M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/Table.java 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 5 files changed, 270 insertions(+), 141 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/21029/8 -- To view, visit http://gerrit.cloudera.org:8080/21029 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79 Gerrit-Change-Number: 21029 Gerrit-PatchSet: 8 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Sai Hemanth Gantasala
[Impala-ASF-CR] IMPALA-12802: Support ALTER TABLE for JDBC tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21088 ) Change subject: IMPALA-12802: Support ALTER TABLE for JDBC tables .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15400/ : 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/21088 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5ebb5de2c686d2015db78641f78299dd5f33621e Gerrit-Change-Number: 21088 Gerrit-PatchSet: 3 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: gaurav singh Gerrit-Comment-Date: Tue, 05 Mar 2024 06:14:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12802: Support ALTER TABLE for JDBC tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21088 ) Change subject: IMPALA-12802: Support ALTER TABLE for JDBC tables .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15399/ : 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/21088 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5ebb5de2c686d2015db78641f78299dd5f33621e Gerrit-Change-Number: 21088 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: gaurav singh Gerrit-Comment-Date: Tue, 05 Mar 2024 06:03:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12802: Support ALTER TABLE for JDBC tables
Wenzhe Zhou has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/21088 ) Change subject: IMPALA-12802: Support ALTER TABLE for JDBC tables .. IMPALA-12802: Support ALTER TABLE for JDBC tables IMPALA-12793 changes the syntax for creating JDBC table. The configurations of connection credentials - url, username, password, jdbc driver, etc, are set as table properties. This patch allows user to change these table properties, or edit columns via ALTER TABLE statement. Testing: - Added frontend analysis unit-tests. - Added end-to-end unit-test. - Passed Core tests Change-Id: I5ebb5de2c686d2015db78641f78299dd5f33621e --- M fe/src/main/java/org/apache/impala/analysis/AlterTableAddColsStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableAddDropRangePartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableDropColStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableExecuteExpireSnapshotsStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableExecuteRollbackStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableExecuteStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewRenameStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetCachedStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetColumnStats.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetPartitionSpecStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableUnSetTblProperties.java M fe/src/main/java/org/apache/impala/analysis/AlterViewSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterViewSetTblProperties.java M fe/src/main/java/org/apache/impala/analysis/AlterViewUnSetTblProperties.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/FeDataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalDataSourceTable.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test 33 files changed, 453 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/21088/3 -- To view, visit http://gerrit.cloudera.org:8080/21088 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5ebb5de2c686d2015db78641f78299dd5f33621e Gerrit-Change-Number: 21088 Gerrit-PatchSet: 3 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-12802: Support ALTER TABLE for JDBC tables
Wenzhe Zhou has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21088 Change subject: IMPALA-12802: Support ALTER TABLE for JDBC tables .. IMPALA-12802: Support ALTER TABLE for JDBC tables IMPALA-12793 changes the syntax for creating JDBC table. The configurations of connection credentials - url, username, password, jdbc driver, etc, are set as table properties. This patch allows user to change these table properties, or edit columns via ALTER TABLE statement. Testing: - Added frontend analysis unit-tests. - Added end-to-end unit-test. - Passed Core tests Change-Id: I5ebb5de2c686d2015db78641f78299dd5f33621e --- M fe/src/main/java/org/apache/impala/analysis/AlterTableAddColsStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableAddDropRangePartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableDropColStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableExecuteExpireSnapshotsStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableExecuteRollbackStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableExecuteStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewRenameStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetCachedStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetColumnStats.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetPartitionSpecStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableUnSetTblProperties.java M fe/src/main/java/org/apache/impala/analysis/AlterViewSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterViewSetTblProperties.java M fe/src/main/java/org/apache/impala/analysis/AlterViewUnSetTblProperties.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/FeDataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalDataSourceTable.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test 33 files changed, 453 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/21088/2 -- To view, visit http://gerrit.cloudera.org:8080/21088 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I5ebb5de2c686d2015db78641f78299dd5f33621e Gerrit-Change-Number: 21088 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-12802: Support ALTER TABLE for JDBC tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21088 ) Change subject: IMPALA-12802: Support ALTER TABLE for JDBC tables .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/21088/2/fe/src/main/java/org/apache/impala/analysis/AlterTableUnSetTblProperties.java File fe/src/main/java/org/apache/impala/analysis/AlterTableUnSetTblProperties.java: http://gerrit.cloudera.org:8080/#/c/21088/2/fe/src/main/java/org/apache/impala/analysis/AlterTableUnSetTblProperties.java@157 PS2, Line 157: throw new AnalysisException(String.format("Unsetting the '%s' table property is " + line too long (91 > 90) -- To view, visit http://gerrit.cloudera.org:8080/21088 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5ebb5de2c686d2015db78641f78299dd5f33621e Gerrit-Change-Number: 21088 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 05 Mar 2024 05:39:40 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12771: Impala catalogd events-skipped may mark the wrong number
cclive1...@gmail.com 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 1: (3 comments) 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) { > I think using "else" without the condition is more robust in case the retur Hi, the reason that I change the catalogOpExecutor_.reloadPartitionsIfExist result from 0 to -1 for cases like : table does not exist or table is IncompleteTable. This means the table has no need to reload the partition, -1 is just a marker for me , But if the result is 0, that may means the function(hdfsTable.reloadPartitionsFromNames() to do real partition may has been executed but the result is 0 which means the event is not skipped but been executed. http://gerrit.cloudera.org:8080/#/c/21045/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1056 PS1, Line 1056: inc > Please add a debug log when increasing this counter. ok http://gerrit.cloudera.org:8080/#/c/21045/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2341 PS1, Line 2341: debugLog("Incremented skipped metric to " + metrics_ > We can improve this to add the reason - "since no partitions were added" ok -- 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: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 05 Mar 2024 03:07:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 10: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 10 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 05 Mar 2024 02:51:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. IMPALA-12864: Deflake test_query_log_size_in_bytes. test_query_log_size_in_bytes is flaky in exhaustive builds. The expected QueryStateRecord is off by around 100KB per query, indicating that the test query might be too complex and lead to variability in final query profile that is being archived. This patch simplifies the test query and relaxes the assertion. This patch also adds QUERY_LOG_EST_TOTAL_BYTES metric (key name "impala-server.query-log-est-total-bytes") that is validated as well in test_query_log_size_in_bytes. query-state-record-test.cc is modified a bit to resolve segmentation fault issue when building unified-be-test-validated-executable target run_clang_tidy.sh. Testing: - Pass test_query_log_size_in_bytes in exhaustive build. Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Reviewed-on: http://gerrit.cloudera.org:8080/21100 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/service/CMakeLists.txt M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-state-record-test.cc M be/src/service/query-state-record.h M be/src/util/impalad-metrics.cc M be/src/util/impalad-metrics.h M bin/run_clang_tidy.sh M common/thrift/metrics.json M tests/custom_cluster/test_web_pages.py 10 files changed, 55 insertions(+), 29 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 11 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] [WIP]IMPALA-12832: Implicit invalidate metadata on event failures
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21065 ) Change subject: [WIP]IMPALA-12832: Implicit invalidate metadata on event failures .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15398/ : 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/21065 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia67fc04c995802d3b6b56f79564bf0954b012c6c Gerrit-Change-Number: 21065 Gerrit-PatchSet: 11 Gerrit-Owner: Anonymous Coward 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, 05 Mar 2024 02:39:38 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP]IMPALA-12832: Implicit invalidate metadata on event failures
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21065 ) Change subject: [WIP]IMPALA-12832: Implicit invalidate metadata on event failures .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15397/ : 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/21065 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia67fc04c995802d3b6b56f79564bf0954b012c6c Gerrit-Change-Number: 21065 Gerrit-PatchSet: 10 Gerrit-Owner: Anonymous Coward 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, 05 Mar 2024 02:37:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/21083 ) Change subject: IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax .. Patch Set 1: (2 comments) This is a welcome addition, I had some ideas about structuring it differently. http://gerrit.cloudera.org:8080/#/c/21083/1/docs/topics/impala_alter_table.xml File docs/topics/impala_alter_table.xml: http://gerrit.cloudera.org:8080/#/c/21083/1/docs/topics/impala_alter_table.xml@515 PS1, Line 515: To change the column or order the records are sorted by: I found this feature a bit confusing. I tried to come up with a simple summary, the best I could come up with quickly was "To specify a sort order for new records that are added to the table" http://gerrit.cloudera.org:8080/#/c/21083/1/docs/topics/impala_alter_table.xml@521 PS1, Line 521: Specifying the sort order is optional. The default sort order is LEXICAL. This seems useful. I wonder if this should be merged into the doc in https://impala.apache.org/docs/build/html/topics/impala_create_table.html as that has a lot of good detail. Then maybe we can somehow have a summary here and link to create table for the details. What do you think? -- To view, visit http://gerrit.cloudera.org:8080/21083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieb348d8395a6140f0be200d73e2f22fded9a5116 Gerrit-Change-Number: 21083 Gerrit-PatchSet: 1 Gerrit-Owner: Noemi Pap-Takacs Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 05 Mar 2024 02:28:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP]IMPALA-12832: Implicit invalidate metadata on event failures
Sai Hemanth Gantasala has uploaded a new patch set (#11) to the change originally created by k.venureddy2...@gmail.com. ( http://gerrit.cloudera.org:8080/21065 ) Change subject: [WIP]IMPALA-12832: Implicit invalidate metadata on event failures .. [WIP]IMPALA-12832: Implicit invalidate metadata on event failures At present, failure in event processing needs manual invalidate metadata. This patch implicitly invalidates the table upon failures in processing of table events with new 'process_event_failure' flag. And a new 'auto_global_invalidate_metadata' flag is added to global invalidate metadata automatically when event processor goes to non-active state. Note: Also introduced a config 'inject_process_event_failure_event_types' for automated tests to simulate event processor failures. This config is used to specify what event types can be intentionally failed. This config should only be used for testing purpose. Testing: - Added end-to-end tests to mimic failures in event processor and verified that event processor is active - Added unit test to verify the 'auto_global_invalidate_metadata' config - Passed FE tests Co-Authored-by: Sai Hemanth Gantasala Change-Id: Ia67fc04c995802d3b6b56f79564bf0954b012c6c --- M be/src/catalog/catalog-server.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java 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/BackendConfig.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M tests/common/impala_test_suite.py A tests/metadata/__init__.py M tests/metadata/test_event_processing.py A tests/metadata/test_event_processing_base.py A tests/metadata/test_event_processing_error.py M tests/util/event_processor_utils.py 16 files changed, 934 insertions(+), 305 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/21065/11 -- To view, visit http://gerrit.cloudera.org:8080/21065 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia67fc04c995802d3b6b56f79564bf0954b012c6c Gerrit-Change-Number: 21065 Gerrit-PatchSet: 11 Gerrit-Owner: Anonymous Coward 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] [WIP]IMPALA-12832: Implicit invalidate metadata on event failures
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21065 ) Change subject: [WIP]IMPALA-12832: Implicit invalidate metadata on event failures .. Patch Set 11: (6 comments) http://gerrit.cloudera.org:8080/#/c/21065/11/tests/metadata/test_event_processing_error.py File tests/metadata/test_event_processing_error.py: http://gerrit.cloudera.org:8080/#/c/21065/11/tests/metadata/test_event_processing_error.py@47 PS11, Line 47: flake8: W291 trailing whitespace http://gerrit.cloudera.org:8080/#/c/21065/11/tests/metadata/test_event_processing_error.py@47 PS11, Line 47: catalogd_args="--catalog_topic_mode=minimal " line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/21065/11/tests/metadata/test_event_processing_error.py@48 PS11, Line 48: flake8: W291 trailing whitespace http://gerrit.cloudera.org:8080/#/c/21065/11/tests/metadata/test_event_processing_error.py@48 PS11, Line 48: "--process_event_failure=true " line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/21065/11/tests/metadata/test_event_processing_error.py@281 PS11, Line 281: # flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/21065/11/tests/metadata/test_event_processing_error.py@286 PS11, Line 286: + flake8: E226 missing whitespace around arithmetic operator -- To view, visit http://gerrit.cloudera.org:8080/21065 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia67fc04c995802d3b6b56f79564bf0954b012c6c Gerrit-Change-Number: 21065 Gerrit-PatchSet: 11 Gerrit-Owner: Anonymous Coward 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, 05 Mar 2024 02:15:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP]IMPALA-12832: Implicit invalidate metadata on event failures
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21065 ) Change subject: [WIP]IMPALA-12832: Implicit invalidate metadata on event failures .. Patch Set 10: (29 comments) http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py File tests/metadata/test_event_processing_error.py: http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@47 PS10, Line 47: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@48 PS10, Line 48: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@63 PS10, Line 63: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@64 PS10, Line 64: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@80 PS10, Line 80: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@81 PS10, Line 81: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@99 PS10, Line 99: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@100 PS10, Line 100: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@119 PS10, Line 119: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@120 PS10, Line 120: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@136 PS10, Line 136: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@137 PS10, Line 137: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@157 PS10, Line 157: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@158 PS10, Line 158: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@176 PS10, Line 176: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@177 PS10, Line 177: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@210 PS10, Line 210: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@211 PS10, Line 211: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@212 PS10, Line 212: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@213 PS10, Line 213: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@231 PS10, Line 231: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@232 PS10, Line 232: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@246 PS10, Line 246: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@247 PS10, Line 247: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@261 PS10, Line 261: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@262 PS10, Line 262: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/21065/10/tests/metadata/test_event_processing_error.py@263 PS10, Line 263
[Impala-ASF-CR] [WIP]IMPALA-12832: Implicit invalidate metadata on event failures
Sai Hemanth Gantasala has uploaded a new patch set (#10) to the change originally created by k.venureddy2...@gmail.com. ( http://gerrit.cloudera.org:8080/21065 ) Change subject: [WIP]IMPALA-12832: Implicit invalidate metadata on event failures .. [WIP]IMPALA-12832: Implicit invalidate metadata on event failures At present, failure in event processing needs manual invalidate metadata. This patch implicitly invalidates the table upon failures in processing of table events with new 'process_event_failure' flag. And a new 'auto_global_invalidate_metadata' flag is added to global invalidate metadata automatically when event processor goes to non-active state. Note: Also introduced a config 'inject_process_event_failure_event_types' for automated tests to simulate event processor failures. This config is used to specify what event types can be intentionally failed. This config should only be used for testing purpose. Testing: - Added end-to-end tests to mimic failures in event processor and verified that event processor is active - Added unit test to verify the 'auto_global_invalidate_metadata' config - Passed FE tests Co-Authored-by: Sai Hemanth Gantasala Change-Id: Ia67fc04c995802d3b6b56f79564bf0954b012c6c --- M be/src/catalog/catalog-server.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java 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/BackendConfig.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M tests/common/impala_test_suite.py A tests/metadata/__init__.py M tests/metadata/test_event_processing.py A tests/metadata/test_event_processing_base.py A tests/metadata/test_event_processing_error.py M tests/util/event_processor_utils.py 16 files changed, 934 insertions(+), 305 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/21065/10 -- To view, visit http://gerrit.cloudera.org:8080/21065 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia67fc04c995802d3b6b56f79564bf0954b012c6c Gerrit-Change-Number: 21065 Gerrit-PatchSet: 10 Gerrit-Owner: Anonymous Coward 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-12543: Detect self-events before finishing DDL
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21029 ) Change subject: IMPALA-12543: Detect self-events before finishing DDL .. Patch Set 7: (8 comments) Thanks for fixing this! It's a great work that requires detailed reviews. Left some comments first. I'll look deeper into the patch. http://gerrit.cloudera.org:8080/#/c/21029/7/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/21029/7/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@909 PS7, Line 909: LOG.info("Received " + response.getEvents().size() + " events. First event id : " : + (response.getEvents().size() > 0 ? response.getEvents().get(0).getEventId() : :"none") : + "."); nit: It'd be more readable to use the message template: LOG.info("Received {} events. First event id: {}.", response.getEvents().size(), (response.getEvents().size() > 0 ? response.getEvents().get(0).getEventId() : "none")); http://gerrit.cloudera.org:8080/#/c/21029/5/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/21029/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1683 PS5, Line 1683: > I don't think that in-flight events are needed here. As for a view reloadin +1. Could you add a comment for this? http://gerrit.cloudera.org:8080/#/c/21029/7/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/21029/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1066 PS7, Line 1066: * Register the new version number into table's in-flight events. Could you add a comment for when should we use this, e.g. before invoking the HMS API for non-insert events? http://gerrit.cloudera.org:8080/#/c/21029/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@ PS7, Line : Preconditions.checkState(!table_.hasInProgressModification()); : Preconditions.checkState(!inflightEventRegistered_); nit: please add error messages for these http://gerrit.cloudera.org:8080/#/c/21029/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1366 PS7, Line 1366: Preconditions.checkState(reloadMetadata); nit: let's add an error message for Preconditions. http://gerrit.cloudera.org:8080/#/c/21029/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4493 PS7, Line 4493: modification.registerInflightEvent(); Might be an existing issue, but should we still do this when 'addedHmsPartitions' is empty, i.e. no partitions are added? http://gerrit.cloudera.org:8080/#/c/21029/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@5819 PS7, Line 5819: private void alterViewSetTblProperties(Table tbl, Just realized this is invoked in alterTable() instead of alterView().. In alterView(), we don't deal with the inflight versions (at L1754). Maybe we should make them consistent. But not a big deal since reload on views should be fast, unless it's a materialized view (IIRC, we can't alter MV yet). http://gerrit.cloudera.org:8080/#/c/21029/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6135 PS7, Line 6135: modification.registerInflightEvent(); Should we do this before L6106 where the events will be generated, and handle the failure at L6128? -- To view, visit http://gerrit.cloudera.org:8080/21029 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79 Gerrit-Change-Number: 21029 Gerrit-PatchSet: 7 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Tue, 05 Mar 2024 02:05:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12426: Query History Table
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20770 ) Change subject: IMPALA-12426: Query History Table .. Patch Set 31: (1 comment) http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc File be/src/service/workload-management.cc: http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc@589 PS31, Line 589: if(LIKELY(!ctx.record->per_host_state.empty())) { nit: space after 'if' -- To view, visit http://gerrit.cloudera.org:8080/20770 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844 Gerrit-Change-Number: 20770 Gerrit-PatchSet: 31 Gerrit-Owner: Jason Fehr Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 05 Mar 2024 01:12:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12426: Query History Table
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20770 ) Change subject: IMPALA-12426: Query History Table .. Patch Set 31: (1 comment) http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc File be/src/service/workload-management.cc: http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc@575 PS31, Line 575: auto min_elem = max_element(ctx.record->per_host_state.cbegin(), Should be max_elem. -- To view, visit http://gerrit.cloudera.org:8080/20770 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844 Gerrit-Change-Number: 20770 Gerrit-PatchSet: 31 Gerrit-Owner: Jason Fehr Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 05 Mar 2024 01:11:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP]IMPALA-12832: Implicit invalidate metadata on event failures
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21065 ) Change subject: [WIP]IMPALA-12832: Implicit invalidate metadata on event failures .. Patch Set 9: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10351/ -- To view, visit http://gerrit.cloudera.org:8080/21065 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia67fc04c995802d3b6b56f79564bf0954b012c6c Gerrit-Change-Number: 21065 Gerrit-PatchSet: 9 Gerrit-Owner: Anonymous Coward 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, 05 Mar 2024 00:08:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12426: Query History Table
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20770 ) Change subject: IMPALA-12426: Query History Table .. Patch Set 31: (16 comments) http://gerrit.cloudera.org:8080/#/c/20770/13/be/src/service/internal-server.cc File be/src/service/internal-server.cc: http://gerrit.cloudera.org:8080/#/c/20770/13/be/src/service/internal-server.cc@117 PS13, Line 117: !UUIDEmpty(internal_query_id)) { > Use the new function in uid-util.h Done http://gerrit.cloudera.org:8080/#/c/20770/12/be/src/service/workload-management.cc File be/src/service/workload-management.cc: http://gerrit.cloudera.org:8080/#/c/20770/12/be/src/service/workload-management.cc@646 PS12, Line 646: fields_.push_back(MakeTuple("exec_summary", _exec_summary)); : fields_.push_back(MakeTuple("plan", _plan)); : fields_.push_back(MakeTuple("num_rows_fetched", _num_rows_fetched, "BIGINT")); : fields_.push_back(MakeTuple( : "row_materialization_bytes_per_sec", _row_materialization_rate, "BIGINT")); : fields_.push_back(MakeTuple( : "row_materializ > I'm going to rearrange these into a different order that puts the string ch Done http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc File be/src/service/workload-management.cc: http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc@171 PS31, Line 171: SHUTDOWN_REQUESTED nit: SHUTTING_DOWN http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc@769 PS31, Line 769: completed_queries_thread_state_ == RUNNING Can ShutdownWorkloadManagement called before completed_queries_thread_state_ becomes RUNNING? http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc@841 PS31, Line 841: completed_queries_thread_state_ What if the previous completed_queries_thread_state_ is SHUTDOWN_REQUESTED or SHUTDOWN here? http://gerrit.cloudera.org:8080/#/c/20770/13/be/src/util/network-util.h File be/src/util/network-util.h: http://gerrit.cloudera.org:8080/#/c/20770/13/be/src/util/network-util.h@114 PS13, Line 114: turn lhs.hostname() == rhs.hostname() && lhs.port() == rhs.port() > Should uds_address be the last tie breaker? Done http://gerrit.cloudera.org:8080/#/c/20770/31/tests/beeswax/impala_beeswax.py File tests/beeswax/impala_beeswax.py: http://gerrit.cloudera.org:8080/#/c/20770/31/tests/beeswax/impala_beeswax.py@185 PS31, Line 185: fetch_profile_after_close=False fetch_profile_after_close=True seems specific to TestQueryLogTable tests only. Can this variable turned into a Client field member and set during Client initialization? Tests that require this behavior can then specify it during create_connection(). http://gerrit.cloudera.org:8080/#/c/20770/31/tests/beeswax/impala_beeswax.py@198 PS31, Line 198: runtime_profile = self.get_runtime_profile(handle) Skip pulling profile if !fetch_profile_after_close http://gerrit.cloudera.org:8080/#/c/20770/31/tests/beeswax/impala_beeswax.py@219 PS31, Line 219: result.runtime_profile = self.get_runtime_profile(handle) Skip pulling profile if !fetch_profile_after_close http://gerrit.cloudera.org:8080/#/c/20770/31/tests/common/custom_cluster_test_suite.py File tests/common/custom_cluster_test_suite.py: http://gerrit.cloudera.org:8080/#/c/20770/31/tests/common/custom_cluster_test_suite.py@244 PS31, Line 244: method=None this is unused? http://gerrit.cloudera.org:8080/#/c/20770/9/tests/custom_cluster/test_query_log.py File tests/custom_cluster/test_query_log.py: http://gerrit.cloudera.org:8080/#/c/20770/9/tests/custom_cluster/test_query_log.py@359 PS9, Line 359: se: : assert exec_group is not None : assert data[index] == exec_group.group(1), "executor > I didn't know it was possible to get the profile from the query handle. I Done http://gerrit.cloudera.org:8080/#/c/20770/12/tests/custom_cluster/test_query_log.py File tests/custom_cluster/test_query_log.py: http://gerrit.cloudera.org:8080/#/c/20770/12/tests/custom_cluster/test_query_log.py@633 PS12, Line 633: catalogd_args="--enable_workload_mgmt", : impalad_graceful_shutdown=True) : @pytest.mark.parametrize("client_protocol", PROTOCOL_ALL) : def test_query_log_table_ddl(self, client_protocol): : """Asserts the values written to the query log table match the values from the :query > Maybe it is due to stale metadata. Is it worth to run "invalidate metadata Moved discussion to patch set 31. http://gerrit.cloudera.org:8080/#/c/20770/31/tests/custom_cluster/test_query_log.py File tests/custom_cluster/test_query_log.py: http://gerrit.cloudera.org:8080/#/c/20770/31/tests/custom_cluster/test_query_log.py
[Impala-ASF-CR] IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21099 ) Change subject: IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21099 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4869eb863ff0afe8f691ccf2fd681a92d36b405c Gerrit-Change-Number: 21099 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 04 Mar 2024 23:37:20 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-12863: Fix toolchain container builds for CentOS 7, SLES 12 & 15
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21091 ) Change subject: IMPALA-12863: Fix toolchain container builds for CentOS 7, SLES 12 & 15 .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/21091/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21091/3//COMMIT_MSG@18 PS3, Line 18: but the Kudu : build requires the 'hostname' utility from it. Can you add "hostname" to the list of required executables in docker/all/assert-dependencies-present.py? -- To view, visit http://gerrit.cloudera.org:8080/21091 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e730abb6667bf00735ea62c35377591b68452ce Gerrit-Change-Number: 21091 Gerrit-PatchSet: 3 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 04 Mar 2024 23:18:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12426: Query History Table
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20770 ) Change subject: IMPALA-12426: Query History Table .. Patch Set 31: (1 comment) http://gerrit.cloudera.org:8080/#/c/20770/22/be/src/service/impala-http-handler.h File be/src/service/impala-http-handler.h: http://gerrit.cloudera.org:8080/#/c/20770/22/be/src/service/impala-http-handler.h@210 PS22, Line 210: void QueryStateToJson(const QueryStateRecord& record, > Shouldn't this be updated in the parent patch? Same with impala-http-handle Done -- To view, visit http://gerrit.cloudera.org:8080/20770 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844 Gerrit-Change-Number: 20770 Gerrit-PatchSet: 31 Gerrit-Owner: Jason Fehr Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 22:42:34 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12426: Query History Table
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20770 ) Change subject: IMPALA-12426: Query History Table .. Patch Set 31: (8 comments) http://gerrit.cloudera.org:8080/#/c/20770/22/be/src/service/impala-http-handler.h File be/src/service/impala-http-handler.h: http://gerrit.cloudera.org:8080/#/c/20770/22/be/src/service/impala-http-handler.h@210 PS22, Line 210: void QueryStateToJson(const QueryStateRecord& record, Shouldn't this be updated in the parent patch? Same with impala-http-handler.cc changes. Please do a test build with each patch to make sure it still compiles on its own, rather than requiring the full change stack. http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc File be/src/service/workload-management.cc: http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc@218 PS31, Line 218: for (auto iter = sql.cbegin(); iter != sql.cend(); iter++) { Could be a range-based for loop taking 'const auto&' (always a good default). http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc@393 PS31, Line 393: for (auto iter = ctx.record->per_host_state.cbegin(); Could be a range-based for loop taking 'const auto&'. http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc@659 PS31, Line 659: fields_.push_back(MakeTuple( Lines 659-677 should be indented. http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc@687 PS31, Line 687: for (auto iter = fields_.cbegin(); iter != fields_.cend(); iter++) { Could be a range-based for loop (const auto&). http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc@726 PS31, Line 726: for (auto iter = fields_.cbegin(); iter != fields_.cend(); iter++) { Range-based for loop. http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc@895 PS31, Line 895: for (auto iter = queries_to_insert.begin(); iter != queries_to_insert.end(); Could be a range-based for loop taking a ref (to update the count). http://gerrit.cloudera.org:8080/#/c/20770/31/be/src/service/workload-management.cc@953 PS31, Line 953: for (auto iter = fields_.cbegin(); iter != fields_.cend(); iter++) { Range-based for loop. -- To view, visit http://gerrit.cloudera.org:8080/20770 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844 Gerrit-Change-Number: 20770 Gerrit-PatchSet: 31 Gerrit-Owner: Jason Fehr Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 22:41:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 9: (1 comment) Thanks all for the review! http://gerrit.cloudera.org:8080/#/c/21100/9/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/21100/9/be/src/service/impala-server.cc@1167 PS9, Line 1167: ImpaladMetrics::QUERY_LOG_EST_TOTAL_BYTES->Increment(record_size); > It would make sense to me for this to be tied to the server rather than a g Right, it looks weird to maintain a state in ImpaladMetrics, but I rather have single place to track this. -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 22:15:00 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 10: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10352/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 10 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 22:15:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 10: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 10 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 22:15:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 9: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/21100/9/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/21100/9/be/src/service/impala-server.cc@1167 PS9, Line 1167: ImpaladMetrics::QUERY_LOG_EST_TOTAL_BYTES->Increment(record_size); It would make sense to me for this to be tied to the server rather than a global. However that doesn't seem to be how everything else is handled in ImpaladMetrics, so :shrug: -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 22:09:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15396/ : 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/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 21:42:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15395/ : 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/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 8 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 21:41:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 9: Code-Review+1 looks great! -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 21:23:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 9: (2 comments) http://gerrit.cloudera.org:8080/#/c/21100/7/be/src/util/impalad-metrics.h File be/src/util/impalad-metrics.h: http://gerrit.cloudera.org:8080/#/c/21100/7/be/src/util/impalad-metrics.h@267 PS7, Line 267: static const char* QUERY_LOG_EST_TOTAL_BYTES; > Nit: rename to match new metric name QUERY_LOG_EST_TOTAL_BYTES Done http://gerrit.cloudera.org:8080/#/c/21100/7/be/src/util/impalad-metrics.h@351 PS7, Line 351: static IntGauge* QUERY_LOG_EST_TOTAL_BYTES; > Nit: rename to match new metric name QUERY_LOG_EST_TOTAL_BYTES Done -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 21:17:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Hello Daniel Becker, Jason Fehr, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21100 to look at the new patch set (#9). Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. IMPALA-12864: Deflake test_query_log_size_in_bytes. test_query_log_size_in_bytes is flaky in exhaustive builds. The expected QueryStateRecord is off by around 100KB per query, indicating that the test query might be too complex and lead to variability in final query profile that is being archived. This patch simplifies the test query and relaxes the assertion. This patch also adds QUERY_LOG_EST_TOTAL_BYTES metric (key name "impala-server.query-log-est-total-bytes") that is validated as well in test_query_log_size_in_bytes. query-state-record-test.cc is modified a bit to resolve segmentation fault issue when building unified-be-test-validated-executable target run_clang_tidy.sh. Testing: - Pass test_query_log_size_in_bytes in exhaustive build. Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 --- M be/src/service/CMakeLists.txt M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-state-record-test.cc M be/src/service/query-state-record.h M be/src/util/impalad-metrics.cc M be/src/util/impalad-metrics.h M bin/run_clang_tidy.sh M common/thrift/metrics.json M tests/custom_cluster/test_web_pages.py 10 files changed, 55 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/21100/9 -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/21100/8/be/src/util/impalad-metrics.cc File be/src/util/impalad-metrics.cc: http://gerrit.cloudera.org:8080/#/c/21100/8/be/src/util/impalad-metrics.cc@348 PS8, Line 348: QUERY_LOG_EST_TOTAL_BYTES = m->AddGauge(ImpaladMetricKeys::QUERY_LOG_EST_TOTAL_BYTES, 0); line too long (91 > 90) -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 8 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 21:15:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Hello Daniel Becker, Jason Fehr, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21100 to look at the new patch set (#8). Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. IMPALA-12864: Deflake test_query_log_size_in_bytes. test_query_log_size_in_bytes is flaky in exhaustive builds. The expected QueryStateRecord is off by around 100KB per query, indicating that the test query might be too complex and lead to variability in final query profile that is being archived. This patch simplifies the test query and relaxes the assertion. This patch also adds QUERY_LOG_EST_TOTAL_BYTES metric (key name "impala-server.query-log-est-total-bytes") that is validated as well in test_query_log_size_in_bytes. query-state-record-test.cc is modified a bit to resolve segmentation fault issue when building unified-be-test-validated-executable target run_clang_tidy.sh. Testing: - Pass test_query_log_size_in_bytes in exhaustive build. Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 --- M be/src/service/CMakeLists.txt M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-state-record-test.cc M be/src/service/query-state-record.h M be/src/util/impalad-metrics.cc M be/src/util/impalad-metrics.h M bin/run_clang_tidy.sh M common/thrift/metrics.json M tests/custom_cluster/test_web_pages.py 10 files changed, 54 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/21100/8 -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 8 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/21100/7/be/src/util/impalad-metrics.h File be/src/util/impalad-metrics.h: http://gerrit.cloudera.org:8080/#/c/21100/7/be/src/util/impalad-metrics.h@267 PS7, Line 267: static const char* QUERY_LOG_EST_TOTAL_SIZE; Nit: rename to match new metric name QUERY_LOG_EST_TOTAL_BYTES http://gerrit.cloudera.org:8080/#/c/21100/7/be/src/util/impalad-metrics.h@351 PS7, Line 351: static IntGauge* QUERY_LOG_EST_TOTAL_SIZE; Nit: rename to match new metric name QUERY_LOG_EST_TOTAL_BYTES -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 7 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 21:11:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15394/ : 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/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 7 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 21:04:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12426: Query History Table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20770 ) Change subject: IMPALA-12426: Query History Table .. Patch Set 30: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15393/ : 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/20770 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844 Gerrit-Change-Number: 20770 Gerrit-PatchSet: 30 Gerrit-Owner: Jason Fehr Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 20:54:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12426: Query History Table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20770 ) Change subject: IMPALA-12426: Query History Table .. Patch Set 29: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15392/ : 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/20770 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844 Gerrit-Change-Number: 20770 Gerrit-PatchSet: 29 Gerrit-Owner: Jason Fehr Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 20:50:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 7: (2 comments) ps7 also tweak run_clang_tidy.sh to grep make Error. Dumped build output is so big (almost 1GB of text file). It is helpful to summarize make Error in the beginning of tidylog.txt. http://gerrit.cloudera.org:8080/#/c/21100/5/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/21100/5/be/src/service/impala-server.h@1137 PS5, Line 1137: std::mutex query_log_lock_; > Thinking about this more, it is good that the QUERY_LOG_EST_TOTAL_SIZE met Added comment at impalad-metrics.h http://gerrit.cloudera.org:8080/#/c/21100/5/common/thrift/metrics.json File common/thrift/metrics.json: http://gerrit.cloudera.org:8080/#/c/21100/5/common/thrift/metrics.json@2864 PS5, Line 2864: "key": "impala-server.query-log-est-total-bytes" > Nit: The suffix "size" on the Impala metrics typically refers to a count of Done -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 7 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 20:43:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Hello Daniel Becker, Jason Fehr, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21100 to look at the new patch set (#7). Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. IMPALA-12864: Deflake test_query_log_size_in_bytes. test_query_log_size_in_bytes is flaky in exhaustive builds. The expected QueryStateRecord is off by around 100KB per query, indicating that the test query might be too complex and lead to variability in final query profile that is being archived. This patch simplifies the test query and relaxes the assertion. This patch also adds QUERY_LOG_EST_TOTAL_SIZE metric (key name "impala-server.query-log-est-total-size") that is validated as well in test_query_log_size_in_bytes. query-state-record-test.cc is modified a bit to resolve segmentation fault issue when building unified-be-test-validated-executable target run_clang_tidy.sh. Testing: - Pass test_query_log_size_in_bytes in exhaustive build. Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 --- M be/src/service/CMakeLists.txt M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-state-record-test.cc M be/src/service/query-state-record.h M be/src/util/impalad-metrics.cc M be/src/util/impalad-metrics.h M bin/run_clang_tidy.sh M common/thrift/metrics.json M tests/custom_cluster/test_web_pages.py 10 files changed, 54 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/21100/7 -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 7 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-12426: Query History Table
Hello Andrew Sherman, Riza Suminto, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20770 to look at the new patch set (#30). Change subject: IMPALA-12426: Query History Table .. IMPALA-12426: Query History Table Adds the ability for users to specify that Impala will create and maintain an internal Iceberg table that contains data about all completed queries. This table is automatically created at startup by each coordinator if it does not exist. Then, most completed queries are queued in memory and flushed to the query history table at a set interval (either minutes or number of records). Set, use, and show queries are not written to this table. This commit leverages the InternalServer class to maintain the query history table. Ctest unit tests have been added to assert the various pieces of code. New custom cluster tests have been added to assert the query history table is properly populated with completed queries. Negative testing consists of attempting sql injection attacks and syntactically incorrect queries. Impala built-in string functions benchmarks have been updated to include the new built-in functions. Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844 --- M be/src/runtime/query-driver.cc M be/src/runtime/query-driver.h M be/src/service/CMakeLists.txt M be/src/service/impala-http-handler.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/internal-server-test.cc M be/src/service/internal-server.cc M be/src/service/internal-server.h A be/src/service/workload-management.cc M be/src/util/backend-gflag-util.cc M be/src/util/impalad-metrics.cc M be/src/util/impalad-metrics.h M common/thrift/BackendGflags.thrift M common/thrift/metrics.json M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/util/CatalogBlacklistUtils.java M tests/beeswax/impala_beeswax.py M tests/common/custom_cluster_test_suite.py M tests/common/impala_connection.py A tests/custom_cluster/test_query_log.py A tests/util/assert_time.py A tests/util/memory.py A tests/util/retry.py 25 files changed, 3,010 insertions(+), 67 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/20770/30 -- To view, visit http://gerrit.cloudera.org:8080/20770 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844 Gerrit-Change-Number: 20770 Gerrit-PatchSet: 30 Gerrit-Owner: Jason Fehr Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15391/ : 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/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 6 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 20:31:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/21100/5/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/21100/5/be/src/service/impala-server.h@1137 PS5, Line 1137: std::mutex query_log_lock_; > Agree. QUERY_LOG_EST_TOTAL_SIZE itself is already Atomic. Drop it from comm Thinking about this more, it is good that the QUERY_LOG_EST_TOTAL_SIZE metric is modified while this lock is held since that ensures this metric always matches the data stored in the query_log_est_sizes_ list. Please consider mentioning that modifications of this metric are done while this lock is held. http://gerrit.cloudera.org:8080/#/c/21100/5/common/thrift/metrics.json File common/thrift/metrics.json: http://gerrit.cloudera.org:8080/#/c/21100/5/common/thrift/metrics.json@2864 PS5, Line 2864: "key": "impala-server.query-log-est-total-size" Nit: The suffix "size" on the Impala metrics typically refers to a count of some sort and not to a specific number of bytes. I suggest naming this metric "impala-server.query-log-est-total-bytes". -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 6 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 20:27:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12426: Query History Table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20770 ) Change subject: IMPALA-12426: Query History Table .. Patch Set 29: (6 comments) http://gerrit.cloudera.org:8080/#/c/20770/29/tests/common/impala_connection.py File tests/common/impala_connection.py: http://gerrit.cloudera.org:8080/#/c/20770/29/tests/common/impala_connection.py@31 PS29, Line 31: from ImpalaService import ImpalaHiveServer2Service flake8: F401 'ImpalaService.ImpalaHiveServer2Service' imported but unused http://gerrit.cloudera.org:8080/#/c/20770/29/tests/custom_cluster/test_query_log.py File tests/custom_cluster/test_query_log.py: http://gerrit.cloudera.org:8080/#/c/20770/29/tests/custom_cluster/test_query_log.py@490 PS29, Line 490: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/20770/29/tests/custom_cluster/test_query_log.py@491 PS29, Line 491: \ flake8: W605 invalid escape sequence '\)' http://gerrit.cloudera.org:8080/#/c/20770/29/tests/custom_cluster/test_query_log.py@491 PS29, Line 491: \ flake8: W605 invalid escape sequence '\s' http://gerrit.cloudera.org:8080/#/c/20770/29/tests/custom_cluster/test_query_log.py@491 PS29, Line 491: \ flake8: W605 invalid escape sequence '\S' http://gerrit.cloudera.org:8080/#/c/20770/29/tests/util/memory.py File tests/util/memory.py: http://gerrit.cloudera.org:8080/#/c/20770/29/tests/util/memory.py@30 PS29, Line 30: def convert_to_bytes(mem_str, unit_combined=False, factor=1024): flake8: E302 expected 2 blank lines, found 1 -- To view, visit http://gerrit.cloudera.org:8080/20770 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844 Gerrit-Change-Number: 20770 Gerrit-PatchSet: 29 Gerrit-Owner: Jason Fehr Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 20:25:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/21099 ) Change subject: IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/21099/2/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test: http://gerrit.cloudera.org:8080/#/c/21099/2/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test@421 PS2, Line 421: BUFFERED DELETE FROM ICEBERG [functional_parquet.iceberg_v2_partitioned_position_deletes-POSITION-DELETE] I'm wondering if getting rid of DELETE FROM ICEBERG in this patch is how tightly related to the issue we are about to solve with this patch. For me it seems kind of a separate stuff at first sight. If you say that this holds low or non-existing risk then I'm fine keeping the patch as is now, I just want to make sure that we won't introduce any further risks with fixing one issue. e.g. perf degradation with switching from DeleteSink to BufferedDeleteSink http://gerrit.cloudera.org:8080/#/c/21099/2/tests/stress/test_update_stress.py File tests/stress/test_update_stress.py: http://gerrit.cloudera.org:8080/#/c/21099/2/tests/stress/test_update_stress.py@279 PS2, Line 279: # Prepare INSERT statement of 'num_rows' records. nit: I meant to add it to a comment that this code prefers the INSERT in such a way that all the rows will be added in a single insert to have a single file (opposed to the similar part of the test above that adds a separate file for each row). -- To view, visit http://gerrit.cloudera.org:8080/21099 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4869eb863ff0afe8f691ccf2fd681a92d36b405c Gerrit-Change-Number: 21099 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 04 Mar 2024 20:24:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12426: Query History Table
Hello Andrew Sherman, Riza Suminto, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20770 to look at the new patch set (#29). Change subject: IMPALA-12426: Query History Table .. IMPALA-12426: Query History Table Adds the ability for users to specify that Impala will create and maintain an internal Iceberg table that contains data about all completed queries. This table is automatically created at startup by each coordinator if it does not exist. Then, most completed queries are queued in memory and flushed to the query history table at a set interval (either minutes or number of records). Set, use, and show queries are not written to this table. This commit leverages the InternalServer class to maintain the query history table. Ctest unit tests have been added to assert the various pieces of code. New custom cluster tests have been added to assert the query history table is properly populated with completed queries. Negative testing consists of attempting sql injection attacks and syntactically incorrect queries. Impala built-in string functions benchmarks have been updated to include the new built-in functions. Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844 --- M be/src/runtime/query-driver.cc M be/src/runtime/query-driver.h M be/src/service/CMakeLists.txt M be/src/service/impala-http-handler.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/internal-server-test.cc M be/src/service/internal-server.cc M be/src/service/internal-server.h A be/src/service/workload-management.cc M be/src/util/backend-gflag-util.cc M be/src/util/impalad-metrics.cc M be/src/util/impalad-metrics.h M common/thrift/BackendGflags.thrift M common/thrift/metrics.json M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/util/CatalogBlacklistUtils.java M tests/beeswax/impala_beeswax.py M tests/common/custom_cluster_test_suite.py M tests/common/impala_connection.py A tests/custom_cluster/test_query_log.py A tests/util/assert_time.py A tests/util/memory.py A tests/util/retry.py 25 files changed, 3,010 insertions(+), 67 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/20770/29 -- To view, visit http://gerrit.cloudera.org:8080/20770 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844 Gerrit-Change-Number: 20770 Gerrit-PatchSet: 29 Gerrit-Owner: Jason Fehr Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/21100/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21100/5//COMMIT_MSG@14 PS5, Line 14: This patch simplifies the test query and relaxes the assertion. This > Nit: and relaxes the assertion Done http://gerrit.cloudera.org:8080/#/c/21100/5//COMMIT_MSG@15 PS5, Line 15: patch also adds QUERY_LOG_EST_TOTAL_SIZE metric (key name > Please mention the name of the new "impala-server.query-log-est-total-size" Done http://gerrit.cloudera.org:8080/#/c/21100/5/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/21100/5/be/src/service/impala-server.h@1137 PS5, Line 1137: std::mutex query_log_lock_; > The lock does not protect the QUERY_LOG_EST_TOTAL_SIZE when it's being read Agree. QUERY_LOG_EST_TOTAL_SIZE itself is already Atomic. Drop it from comment. http://gerrit.cloudera.org:8080/#/c/21100/5/tests/custom_cluster/test_web_pages.py File tests/custom_cluster/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/21100/5/tests/custom_cluster/test_web_pages.py@153 PS5, Line 153: @CustomClusterTestSuite.with_args( > Nit: Since the (40 * 1024) calculation is used in two places, it would elim Done http://gerrit.cloudera.org:8080/#/c/21100/5/tests/custom_cluster/test_web_pages.py@178 PS5, Line 178: # 'should_exist' > The above code can be replaced with: Done. I'd like to keep the assertion separate so it is clear which expression is failed. -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 6 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 20:07:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Hello Daniel Becker, Jason Fehr, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21100 to look at the new patch set (#6). Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. IMPALA-12864: Deflake test_query_log_size_in_bytes. test_query_log_size_in_bytes is flaky in exhaustive builds. The expected QueryStateRecord is off by around 100KB per query, indicating that the test query might be too complex and lead to variability in final query profile that is being archived. This patch simplifies the test query and relaxes the assertion. This patch also adds QUERY_LOG_EST_TOTAL_SIZE metric (key name "impala-server.query-log-est-total-size") that is validated as well in test_query_log_size_in_bytes. query-state-record-test.cc is modified a bit to resolve segmentation fault issue when building unified-be-test-validated-executable target run_clang_tidy.sh. Testing: - Pass test_query_log_size_in_bytes in exhaustive build. Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 --- M be/src/service/CMakeLists.txt M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-state-record-test.cc M be/src/service/query-state-record.h M be/src/util/impalad-metrics.cc M be/src/util/impalad-metrics.h M common/thrift/metrics.json M tests/custom_cluster/test_web_pages.py 9 files changed, 50 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/21100/6 -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 6 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/21100/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21100/5//COMMIT_MSG@14 PS5, Line 14: This patch simplifies the test query add relaxes the assertion. This Nit: and relaxes the assertion http://gerrit.cloudera.org:8080/#/c/21100/5//COMMIT_MSG@15 PS5, Line 15: patch also adds QUERY_LOG_EST_TOTAL_SIZE metric that is validated as Please mention the name of the new "impala-server.query-log-est-total-size" metric as well. http://gerrit.cloudera.org:8080/#/c/21100/5/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/21100/5/be/src/service/impala-server.h@1137 PS5, Line 1137: /// QUERY_LOG_EST_TOTAL_SIZE metric. The lock does not protect the QUERY_LOG_EST_TOTAL_SIZE when it's being read by the Impala http server. I don't think that's an issue since the metric is initialized once, and that initialization happens long before the Impala http server can access the metric. As far as I know, the only potential issue is the impala-server.query-log-est-total-size metric may be slightly mis-reported, but that is ok since the Impala http server always lags behind the query_log_ anyways. http://gerrit.cloudera.org:8080/#/c/21100/5/be/src/service/query-state-record.h File be/src/service/query-state-record.h: http://gerrit.cloudera.org:8080/#/c/21100/5/be/src/service/query-state-record.h@181 PS5, Line 181: int64_t peak_memory_usage = 0; Thanks for fixing! http://gerrit.cloudera.org:8080/#/c/21100/5/tests/custom_cluster/test_web_pages.py File tests/custom_cluster/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/21100/5/tests/custom_cluster/test_web_pages.py@153 PS5, Line 153: impalad_args="--query_log_size_in_bytes=" + str(40 * 1024) Nit: Since the (40 * 1024) calculation is used in two places, it would eliminate the repetition to declare a variable above this test function as use that variable in both places. http://gerrit.cloudera.org:8080/#/c/21100/5/tests/custom_cluster/test_web_pages.py@178 PS5, Line 178: assert log_metric["value"] <= 40 * 1024, log_metric The above code can be replaced with: impalad = self.cluster.get_first_impalad() assert 0 < impalad.service.get_metric_value("impala-server.query-log-est-total-size") < 40 * 1024 -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 5 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 19:43:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP]IMPALA-12832: Implicit invalidate metadata on event failures
k.venureddy2...@gmail.com has abandoned this change. ( http://gerrit.cloudera.org:8080/21106 ) Change subject: [WIP]IMPALA-12832: Implicit invalidate metadata on event failures .. Abandoned duplicate of another commit -- To view, visit http://gerrit.cloudera.org:8080/21106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: Ib11c554f5692e4fb8a4c63ef5ba8f8ae0e787826 Gerrit-Change-Number: 21106 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] [WIP]IMPALA-12832: Implicit invalidate metadata on event failures
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21065 ) Change subject: [WIP]IMPALA-12832: Implicit invalidate metadata on event failures .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10351/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21065 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia67fc04c995802d3b6b56f79564bf0954b012c6c Gerrit-Change-Number: 21065 Gerrit-PatchSet: 9 Gerrit-Owner: Anonymous Coward 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: Mon, 04 Mar 2024 19:42:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21099 ) Change subject: IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10350/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21099 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4869eb863ff0afe8f691ccf2fd681a92d36b405c Gerrit-Change-Number: 21099 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 04 Mar 2024 18:58:43 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP]IMPALA-12832: Implicit invalidate metadata on event failures
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21065 ) Change subject: [WIP]IMPALA-12832: Implicit invalidate metadata on event failures .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15390/ : 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/21065 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia67fc04c995802d3b6b56f79564bf0954b012c6c Gerrit-Change-Number: 21065 Gerrit-PatchSet: 9 Gerrit-Owner: Anonymous Coward 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: Mon, 04 Mar 2024 18:35:48 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-12863: Fix toolchain container builds for CentOS 7, SLES 12 & 15
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21091 ) Change subject: IMPALA-12863: Fix toolchain container builds for CentOS 7, SLES 12 & 15 .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21091 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e730abb6667bf00735ea62c35377591b68452ce Gerrit-Change-Number: 21091 Gerrit-PatchSet: 3 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 04 Mar 2024 18:25:56 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP]IMPALA-12832: Implicit invalidate metadata on event failures
Hello Quanlong Huang, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21065 to look at the new patch set (#9). Change subject: [WIP]IMPALA-12832: Implicit invalidate metadata on event failures .. [WIP]IMPALA-12832: Implicit invalidate metadata on event failures At present, failure in event processing needs manual invalidate metadata. This patch implicitly invalidates the table upon failures in processing of table events with new 'process_event_failure' flag. And a new 'auto_global_invalidate_metadata' flag is added to global invalidate metadata automatically when event processor goes to non-active state. Note: Also introduced a config 'inject_process_event_failure_event_types' for automated tests to simulate event processor failures. This config is used to specify what event types can be intentionally failed. This config should only be used for testing purpose. Testing: - Added end-to-end tests to mimic failures in event processor and verified that event processor is active - Passed FE tests Co-Authored-by: Sai Hemanth Gantasala Change-Id: Ia67fc04c995802d3b6b56f79564bf0954b012c6c --- M be/src/catalog/catalog-server.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java 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/BackendConfig.java M tests/common/impala_test_suite.py A tests/metadata/__init__.py M tests/metadata/test_event_processing.py A tests/metadata/test_event_processing_base.py A tests/metadata/test_event_processing_error.py M tests/util/event_processor_utils.py 15 files changed, 664 insertions(+), 304 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/21065/9 -- To view, visit http://gerrit.cloudera.org:8080/21065 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia67fc04c995802d3b6b56f79564bf0954b012c6c Gerrit-Change-Number: 21065 Gerrit-PatchSet: 9 Gerrit-Owner: Anonymous Coward 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] [WIP]IMPALA-12832: Implicit invalidate metadata on event failures
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21065 ) Change subject: [WIP]IMPALA-12832: Implicit invalidate metadata on event failures .. Patch Set 9: (4 comments) http://gerrit.cloudera.org:8080/#/c/21065/9/tests/metadata/__init__.py File tests/metadata/__init__.py: http://gerrit.cloudera.org:8080/#/c/21065/9/tests/metadata/__init__.py@1 PS9, Line 1: flake8: W292 no newline at end of file http://gerrit.cloudera.org:8080/#/c/21065/9/tests/metadata/test_event_processing_base.py File tests/metadata/test_event_processing_base.py: http://gerrit.cloudera.org:8080/#/c/21065/9/tests/metadata/test_event_processing_base.py@348 PS9, Line 348: flake8: W292 no newline at end of file http://gerrit.cloudera.org:8080/#/c/21065/9/tests/metadata/test_event_processing_error.py File tests/metadata/test_event_processing_error.py: http://gerrit.cloudera.org:8080/#/c/21065/9/tests/metadata/test_event_processing_error.py@30 PS9, Line 30: @SkipIfCatalogV2.hms_event_polling_disabled() flake8: E302 expected 2 blank lines, found 1 http://gerrit.cloudera.org:8080/#/c/21065/9/tests/metadata/test_event_processing_error.py@64 PS9, Line 64: flake8: W292 no newline at end of file -- To view, visit http://gerrit.cloudera.org:8080/21065 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia67fc04c995802d3b6b56f79564bf0954b012c6c Gerrit-Change-Number: 21065 Gerrit-PatchSet: 9 Gerrit-Owner: Anonymous Coward 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: Mon, 04 Mar 2024 18:09:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21099 ) Change subject: IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15389/ : 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/21099 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4869eb863ff0afe8f691ccf2fd681a92d36b405c Gerrit-Change-Number: 21099 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 04 Mar 2024 18:05:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10949: (Addendum) Improve batching logic of events
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21032 ) Change subject: IMPALA-10949: (Addendum) Improve batching logic of events .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/21032/1/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/21032/1/tests/custom_cluster/test_events_custom_configs.py@411 PS1, Line 411: self.run_stmt_in_hive( : "analyze table {}.{} compute statistics".format(unique_database, test_batch_table)) : self.client.execute("refresh {0}.{1}".format(unique_database, test_batch_table)) Is it possible that the event poll and processing complete faster than catalogd handling the REFRESH query? If it possible, then a better fix is to inject sleep in Event Processor to hold processing event batch after poll, so that REFRESH happen before event batch processing. This will require exposing debug_action flag to Java code via BackendConfig.java. -- To view, visit http://gerrit.cloudera.org:8080/21032 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0c6d0c3e3449f98a41058d0186208f17eb91cd0d Gerrit-Change-Number: 21032 Gerrit-PatchSet: 1 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 04 Mar 2024 17:44:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations
Hello Daniel Becker, Gabor Kaszab, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21099 to look at the new patch set (#2). Change subject: IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations .. IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations We must invoke validateDataFilesExist for RowDelta operations (DELETE/ UPDATE/MERGE). Without this a concurrent RewriteFiles (compaction) and RowDelta can corrupt a table. IcebergBufferedDeleteSink now also collects the filenames of the data files that are referenced in the position delete files. It adds them to the DML exec state which is then collected by the Coordinator. The Coordinator passes the file paths to CatalogD which executes Iceberg's RowDelta operation and now invokes validateDataFilesExist() with the file paths. Additionally it also invokes validateDeletedFiles(). This patch set also resolves IMPALA-12640 which is about replacing IcebergDeleteSink with IcebergBufferedDeleteSink, as from now on we use the buffered version for all DML operations that write position delete files. Testing: * adds new stress test with DELETE + UPDATE + OPTIMIZE Change-Id: I4869eb863ff0afe8f691ccf2fd681a92d36b405c --- M be/src/exec/CMakeLists.txt M be/src/exec/iceberg-buffered-delete-sink.cc M be/src/exec/iceberg-buffered-delete-sink.h M be/src/exec/iceberg-delete-sink-config.cc D be/src/exec/iceberg-delete-sink.cc D be/src/exec/iceberg-delete-sink.h M be/src/exec/multi-table-sink.cc M be/src/runtime/dml-exec-state.cc M be/src/runtime/dml-exec-state.h M be/src/service/client-request-state.cc M common/protobuf/control_service.proto M common/thrift/CatalogService.thrift M common/thrift/DataSinks.thrift M fe/src/main/java/org/apache/impala/analysis/IcebergDeleteImpl.java M fe/src/main/java/org/apache/impala/planner/IcebergBufferedDeleteSink.java D fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java M fe/src/main/java/org/apache/impala/planner/TableSink.java M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test M tests/stress/test_update_stress.py 20 files changed, 163 insertions(+), 580 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/21099/2 -- To view, visit http://gerrit.cloudera.org:8080/21099 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4869eb863ff0afe8f691ccf2fd681a92d36b405c Gerrit-Change-Number: 21099 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15388/ : 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/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 5 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 16:20:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 5: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py File tests/custom_cluster/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py@180 PS4, Line 180: he query page only contai > This one is scraped from /queries page, while the one in L166 is scraped fr You're right, thanks for clarifying it. -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 5 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 16:10:19 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 5: (12 comments) http://gerrit.cloudera.org:8080/#/c/21100/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21100/4//COMMIT_MSG@9 PS4, Line 9: build > Nit: builds. Done http://gerrit.cloudera.org:8080/#/c/21100/4//COMMIT_MSG@14 PS4, Line 14: d rel > Nit: relaxes. Done http://gerrit.cloudera.org:8080/#/c/21100/4//COMMIT_MSG@14 PS4, Line 14: simplifi > Nit: simplifies. Done http://gerrit.cloudera.org:8080/#/c/21100/4//COMMIT_MSG@15 PS4, Line 15: al > Nit: adds. Done http://gerrit.cloudera.org:8080/#/c/21100/4/be/src/util/impalad-metrics.h File be/src/util/impalad-metrics.h: http://gerrit.cloudera.org:8080/#/c/21100/4/be/src/util/impalad-metrics.h@264 PS4, Line 264: that > Nit: that are currently. Done http://gerrit.cloudera.org:8080/#/c/21100/4/common/thrift/metrics.json File common/thrift/metrics.json: http://gerrit.cloudera.org:8080/#/c/21100/4/common/thrift/metrics.json@2857 PS4, Line 2857: that are curre > Nit: that are currently. Done http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py File tests/custom_cluster/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py@173 PS4, Line 173: log_metric = metric > Could add a break here. Or do we reach this multiple times and we need the Done. There is only 1 entry. http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py@174 PS4, Line 174: > Before writing the response text we could add some info about what's coming Done http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py@178 PS4, Line 178: ric > Nit: the test queries. Done http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py@178 PS4, Line 178: ric[" > Nit: the query page. Done http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py@178 PS4, Line 178: 4, log > Nit: a subset. Done http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py@180 PS4, Line 180: he query page only contai > The text is already parsed into JSON on L166, we could move this variable t This one is scraped from /queries page, while the one in L166 is scraped from /metrics page. Renamed the variable to clarify. -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 5 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 15:56:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Hello Daniel Becker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21100 to look at the new patch set (#5). Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. IMPALA-12864: Deflake test_query_log_size_in_bytes. test_query_log_size_in_bytes is flaky in exhaustive builds. The expected QueryStateRecord is off by around 100KB per query, indicating that the test query might be too complex and lead to variability in final query profile that is being archived. This patch simplifies the test query add relaxes the assertion. This patch also adds QUERY_LOG_EST_TOTAL_SIZE metric that is validated as well in test_query_log_size_in_bytes. query-state-record-test.cc is modified a bit to resolve segmentation fault issue when building unified-be-test-validated-executable target run_clang_tidy.sh. Testing: - Pass test_query_log_size_in_bytes in exhaustive build. Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 --- M be/src/service/CMakeLists.txt M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-state-record-test.cc M be/src/service/query-state-record.h M be/src/util/impalad-metrics.cc M be/src/util/impalad-metrics.h M common/thrift/metrics.json M tests/custom_cluster/test_web_pages.py 9 files changed, 60 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/21100/5 -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 5 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-12864: Deflake test query log size in bytes.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21100 ) Change subject: IMPALA-12864: Deflake test_query_log_size_in_bytes. .. Patch Set 4: (12 comments) Thanks Riza! http://gerrit.cloudera.org:8080/#/c/21100/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21100/4//COMMIT_MSG@9 PS4, Line 9: build Nit: builds. http://gerrit.cloudera.org:8080/#/c/21100/4//COMMIT_MSG@14 PS4, Line 14: relax Nit: relaxes. http://gerrit.cloudera.org:8080/#/c/21100/4//COMMIT_MSG@14 PS4, Line 14: simplify Nit: simplifies. http://gerrit.cloudera.org:8080/#/c/21100/4//COMMIT_MSG@15 PS4, Line 15: add Nit: adds. http://gerrit.cloudera.org:8080/#/c/21100/4/be/src/util/impalad-metrics.h File be/src/util/impalad-metrics.h: http://gerrit.cloudera.org:8080/#/c/21100/4/be/src/util/impalad-metrics.h@264 PS4, Line 264: that Nit: that are currently. http://gerrit.cloudera.org:8080/#/c/21100/4/common/thrift/metrics.json File common/thrift/metrics.json: http://gerrit.cloudera.org:8080/#/c/21100/4/common/thrift/metrics.json@2857 PS4, Line 2857: that currently Nit: that are currently. http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py File tests/custom_cluster/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py@173 PS4, Line 173: log_metric = metric Could add a break here. Or do we reach this multiple times and we need the last one? http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py@174 PS4, Line 174: response.text Before writing the response text we could add some info about what's coming, for example "Metrics: " or "Metrics received from WebUI: ". http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py@178 PS4, Line 178: test Nit: the test queries. http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py@178 PS4, Line 178: query Nit: the query page. http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py@178 PS4, Line 178: subset Nit: a subset. http://gerrit.cloudera.org:8080/#/c/21100/4/tests/custom_cluster/test_web_pages.py@180 PS4, Line 180: json.loads(response.text) The text is already parsed into JSON on L166, we could move this variable there so we don't have to parse the text twice. -- To view, visit http://gerrit.cloudera.org:8080/21100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9b543ffa931a01c11d58e45774ea46af78b3a25 Gerrit-Change-Number: 21100 Gerrit-PatchSet: 4 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Mar 2024 15:02:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21099 ) Change subject: IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations .. Patch Set 1: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10349/ -- To view, visit http://gerrit.cloudera.org:8080/21099 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4869eb863ff0afe8f691ccf2fd681a92d36b405c Gerrit-Change-Number: 21099 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 04 Mar 2024 14:26:57 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP]IMPALA-12832: Implicit invalidate metadata on event failures
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21106 ) Change subject: [WIP]IMPALA-12832: Implicit invalidate metadata on event failures .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15387/ : 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/21106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib11c554f5692e4fb8a4c63ef5ba8f8ae0e787826 Gerrit-Change-Number: 21106 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 04 Mar 2024 13:48:04 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP]IMPALA-12832: Implicit invalidate metadata on event failures
k.venureddy2...@gmail.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/21065 ) Change subject: [WIP]IMPALA-12832: Implicit invalidate metadata on event failures .. Patch Set 8: (13 comments) http://gerrit.cloudera.org:8080/#/c/21065/8/be/src/catalog/catalog-server.cc File be/src/catalog/catalog-server.cc: http://gerrit.cloudera.org:8080/#/c/21065/8/be/src/catalog/catalog-server.cc@174 PS8, Line 174: DEFINE_string > nit: define this as hidden using DEFINE_string_hidden() Done http://gerrit.cloudera.org:8080/#/c/21065/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/21065/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2753 PS3, Line 2753: etTable( > I am not sure if it is really needed to deal with rename. Generally events Agreed. Not invalidating tables for create/drop/alter table rename events. http://gerrit.cloudera.org:8080/#/c/21065/7/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/21065/7/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1292 PS7, Line 1292: e); : catalog_.invalidateTableIfExists(dbName_, tblName_); : } > nit: we can use warnLog() Done http://gerrit.cloudera.org:8080/#/c/21065/8/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/21065/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@a2811 PS8, Line 2811: > Could you explain why we remove this? Have removed it from this gerrit. It is fixed with https://issues.apache.org/jira/browse/IMPALA-12851 now. http://gerrit.cloudera.org:8080/#/c/21065/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@677 PS8, Line 677: BackendConfig.INSTANCE.getProcessEventFailureEventTypes() > nit: this can also be replaced with catalog_.getFailureEventsForTesting() We don't need this check anymore. Have directly call contains() instead. http://gerrit.cloudera.org:8080/#/c/21065/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@679 PS8, Line 679: Double > nit: use primitive type 'double' Done http://gerrit.cloudera.org:8080/#/c/21065/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1368 PS8, Line 1368: return false; > Maybe we can use CatalogServiceCatalog.invalidateDb() here. But it seems lo Have added a todo for now http://gerrit.cloudera.org:8080/#/c/21065/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1787 PS8, Line 1787: tableBefore_.getDbName(), tableBefore_.getTableName()); > Please add a comment saying the new table will be invalidated in MetastoreT Not doing invalidateTableIfExists in case of rename table. Have added a comment for it now. http://gerrit.cloudera.org:8080/#/c/21065/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2902 PS8, Line 2902: is not processed by then. So try to get the table again here if it is null */ > This seems fixing an existing bug. Can we split this into another patch? Done http://gerrit.cloudera.org:8080/#/c/21065/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@3105 PS8, Line 3105: tableWriteId.getDbName(), tableWriteId.getTblName()); > I think we should deduplicate the table names before invalidating them. The Done http://gerrit.cloudera.org:8080/#/c/21065/8/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/21065/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1006 PS8, Line 1006: try { > Can we add a log for this? Done http://gerrit.cloudera.org:8080/#/c/21065/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1188 PS8, Line 1188: if (!event.onFailure(e)) { throw e; } > Can we add a log when throwing the exception? e.g. "Unable to handle failur Done http://gerrit.cloudera.org:8080/#/c/21065/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1190 PS8, Line 1190: LOG.error(event.debugString("Failed to handle event processing failure")); > The invalidation in onFailure() could still fail by bugs like IMPALA-12831. Done -- To view, visit http://gerrit.cloudera.org:8080/21065 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Ge
[Impala-ASF-CR] IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21099 ) Change subject: IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21099 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4869eb863ff0afe8f691ccf2fd681a92d36b405c Gerrit-Change-Number: 21099 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 04 Mar 2024 13:23:46 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP]IMPALA-12832: Implicit invalidate metadata on event failures
k.venureddy2...@gmail.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21106 Change subject: [WIP]IMPALA-12832: Implicit invalidate metadata on event failures .. [WIP]IMPALA-12832: Implicit invalidate metadata on event failures At present, failure in event processing needs manual invalidate metadata. This patch implicitly invalidates the table upon failures in processing of table events with new 'process_event_failure' flag. And a new 'auto_global_invalidate_metadata' flag is added to global invalidate metadata automatically when event processor goes to non-active state. Note: Also introduced a config 'inject_process_event_failure_event_types' for automated tests to simulate event processor failures. This config is used to specify what event types can be intentionally failed. This config should only be used for testing purpose. Testing: - Added end-to-end tests to mimic failures in event processor and verified that event processor is active - Passed FE tests Change-Id: Ib11c554f5692e4fb8a4c63ef5ba8f8ae0e787826 --- M be/src/catalog/catalog-server.cc M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java 6 files changed, 52 insertions(+), 59 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/21106/1 -- To view, visit http://gerrit.cloudera.org:8080/21106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib11c554f5692e4fb8a4c63ef5ba8f8ae0e787826 Gerrit-Change-Number: 21106 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward
[Impala-ASF-CR] IMPALA-12845: Crash with DESCRIBE on a complex type from an Iceberg table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21105 ) Change subject: IMPALA-12845: Crash with DESCRIBE on a complex type from an Iceberg table .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15386/ : 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/21105 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5eda21a41167cc1fda183aa16fd6276a6a16f5d3 Gerrit-Change-Number: 21105 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 04 Mar 2024 13:01:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12845: Crash with DESCRIBE on a complex type from an Iceberg table
Daniel Becker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21105 Change subject: IMPALA-12845: Crash with DESCRIBE on a complex type from an Iceberg table .. IMPALA-12845: Crash with DESCRIBE on a complex type from an Iceberg table A DESCRIBE statement on a complex column contained in an Iceberg table runs into a DCHECK and crashes Impala. An example with an array: describe functional_parquet.iceberg_resolution_test_external.phone Note that this also happens with Iceberg metadata tables, for example: describe functional_parquet.iceberg_query_metadata.\ entries.readable_metrics; With non-Iceberg tables there is no error. The problem is that for Iceberg tables, the DESCRIBE statement returns four columns: "name", "type", "comment" and "nullable" (only Iceberg and Kudu tables have "nullable"). However, the DESCRIBE statement response for complex types only contains the first three columns as they are always nullable. But as the table is an Iceberg table, the 'metadata_' field of HS2ColumnarResultSet is still populated with four columns. The DCHECK in HS2ColumnarResultSet::AddOneRow() expects the number of columns to be the same in the DESCRIBE statement response and the 'metadata_' field. This commit solves the problem by only adding the "nullable" column to the 'metadata_' field if the target of the DESCRIBE statement is a table, not a complex type. Note that Kudu tables do not support complex types so this issue does not arise there. This change also addresses a minor issue: DescribeTableStmt::analyze() did not check whether the statement was already analyzed and after analysis did not set the 'analyzer_' field which would indicate that analysis had already been done. This is now corrected. Testing: - added tests in describe-path.test for arrays, maps and structs from regular Iceberg tables and metadata tables. Change-Id: I5eda21a41167cc1fda183aa16fd6276a6a16f5d3 --- M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/service/Frontend.java M testdata/workloads/functional-query/queries/QueryTest/describe-path.test 3 files changed, 91 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/21105/1 -- To view, visit http://gerrit.cloudera.org:8080/21105 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I5eda21a41167cc1fda183aa16fd6276a6a16f5d3 Gerrit-Change-Number: 21105 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Becker
[Impala-ASF-CR] IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/21099 ) Change subject: IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations .. Patch Set 1: (7 comments) great job with this quick fix, Zoltan! I added some nits but in general the patch is good. http://gerrit.cloudera.org:8080/#/c/21099/1/be/src/runtime/dml-exec-state.h File be/src/runtime/dml-exec-state.h: http://gerrit.cloudera.org:8080/#/c/21099/1/be/src/runtime/dml-exec-state.h@135 PS1, Line 135: const std::vector& ReferencedDataFilesByPositionDeletes() { since it returns const ref, can the function also be const? http://gerrit.cloudera.org:8080/#/c/21099/1/be/src/runtime/dml-exec-state.h@170 PS1, Line 170: referenced_data_files_by_position_deletes_ nit: data_files_referenced_by_position_deletes http://gerrit.cloudera.org:8080/#/c/21099/1/common/protobuf/control_service.proto File common/protobuf/control_service.proto: http://gerrit.cloudera.org:8080/#/c/21099/1/common/protobuf/control_service.proto@115 PS1, Line 115: referenced_data_files_by_position_deletes nit: data_files_referenced_by_position_deletes? http://gerrit.cloudera.org:8080/#/c/21099/1/common/thrift/CatalogService.thrift File common/thrift/CatalogService.thrift: http://gerrit.cloudera.org:8080/#/c/21099/1/common/thrift/CatalogService.thrift@249 PS1, Line 249: referenced_data_files_by_position_deletes nit: data_files_referenced_by_position_deletes maybe? http://gerrit.cloudera.org:8080/#/c/21099/1/tests/stress/test_update_stress.py File tests/stress/test_update_stress.py: http://gerrit.cloudera.org:8080/#/c/21099/1/tests/stress/test_update_stress.py@269 PS1, Line 269: test_concurrent_deletes_and_updates nit: this table name omits optimize. test_concurrent_write_and_optimize? http://gerrit.cloudera.org:8080/#/c/21099/1/tests/stress/test_update_stress.py@277 PS1, Line 277: for i in range(num_rows): Could you add a comment that here you prepare the INSERT query so that these rows can be put into the same file? http://gerrit.cloudera.org:8080/#/c/21099/1/tests/stress/test_update_stress.py@283 PS1, Line 283: flag I know we've been using this variable name for a while now, but I feel that the name doesn't suggest anything what it is used for. Can we rename this 'all_rows_deleted' or such? -- To view, visit http://gerrit.cloudera.org:8080/21099 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4869eb863ff0afe8f691ccf2fd681a92d36b405c Gerrit-Change-Number: 21099 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 04 Mar 2024 12:15:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21099 ) Change subject: IMPALA-12860: Invoke validateDataFilesExist for RowDelta operations .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10349/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21099 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4869eb863ff0afe8f691ccf2fd681a92d36b405c Gerrit-Change-Number: 21099 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 04 Mar 2024 09:47:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21096 ) Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/21096/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/21096/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6799 PS1, Line 6799: List eventId > Ah, sorry to misunderstood this. That's really a problem! So we shouldn't u A good news is that this only happens when --enable_skipping_older_events is true and it's false by default: https://github.com/apache/impala/blob/784971c018c2dc44c53d7c0f366ad49cd8681ac6/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java#L1199 Filed IMPALA-12865 to track this issue. -- To view, visit http://gerrit.cloudera.org:8080/21096 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504 Gerrit-Change-Number: 21096 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang 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: Mon, 04 Mar 2024 08:23:13 + Gerrit-HasComments: Yes