You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2018/09/06 02:39:25 UTC
[Impala-ASF-CR] IMPALA-7541. Avoid initializing Metrics for IncompleteTables
Hello Bharath Vissapragada,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/11393
to review the following change.
Change subject: IMPALA-7541. Avoid initializing Metrics for IncompleteTables
......................................................................
IMPALA-7541. Avoid initializing Metrics for IncompleteTables
Each 'Metrics' object is relatively large, and incomplete tables don't
support viewing metrics anyway. This makes the initialization of Metrics
lazy for IncompleteTable.
I compared a jmap -histo:live from the catalogd before and after this change,
after having loaded one table.
Object counts are reduced as follows:
java.util.concurrent.atomic.LongAdder: 16760 -> 20 (-16740)
com.codahale.metrics.LongAdderProxy$JdkProvider$1: 16760 -> 20 (-16740)
java.util.concurrent.atomic.AtomicLong: 14686 -> 4641 (-10045)
com.codahale.metrics.EWMA: 10056 -> 12 (-10044)
java.util.concurrent.ConcurrentSkipListMap$Node: 3353 -> 5 (-3348)
com.codahale.metrics.Meter: 3352 -> 4 (-3348)
java.util.concurrent.locks.ReentrantReadWriteLock: 3353 -> 5 (-3348)
com.codahale.metrics.ExponentiallyDecayingReservoir: 3352 -> 4 (-3348)
java.util.concurrent.locks.ReentrantReadWriteLock$Sync$ThreadLocalHoldCounter: 3353 -> 5 (-3348)
com.codahale.metrics.Histogram: 3352 -> 4 (-3348)
java.util.concurrent.locks.ReentrantReadWriteLock$NonfairSync: 3352 -> 4 (-3348)
java.util.concurrent.ConcurrentSkipListMap: 3352 -> 4 (-3348)
java.util.concurrent.ConcurrentSkipListMap$HeadIndex: 3352 -> 4 (-3348)
com.codahale.metrics.Timer: 3352 -> 4 (-3348)
java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock: 3353 -> 5 (-3348)
java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock: 3353 -> 5 (-3348)
java.util.concurrent.ConcurrentHashMap$Node: 38419 -> 35140 (-3279)
java.util.concurrent.locks.ReentrantLock: 2277 -> 1161 (-1116)
java.util.concurrent.locks.ReentrantLock$NonfairSync: 2309 -> 1193 (-1116)
com.codahale.metrics.MetricRegistry: 1117 -> 1 (-1116)
[Ljava.util.concurrent.ConcurrentHashMap$Node;: 1264 -> 148 (-1116)
java.util.concurrent.ConcurrentHashMap: 1281 -> 165 (-1116)
java.util.concurrent.CopyOnWriteArrayList: 1126 -> 10 (-1116)
org.apache.impala.common.Metrics: 1117 -> 1 (-1116)
[Ljava.lang.Object;: 4441 -> 3346 (-1095)
(94 objects per table)
With heap usage (bytes) reduced as follows:
java.util.concurrent.atomic.LongAdder: 536320 -> 640 (-535680)
com.codahale.metrics.EWMA: 482688 -> 576 (-482112)
com.codahale.metrics.LongAdderProxy$JdkProvider$1: 402240 -> 480 (-401760)
java.util.concurrent.atomic.AtomicLong: 352464 -> 111384 (-241080)
com.codahale.metrics.ExponentiallyDecayingReservoir: 187712 -> 224 (-187488)
com.codahale.metrics.Meter: 160896 -> 192 (-160704)
java.util.concurrent.locks.ReentrantReadWriteLock$NonfairSync: 160896 -> 192 (-160704)
java.util.concurrent.ConcurrentSkipListMap: 160896 -> 192 (-160704)
java.util.concurrent.ConcurrentSkipListMap$HeadIndex: 107264 -> 128 (-107136)
java.util.concurrent.ConcurrentHashMap$Node: 1229408 -> 1124480 (-104928)
[Ljava.util.concurrent.ConcurrentHashMap$Node;: 532192 -> 442912 (-89280)
java.util.concurrent.ConcurrentSkipListMap$Node: 80472 -> 120 (-80352)
java.util.concurrent.locks.ReentrantReadWriteLock: 80472 -> 120 (-80352)
com.codahale.metrics.Histogram: 80448 -> 96 (-80352)
com.codahale.metrics.Timer: 80448 -> 96 (-80352)
java.util.concurrent.ConcurrentHashMap: 81984 -> 10560 (-71424)
java.util.concurrent.locks.ReentrantReadWriteLock$Sync$ThreadLocalHoldCounter: 53648 -> 80 (-53568)
java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock: 53648 -> 80 (-53568)
java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock: 53648 -> 80 (-53568)
java.util.concurrent.locks.ReentrantLock$NonfairSync: 73888 -> 38176 (-35712)
com.codahale.metrics.MetricRegistry: 26808 -> 24 (-26784)
java.util.concurrent.CopyOnWriteArrayList: 27024 -> 240 (-26784)
java.util.concurrent.locks.ReentrantLock: 36432 -> 18576 (-17856)
org.apache.impala.common.Metrics: 17872 -> 16 (-17856)
[Ljava.lang.Object;: 260912 -> 243840 (-17072)
This works out to a total of 3249KB saved (2981 bytes per table). In a
large production environment with 100k tables this would save 284MB of
heap and 9.4M objects.
Change-Id: Id0bcaa9c45a8cf75266d4b7c16cc14f0fd669b92
---
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/KuduTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
4 files changed, 13 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/11393/1
--
To view, visit http://gerrit.cloudera.org:8080/11393
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id0bcaa9c45a8cf75266d4b7c16cc14f0fd669b92
Gerrit-Change-Number: 11393
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
[Impala-ASF-CR] IMPALA-7541. Avoid initializing Metrics for IncompleteTables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11393 )
Change subject: IMPALA-7541. Avoid initializing Metrics for IncompleteTables
......................................................................
Patch Set 1: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3123/
--
To view, visit http://gerrit.cloudera.org:8080/11393
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0bcaa9c45a8cf75266d4b7c16cc14f0fd669b92
Gerrit-Change-Number: 11393
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 06 Sep 2018 18:04:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7541. Avoid initializing Metrics for IncompleteTables
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11393 )
Change subject: IMPALA-7541. Avoid initializing Metrics for IncompleteTables
......................................................................
Patch Set 1:
No problem. I bumped the priority on the JIRA, though I dont have much time to work on this in the coming weeks.
--
To view, visit http://gerrit.cloudera.org:8080/11393
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0bcaa9c45a8cf75266d4b7c16cc14f0fd669b92
Gerrit-Change-Number: 11393
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 26 Nov 2018 18:55:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7541. Avoid initializing Metrics for IncompleteTables
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11393 )
Change subject: IMPALA-7541. Avoid initializing Metrics for IncompleteTables
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11393/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/11393/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1194
PS1, Line 1194: initMetrics();
> Doesn't this reset the metrics even for incremental loads? Also doesn't it
ah, indeed you're right, I think I need to find a different way to accomplish this patch. I didn't run tests before posting it for review :)
--
To view, visit http://gerrit.cloudera.org:8080/11393
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0bcaa9c45a8cf75266d4b7c16cc14f0fd669b92
Gerrit-Change-Number: 11393
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 06 Sep 2018 22:36:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7541. Avoid initializing Metrics for IncompleteTables
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11393 )
Change subject: IMPALA-7541. Avoid initializing Metrics for IncompleteTables
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11393/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/11393/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1194
PS1, Line 1194: initMetrics();
Doesn't this reset the metrics even for incremental loads? Also doesn't it hit the Preconditions check?
--
To view, visit http://gerrit.cloudera.org:8080/11393
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0bcaa9c45a8cf75266d4b7c16cc14f0fd669b92
Gerrit-Change-Number: 11393
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 06 Sep 2018 21:48:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7541. Avoid initializing Metrics for IncompleteTables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11393 )
Change subject: IMPALA-7541. Avoid initializing Metrics for IncompleteTables
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/602/ : 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/11393
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0bcaa9c45a8cf75266d4b7c16cc14f0fd669b92
Gerrit-Change-Number: 11393
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 06 Sep 2018 03:21:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7541. Avoid initializing Metrics for IncompleteTables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11393 )
Change subject: IMPALA-7541. Avoid initializing Metrics for IncompleteTables
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3123/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/11393
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0bcaa9c45a8cf75266d4b7c16cc14f0fd669b92
Gerrit-Change-Number: 11393
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 06 Sep 2018 16:28:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7541. Avoid initializing Metrics for IncompleteTables
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has abandoned this change. ( http://gerrit.cloudera.org:8080/11393 )
Change subject: IMPALA-7541. Avoid initializing Metrics for IncompleteTables
......................................................................
Abandoned
Todd, abandoning for now, doesn't look like the right fix. Feel free to reopen.
--
To view, visit http://gerrit.cloudera.org:8080/11393
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Id0bcaa9c45a8cf75266d4b7c16cc14f0fd669b92
Gerrit-Change-Number: 11393
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>