Jiawei Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14611 )
Change subject: IMPALA-9110: Add table loading time break-down metrics for HdfsTable ...................................................................... Patch Set 2: (4 comments) Thanks for the feedback! I add the time for measuring HMS loading table schema. For the table loading queue time, Quanlong suggest we should do the following: "Table queue time is not related to the table itself. But just like query queue time, it's an important metric to understand why the loading is slow. I think we can track it as a global metric that not bind to any tables. Showing p90th table loading queue time in recent time window (e.g. 1h) can be helpful." It makes a lot of sense to me. But I am wondering how will we approve this. Might need some guidance on getting the queue time done. http://gerrit.cloudera.org:8080/#/c/14611/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/14611/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@962 PS1, Line 962: > Does this also account for the time taken for loading file metadata? If yes I am not sure if I understand the question here. The time taken for updatePartitionsFromHms() will also contribute to storageMetadataLoadTime_ (which is time spent in the source systems loading/reloading the fs metadata for the table) And we have this metrics "storage-metadata-load-duration" to keep track of the storageMetadataLoadTime_. http://gerrit.cloudera.org:8080/#/c/14611/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@975 PS1, Line 975: _ = load > I would recommend something like Time Taken: <value> ns. here. Also, loggin Adding a PrettyPrint function for Time in frontend. Done. http://gerrit.cloudera.org:8080/#/c/14611/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1229 PS1, Line 1229: /** > I think this function does not invoke any HMS RPCs so the time will be very Done http://gerrit.cloudera.org:8080/#/c/14611/1/fe/src/main/java/org/apache/impala/catalog/Table.java File fe/src/main/java/org/apache/impala/catalog/Table.java: http://gerrit.cloudera.org:8080/#/c/14611/1/fe/src/main/java/org/apache/impala/catalog/Table.java@334 PS1, Line 334: } > I don't think there is much value in having this since we don't expect this Done -- To view, visit http://gerrit.cloudera.org:8080/14611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5381f9316df588b2004876c6cd9fb7e674085b10 Gerrit-Change-Number: 14611 Gerrit-PatchSet: 2 Gerrit-Owner: Jiawei Wang <jiawei.w...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Jiawei Wang <jiawei.w...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Comment-Date: Wed, 06 Nov 2019 06:00:37 +0000 Gerrit-HasComments: Yes
