[Impala-ASF-CR] fe: improve logging for metadata loading
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13463 ) Change subject: fe: improve logging for metadata loading .. fe: improve logging for metadata loading This annotates various catalogd-internal calls associated with refreshing and loading metadata with a 'String why' parameter, useful for logging. This can help understand why a particular piece of metadata was loaded, and in the case of REFRESH calls, who issued the original refresh. Additionally, some of the log statements were improved to add a bit of extra detail such as the list of partitions being refreshed (abbreviated) and whether or not the table schema is being refreshed. Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Reviewed-on: http://gerrit.cloudera.org:8080/13463 Tested-by: Impala Public Jenkins Reviewed-by: Vihang Karajgaonkar --- M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogdTableInvalidatorTest.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java 18 files changed, 207 insertions(+), 144 deletions(-) Approvals: Impala Public Jenkins: Verified Vihang Karajgaonkar: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] fe: improve logging for metadata loading
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13463 ) Change subject: fe: improve logging for metadata loading .. Patch Set 5: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/13463/3/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/13463/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@a948 PS3, Line 948: > OK. Planning on working on a nicer patch to expose these timings as more st thanks -- To view, visit http://gerrit.cloudera.org:8080/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 18 Jun 2019 00:21:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] fe: improve logging for metadata loading
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13463 ) Change subject: fe: improve logging for metadata loading .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Sat, 15 Jun 2019 04:41:34 + Gerrit-HasComments: No
[Impala-ASF-CR] fe: improve logging for metadata loading
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13463 ) Change subject: fe: improve logging for metadata loading .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3634/ : 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/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Sat, 15 Jun 2019 00:00:52 + Gerrit-HasComments: No
[Impala-ASF-CR] fe: improve logging for metadata loading
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13463 ) Change subject: fe: improve logging for metadata loading .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4481/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Fri, 14 Jun 2019 23:14:29 + Gerrit-HasComments: No
[Impala-ASF-CR] fe: improve logging for metadata loading
Hello Vihang Karajgaonkar, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13463 to look at the new patch set (#4). Change subject: fe: improve logging for metadata loading .. fe: improve logging for metadata loading This annotates various catalogd-internal calls associated with refreshing and loading metadata with a 'String why' parameter, useful for logging. This can help understand why a particular piece of metadata was loaded, and in the case of REFRESH calls, who issued the original refresh. Additionally, some of the log statements were improved to add a bit of extra detail such as the list of partitions being refreshed (abbreviated) and whether or not the table schema is being refreshed. Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 --- M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogdTableInvalidatorTest.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java 18 files changed, 207 insertions(+), 144 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/13463/4 -- To view, visit http://gerrit.cloudera.org:8080/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] fe: improve logging for metadata loading
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13463 ) Change subject: fe: improve logging for metadata loading .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/13463/3/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/13463/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@a948 PS3, Line 948: > having this and the one after fetchAllPartitions call seems useful to quick OK. Planning on working on a nicer patch to expose these timings as more structured information and even back to the profile of the user who submitted it, but will just add these back for now. http://gerrit.cloudera.org:8080/#/c/13463/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@103 PS3, Line 103: > nit, unnecessary new line Done -- To view, visit http://gerrit.cloudera.org:8080/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Fri, 14 Jun 2019 22:58:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] fe: improve logging for metadata loading
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13463 ) Change subject: fe: improve logging for metadata loading .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/13463/1/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/13463/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@515 PS1, Line 515: needed to fetch partition stats > thats fine with. I don't have strong preference on this. edit : thats fine with me. -- To view, visit http://gerrit.cloudera.org:8080/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 13 Jun 2019 22:14:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] fe: improve logging for metadata loading
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13463 ) Change subject: fe: improve logging for metadata loading .. Patch Set 3: (3 comments) Thanks for making changes. Left a couple of comments below. Rest looks good to me. http://gerrit.cloudera.org:8080/#/c/13463/1/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/13463/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@515 PS1, Line 515: needed to fetch partition stats > Given they're only used in one place, and not 'magic numbers' that benefit thats fine with. I don't have strong preference on this. http://gerrit.cloudera.org:8080/#/c/13463/3/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/13463/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@a948 PS3, Line 948: having this and the one after fetchAllPartitions call seems useful to quickly determine how much time it took to fetch all the partitions for a table. I think we should re-instate these logs. http://gerrit.cloudera.org:8080/#/c/13463/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@103 PS3, Line 103: nit, unnecessary new line -- To view, visit http://gerrit.cloudera.org:8080/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 13 Jun 2019 22:14:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] fe: improve logging for metadata loading
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13463 ) Change subject: fe: improve logging for metadata loading .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3617/ : 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/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 13 Jun 2019 21:06:42 + Gerrit-HasComments: No
[Impala-ASF-CR] fe: improve logging for metadata loading
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/13463 )
Change subject: fe: improve logging for metadata loading
..
Patch Set 1:
(7 comments)
patch set r2 has fixes for the comments. r3 will rebase to tip of master
http://gerrit.cloudera.org:8080/#/c/13463/1/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/13463/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1954
PS1, Line 1954: why
> will do
Done
http://gerrit.cloudera.org:8080/#/c/13463/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/13463/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@103
PS1, Line 103: hiveexec
> mistake -- eclipse loves to auto-import shaded crap :(
Done
http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@628
PS1, Line 628: new Exception()
> yea, this was some debugging junk I left in by mistake
Done
http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@891
PS1, Line 891: org.apache.hadoop.hive.metastore.api.Table msTbl, String
why) throws TableLoadingException {
> line too long (98 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13463/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/13463/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@813
PS1, Line 813: insert
> would be used to say processing table-level insert event from HMS for consi
Done
http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
File fe/src/test/java/org/apache/impala/catalog/CatalogTest.java:
http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java@170
PS1, Line 170: assertNotNull(catalog_.getOrLoadTable(hbaseDb.getName(),
"alltypessmallbinary", "test"));
> line too long (93 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java@172
PS1, Line 172: assertNotNull(catalog_.getOrLoadTable(hbaseDb.getName(),
"hbasealltypeserror", "test"));
> line too long (92 > 90)
Done
--
To view, visit http://gerrit.cloudera.org:8080/13463
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8
Gerrit-Change-Number: 13463
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Thu, 13 Jun 2019 20:23:24 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] fe: improve logging for metadata loading
Hello Vihang Karajgaonkar, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13463 to look at the new patch set (#2). Change subject: fe: improve logging for metadata loading .. fe: improve logging for metadata loading This annotates various catalogd-internal calls associated with refreshing and loading metadata with a 'String why' parameter, useful for logging. This can help understand why a particular piece of metadata was loaded, and in the case of REFRESH calls, who issued the original refresh. Additionally, some of the log statements were improved to add a bit of extra detail such as the list of partitions being refreshed (abbreviated) and whether or not the table schema is being refreshed. Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 --- M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogdTableInvalidatorTest.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java 18 files changed, 207 insertions(+), 147 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/13463/2 -- To view, visit http://gerrit.cloudera.org:8080/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] fe: improve logging for metadata loading
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13463 ) Change subject: fe: improve logging for metadata loading .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/13463/1/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/13463/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@515 PS1, Line 515: needed to fetch partition stats > Can we create constants for these reasons. Seems they never change. Given they're only used in one place, and not 'magic numbers' that benefit from a descriptive variable name, not sure what the benefit is. Doesn't it just add another line of code and cause you to have to navigate around when reading the code? http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1954 PS1, Line 1954: why > Nit, I think have nouns as the arguments are more readable. eg: reload(Tabl will do http://gerrit.cloudera.org:8080/#/c/13463/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/13463/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@103 PS1, Line 103: hiveexec > Is there a particular reason to use the shaded version of Joiner? mistake -- eclipse loves to auto-import shaded crap :( http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@628 PS1, Line 628: new Exception() > This will potentially add a lot of traces in the log. Don't we generally lo yea, this was some debugging junk I left in by mistake -- To view, visit http://gerrit.cloudera.org:8080/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 13 Jun 2019 20:10:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] fe: improve logging for metadata loading
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13463 ) Change subject: fe: improve logging for metadata loading .. Patch Set 1: (5 comments) Patch looks good to me. left some non-blocking nits and naming suggestions. http://gerrit.cloudera.org:8080/#/c/13463/1/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/13463/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@515 PS1, Line 515: needed to fetch partition stats Can we create constants for these reasons. Seems they never change. http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1954 PS1, Line 1954: why Nit, I think have nouns as the arguments are more readable. eg: reload(Table tbl, String reason) Same comment for the other method arg names where the patch uses "why" http://gerrit.cloudera.org:8080/#/c/13463/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/13463/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@103 PS1, Line 103: hiveexec Is there a particular reason to use the shaded version of Joiner? http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@628 PS1, Line 628: new Exception() This will potentially add a lot of traces in the log. Don't we generally log the trace for errors. Could be confusing to the users when they see these in the log. Perhaps change this to debug? http://gerrit.cloudera.org:8080/#/c/13463/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/13463/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@813 PS1, Line 813: insert would be used to say processing table-level insert event from HMS for consistency with the other message. Also, might want to change it to INSERT event similar to other messages. -- To view, visit http://gerrit.cloudera.org:8080/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 11 Jun 2019 18:16:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] fe: improve logging for metadata loading
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13463 ) Change subject: fe: improve logging for metadata loading .. Patch Set 1: Vihang, any chance you can take a look at this? -- To view, visit http://gerrit.cloudera.org:8080/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Mon, 10 Jun 2019 19:11:03 + Gerrit-HasComments: No
[Impala-ASF-CR] fe: improve logging for metadata loading
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13463 ) Change subject: fe: improve logging for metadata loading .. Patch Set 1: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/3432/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 30 May 2019 05:06:33 + Gerrit-HasComments: No
[Impala-ASF-CR] fe: improve logging for metadata loading
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/13463 )
Change subject: fe: improve logging for metadata loading
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13463/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/13463/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@891
PS1, Line 891: org.apache.hadoop.hive.metastore.api.Table msTbl, String
why) throws TableLoadingException {
line too long (98 > 90)
http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
File fe/src/test/java/org/apache/impala/catalog/CatalogTest.java:
http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java@170
PS1, Line 170: assertNotNull(catalog_.getOrLoadTable(hbaseDb.getName(),
"alltypessmallbinary", "test"));
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java@172
PS1, Line 172: assertNotNull(catalog_.getOrLoadTable(hbaseDb.getName(),
"hbasealltypeserror", "test"));
line too long (92 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/13463
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8
Gerrit-Change-Number: 13463
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Thu, 30 May 2019 04:26:54 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] fe: improve logging for metadata loading
Hello Vihang Karajgaonkar, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/13463 to review the following change. Change subject: fe: improve logging for metadata loading .. fe: improve logging for metadata loading This annotates various catalogd-internal calls associated with refreshing and loading metadata with a 'String why' parameter, useful for logging. This can help understand why a particular piece of metadata was loaded, and in the case of REFRESH calls, who issued the original refresh. Additionally, some of the log statements were improved to add a bit of extra detail such as the list of partitions being refreshed (abbreviated) and whether or not the table schema is being refreshed. Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 --- M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogdTableInvalidatorTest.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java 18 files changed, 204 insertions(+), 147 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/13463/1 -- To view, visit http://gerrit.cloudera.org:8080/13463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8 Gerrit-Change-Number: 13463 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar
