Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/22871 )
Change subject: IMPALA-12162: Checksum files before lock in INSERT ...................................................................... Patch Set 6: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/22871/6/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/22871/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7347 PS6, Line 7347: = null; : if (shouldGenerateInsertEvents(feFsTable)) { : try { : // Collect file checksums (and ACID dir path) before taking table lock. : fileMetadata = getFileMetadata( : feFsTable, update.getUpdated_partitions().values()); : catalogTimeline.markEvent("Collected file checksums"); : } catch (CatalogException e) { : // As with createInsertEvents, this is best-effort. Any errors in it should not : // fail the INSERT. : catalogTimeline.markEvent("Error collecting file checksums"); : LOG.error("Failed to collect insert metadata for table {}", : table.getFullName(), e); : } : } nit: updateCatalogImpl() is already a long method. Can we wrap these into getFileMetadata() to save some lines? http://gerrit.cloudera.org:8080/#/c/22871/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7908 PS6, Line 7908: FeFsTable tbl, nit: not used anymore http://gerrit.cloudera.org:8080/#/c/22871/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7918 PS6, Line 7918: FileMetadata metadata = fileMetadata.get(file); nit: add checkNotNull(metadata)? -- To view, visit http://gerrit.cloudera.org:8080/22871 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18f9686f5d53cf1e7c384684c25427fb5353e2af Gerrit-Change-Number: 22871 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Comment-Date: Tue, 20 May 2025 01:17:54 +0000 Gerrit-HasComments: Yes
