Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-4768: Improve logging of table loading. ......................................................................
Patch Set 3: (2 comments) To answer Alex's question for alternatives to logging approaches. I would prefer a more principled approach like java method annotations using AOP. I had a proof-of-concept patch out for review but it got discarded with some gerrit changes; I can resume it. The reason I prefer the annotation based approach is that a) avoids spurious log statements in the codebase and in the logs, b) it automatically gives latency of each annotated method execution, c) you can control the log level at which each set of methods is logged and d) can log information about input and output parameters. For performance monitoring and debugging I would prefer the use of something like Java JMX. I am not arguing we should do these changes for 5.10. I understand the immediate need for better logging in some cases but we should definitely consider other options other than adding log statements here and there. http://gerrit.cloudera.org:8080/#/c/5709/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: PS3, Line 241: BlockMetadataLoadStats > The file counts are specific to the block metadata loading. For full loads Thanks for explaining. Based on the name, I would also expect something like # blocks loaded, avg block size, etc. If we believe the information here will help us diagnose perf problems, I don't mind having this change. PS3, Line 244: // Number of files that were found to belong to the table being loaded. : public long tableFileCount = 0; > See above comment, hope it explains it. Hm, maybe a better name for these fields? e.g. examinedFilesCount and tableFileCount, or something along these lines? -- To view, visit http://gerrit.cloudera.org:8080/5709 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8de96d0cb6d09b2272b1925d42cb059367fe7196 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-HasComments: Yes
