Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/16375 )
Change subject: IMPALA-10076: Reduce partition level update logs ...................................................................... Patch Set 3: (4 comments) Addressed the comments. Thanks for your review, Tim! http://gerrit.cloudera.org:8080/#/c/16375/2/be/src/catalog/catalog-server.cc File be/src/catalog/catalog-server.cc: http://gerrit.cloudera.org:8080/#/c/16375/2/be/src/catalog/catalog-server.cc@729 PS2, Line 729: return -1; > There's an overflow here, since the return value is an unsigned int - it'll Oops! Thanks for pointing this! http://gerrit.cloudera.org:8080/#/c/16375/2/be/src/catalog/catalog-server.cc@737 PS2, Line 737: // Skip logging partition items since FE will log their summary (IMPALA-10076). > Maybe leave the JIRA here for context? Done http://gerrit.cloudera.org:8080/#/c/16375/2/be/src/service/fe-support.cc File be/src/service/fe-support.cc: http://gerrit.cloudera.org:8080/#/c/16375/2/be/src/service/fe-support.cc@483 PS2, Line 483: int res > int or int32_t to avoid the overflow Thanks for pointing this! http://gerrit.cloudera.org:8080/#/c/16375/2/fe/src/main/java/org/apache/impala/catalog/PartitionMetaSummary.java File fe/src/main/java/org/apache/impala/catalog/PartitionMetaSummary.java: http://gerrit.cloudera.org:8080/#/c/16375/2/fe/src/main/java/org/apache/impala/catalog/PartitionMetaSummary.java@125 PS2, Line 125: } else if (partName.compareTo(partNames[0]) < 0) { > This all feels a little complex, but I think it makes sense and I don't thi Yeah, I was thinking about other simpler solutions but I was not satified on them: a) Just showing the first 2 and last one partition name, regardless of their comparision. However, the coordinator will receive them out of order, just showing the first 2 and last one can't help to understand the whole affected partition group. b) Keeping all the partition names. Sort them and then show the first 2 and last one. I don't want to introduce any overhead on sorting, especially when processing tables with >100K partitions. So I finally choose the current solution, which won't lose too much infomation and won't increase the complexity (still O(N)). -- To view, visit http://gerrit.cloudera.org:8080/16375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic48946b2f8b0be1e73988092d03a004836f1b368 Gerrit-Change-Number: 16375 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Mon, 07 Sep 2020 13:59:54 +0000 Gerrit-HasComments: Yes
