Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20367 )
Change subject: IMPALA-10976: Sync db/table to latest HMS event for all DDL/DMLs ...................................................................... Patch Set 33: (5 comments) > Patch Set 31: Verified-1 > > Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10201/ There are still test failures. Retriggered the job on the latest patch set. http://gerrit.cloudera.org:8080/#/c/20367/33/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/20367/33/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1401 PS33, Line 1401: debug Let's use trace() since this will be logged for each new loaded partition. For a table with 10K partitions, when it's loaded, we'll log 10K lines for this.. Use parameters so the string won't be constructed when TRACE logging is off, i.e. LOG.trace("Previous id for the partition object is not set. Partition info: {}", obj.hdfs_partition); http://gerrit.cloudera.org:8080/#/c/20367/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/20367/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3357 PS33, Line 3357: else { : // when table is replicated we let the HMS API handle the file deletion logic : // otherwise we delete the files. : Collection<? extends FeFsPartition> parts = FeCatalogUtils : .loadAllPartitions(hdfsTable); : for (FeFsPartition part : parts) { : FileSystemUtil.deleteAllVisibleFiles(new Path(part.getLocation())); : } : LOG.trace("Time elapsed after deleting files for table {}: {} msec", : table.getFullName(), sw.elapsed(TimeUnit.MILLISECONDS)); : } It'd be better to keep these unchanged since previously they are outside the try-block so don't need to acquire a MetaStoreClient. http://gerrit.cloudera.org:8080/#/c/20367/33/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/20367/33/tests/common/impala_test_suite.py@958 PS33, Line 958: result = cls.__execute_query(impalad_client, query, query_options, user) Let's check success first, i.e. like what we does in execute_query_expect_success(): assert result.success http://gerrit.cloudera.org:8080/#/c/20367/33/tests/common/impala_test_suite.py@1046 PS33, Line 1046: self nit: use 'cls' for classmethod http://gerrit.cloudera.org:8080/#/c/20367/33/tests/common/impala_test_suite.py@1052 PS33, Line 1052: self nit: use 'cls' for classmethod -- To view, visit http://gerrit.cloudera.org:8080/20367 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia250d0a943838086c187e5cb7c60035e5a564bbf Gerrit-Change-Number: 20367 Gerrit-PatchSet: 33 Gerrit-Owner: Sai Hemanth Gantasala <saihema...@cloudera.com> Gerrit-Reviewer: Anonymous Coward <k.venureddy2...@gmail.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Sai Hemanth Gantasala <saihema...@cloudera.com> Gerrit-Comment-Date: Mon, 29 Jan 2024 08:28:27 +0000 Gerrit-HasComments: Yes
