You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Xiang Yang (Code Review)" <ge...@cloudera.org> on 2023/06/27 00:26:05 UTC

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Xiang Yang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20096


Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................

IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

The CatalogTableMetrics maintains a cache of 'org.apache.impala.catalog.
Table' which may obtain references to CatalogServiceCatalog 'dbCache_'
field's old value after 'invalidate metadata' triggered, in which case
the old db cache can't be recycled by GC, and causing a possible memory
leak in catalogd.

The solution is simpling the cache element's type to a new class
'TableMetric' which only contains necessary value fields and doesn't
contains complex reference such as 'org.apache.impala.catalog.Db'.

Testing:
 - Manually test:
   1. trigger CatalogTableMetrics's cache by executing general query on
      some tables and other SQL such as 'create table like' SQL.
   2. execute 'invalidate metadata' to trigger catalog version upgrade.
   3. dump catalogd's JVM memory image by command 'jmap -dump:live,
      format=b,file=<path> <catalog_pid>'.
   4. count the 'org.apache.impala.catalog.Db' class's object number
      by analyze the dump file using Eclipse Memory Analyzer, and check
      whether the Db's object number match the actual.
 - Add FE test 'CatalogTableMetricsTest'.

Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java
A fe/src/main/java/org/apache/impala/catalog/monitor/TableLoadingTimeHistogram.java
M fe/src/main/java/org/apache/impala/common/Pair.java
A fe/src/test/java/org/apache/impala/catalog/monitor/CatalogTableMetricsTest.java
5 files changed, 222 insertions(+), 59 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/20096/2
-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 2
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 8: Code-Review+1


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 8
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 09:45:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9477/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 8
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 07:58:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Xiang Yang (Code Review)" <ge...@cloudera.org>.
Xiang Yang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 4:

(3 comments)

> Patch Set 4:
> 
> (7 comments)
> 
> looks good, some minor comments

Hi wenzhe, thanks to point these style problems, no explicit style rules for Impala's java codes is a bit of a headache.
For now I indent java codes with 2 spaces and indent uncomplete lines with 4 spaces. Is this what you mean?

http://gerrit.cloudera.org:8080/#/c/20096/4/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/20096/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@20
PS4, Line 20: import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P100;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P50;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P75;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P95;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P99;
> import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Q
According to google java style: https://google.github.io/styleguide/javaguide.html#s3.3.1-wildcard-imports, 'import *' is not recommended.


http://gerrit.cloudera.org:8080/#/c/20096/4/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java
File fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java:

http://gerrit.cloudera.org:8080/#/c/20096/4/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java@27
PS4, Line 27: import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P100;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P50;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P75;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P95;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P99;
> change to "import static org.apache.impala.catalog.monitor.TableLoadingTime
same as above.


http://gerrit.cloudera.org:8080/#/c/20096/4/fe/src/main/java/org/apache/impala/catalog/monitor/TableLoadingTimeHistogram.java
File fe/src/main/java/org/apache/impala/catalog/monitor/TableLoadingTimeHistogram.java:

http://gerrit.cloudera.org:8080/#/c/20096/4/fe/src/main/java/org/apache/impala/catalog/monitor/TableLoadingTimeHistogram.java@34
PS4, Line 34: 8
> why initial with 8?
it's a small optimization, the default initial capacity of HashMap is 16, 8 is the smallest nth power of 2 which more than enum's size.



-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 4
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 02:56:17 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 7: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 7
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 08:11:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9476/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 8
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 07:44:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 6: Code-Review+1


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 6
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 04:09:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Xiang Yang (Code Review)" <ge...@cloudera.org>.
Xiang Yang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 6:

Hi wenzhe, I reformat the added java file with clang-format.


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 6
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 03:20:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................

IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

The CatalogTableMetrics maintains a cache of 'org.apache.impala.catalog.
Table' which may obtain references to CatalogServiceCatalog 'dbCache_'
field's old value after 'invalidate metadata' triggered, in which case
the old db cache can't be recycled by GC, and causing a possible memory
leak in catalogd.

The solution is simpling the cache element's type to a new class
'TableMetric' which only contains necessary value fields and doesn't
contains complex reference such as 'org.apache.impala.catalog.Db'.

Testing:
 - Manually test:
   1. trigger CatalogTableMetrics's cache by executing general query on
      some tables and other SQL such as 'create table like' SQL.
   2. execute 'invalidate metadata' to trigger catalog version upgrade.
   3. dump catalogd's JVM memory image by command 'jmap -dump:live,
      format=b,file=<path> <catalog_pid>'.
   4. count the 'org.apache.impala.catalog.Db' class's object number
      by analyze the dump file using Eclipse Memory Analyzer, and check
      whether the Db's object number match the actual.
 - Add FE test 'CatalogTableMetricsTest'.

Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Reviewed-on: http://gerrit.cloudera.org:8080/20096
Reviewed-by: Quanlong Huang <hu...@gmail.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java
A fe/src/main/java/org/apache/impala/catalog/monitor/TableLoadingTimeHistogram.java
M fe/src/main/java/org/apache/impala/common/Pair.java
M fe/src/main/java/org/apache/impala/util/TopNCache.java
A fe/src/test/java/org/apache/impala/catalog/monitor/CatalogTableMetricsTest.java
6 files changed, 238 insertions(+), 66 deletions(-)

Approvals:
  Quanlong Huang: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 10
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Xiang Yang (Code Review)" <ge...@cloudera.org>.
Xiang Yang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 4:

Hi wenzhe and quanlong, can you help me review this? thanks.


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 4
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Thu, 29 Jun 2023 02:34:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9466/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 7
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 08:11:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 9:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9478/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 9
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 09:45:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Xiang Yang (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20096

to look at the new patch set (#3).

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................

IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

The CatalogTableMetrics maintains a cache of 'org.apache.impala.catalog.
Table' which may obtain references to CatalogServiceCatalog 'dbCache_'
field's old value after 'invalidate metadata' triggered, in which case
the old db cache can't be recycled by GC, and causing a possible memory
leak in catalogd.

The solution is simpling the cache element's type to a new class
'TableMetric' which only contains necessary value fields and doesn't
contains complex reference such as 'org.apache.impala.catalog.Db'.

Testing:
 - Manually test:
   1. trigger CatalogTableMetrics's cache by executing general query on
      some tables and other SQL such as 'create table like' SQL.
   2. execute 'invalidate metadata' to trigger catalog version upgrade.
   3. dump catalogd's JVM memory image by command 'jmap -dump:live,
      format=b,file=<path> <catalog_pid>'.
   4. count the 'org.apache.impala.catalog.Db' class's object number
      by analyze the dump file using Eclipse Memory Analyzer, and check
      whether the Db's object number match the actual.
 - Add FE test 'CatalogTableMetricsTest'.

Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java
A fe/src/main/java/org/apache/impala/catalog/monitor/TableLoadingTimeHistogram.java
M fe/src/main/java/org/apache/impala/common/Pair.java
A fe/src/test/java/org/apache/impala/catalog/monitor/CatalogTableMetricsTest.java
5 files changed, 234 insertions(+), 59 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/20096/3
-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 3
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 2:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/20096/2/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/20096/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3326
PS2, Line 3326:     for (Pair<TTableName, Long> frequentTable : catalogTableMetrics.getFrequentlyAccessedTables()) {
line too long (100 > 90)


http://gerrit.cloudera.org:8080/#/c/20096/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3332
PS2, Line 3332:     for (Pair<TTableName, Long> mostFilesTable : catalogTableMetrics.getHighFileCountTables()) {
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/20096/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3338
PS2, Line 3338:     for (Pair<TTableName, TableLoadingTimeHistogram> longestLoadingTable : catalogTableMetrics.getLongMetadataLoadingTables()) {
line too long (128 > 90)


http://gerrit.cloudera.org:8080/#/c/20096/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3342
PS2, Line 3342:           longestLoadingTable.getSecond().getQuantile(TableLoadingTimeHistogram.Quantile.P50));
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/20096/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3344
PS2, Line 3344:           longestLoadingTable.getSecond().getQuantile(TableLoadingTimeHistogram.Quantile.P100));
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/20096/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3346
PS2, Line 3346:           longestLoadingTable.getSecond().getQuantile(TableLoadingTimeHistogram.Quantile.P75));
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/20096/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3348
PS2, Line 3348:           longestLoadingTable.getSecond().getQuantile(TableLoadingTimeHistogram.Quantile.P95));
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/20096/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3350
PS2, Line 3350:           longestLoadingTable.getSecond().getQuantile(TableLoadingTimeHistogram.Quantile.P99));
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/20096/2/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java
File fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java:

http://gerrit.cloudera.org:8080/#/c/20096/2/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java@51
PS2, Line 51:   private final TopNCache<TableMetric<TableLoadingTimeHistogram>, Long> longMetadataLoadingTables_;
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/20096/2/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java@59
PS2, Line 59:     frequentlyAccessedTables_ = new TopNCache<>(TableMetric::getSecond, num_tables_tracked, true);
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/20096/2/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java@63
PS2, Line 63:     highFileCountTables_ = new TopNCache<>(TableMetric::getSecond, num_tables_tracked, false);
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/20096/2/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java@121
PS2, Line 121:   public List<? extends Pair<TTableName, TableLoadingTimeHistogram>> getLongMetadataLoadingTables() {
line too long (101 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 2
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Jun 2023 00:27:02 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Xiang Yang (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Wenzhe Zhou, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20096

to look at the new patch set (#8).

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................

IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

The CatalogTableMetrics maintains a cache of 'org.apache.impala.catalog.
Table' which may obtain references to CatalogServiceCatalog 'dbCache_'
field's old value after 'invalidate metadata' triggered, in which case
the old db cache can't be recycled by GC, and causing a possible memory
leak in catalogd.

The solution is simpling the cache element's type to a new class
'TableMetric' which only contains necessary value fields and doesn't
contains complex reference such as 'org.apache.impala.catalog.Db'.

Testing:
 - Manually test:
   1. trigger CatalogTableMetrics's cache by executing general query on
      some tables and other SQL such as 'create table like' SQL.
   2. execute 'invalidate metadata' to trigger catalog version upgrade.
   3. dump catalogd's JVM memory image by command 'jmap -dump:live,
      format=b,file=<path> <catalog_pid>'.
   4. count the 'org.apache.impala.catalog.Db' class's object number
      by analyze the dump file using Eclipse Memory Analyzer, and check
      whether the Db's object number match the actual.
 - Add FE test 'CatalogTableMetricsTest'.

Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java
A fe/src/main/java/org/apache/impala/catalog/monitor/TableLoadingTimeHistogram.java
M fe/src/main/java/org/apache/impala/common/Pair.java
M fe/src/main/java/org/apache/impala/util/TopNCache.java
A fe/src/test/java/org/apache/impala/catalog/monitor/CatalogTableMetricsTest.java
6 files changed, 238 insertions(+), 66 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/20096/8
-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 8
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 9:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9479/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 9
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 12:44:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Xiang Yang (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Wenzhe Zhou, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20096

to look at the new patch set (#6).

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................

IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

The CatalogTableMetrics maintains a cache of 'org.apache.impala.catalog.
Table' which may obtain references to CatalogServiceCatalog 'dbCache_'
field's old value after 'invalidate metadata' triggered, in which case
the old db cache can't be recycled by GC, and causing a possible memory
leak in catalogd.

The solution is simpling the cache element's type to a new class
'TableMetric' which only contains necessary value fields and doesn't
contains complex reference such as 'org.apache.impala.catalog.Db'.

Testing:
 - Manually test:
   1. trigger CatalogTableMetrics's cache by executing general query on
      some tables and other SQL such as 'create table like' SQL.
   2. execute 'invalidate metadata' to trigger catalog version upgrade.
   3. dump catalogd's JVM memory image by command 'jmap -dump:live,
      format=b,file=<path> <catalog_pid>'.
   4. count the 'org.apache.impala.catalog.Db' class's object number
      by analyze the dump file using Eclipse Memory Analyzer, and check
      whether the Db's object number match the actual.
 - Add FE test 'CatalogTableMetricsTest'.

Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java
A fe/src/main/java/org/apache/impala/catalog/monitor/TableLoadingTimeHistogram.java
M fe/src/main/java/org/apache/impala/common/Pair.java
A fe/src/test/java/org/apache/impala/catalog/monitor/CatalogTableMetricsTest.java
5 files changed, 216 insertions(+), 65 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/20096/6
-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 6
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 9: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 9
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 13:54:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 9: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 9
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 12:44:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 3:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/13397/ : Initial code review checks failed. See linked job for details on the failure.


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 3
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Jun 2023 02:45:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Xiang Yang (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Wenzhe Zhou, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20096

to look at the new patch set (#5).

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................

IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

The CatalogTableMetrics maintains a cache of 'org.apache.impala.catalog.
Table' which may obtain references to CatalogServiceCatalog 'dbCache_'
field's old value after 'invalidate metadata' triggered, in which case
the old db cache can't be recycled by GC, and causing a possible memory
leak in catalogd.

The solution is simpling the cache element's type to a new class
'TableMetric' which only contains necessary value fields and doesn't
contains complex reference such as 'org.apache.impala.catalog.Db'.

Testing:
 - Manually test:
   1. trigger CatalogTableMetrics's cache by executing general query on
      some tables and other SQL such as 'create table like' SQL.
   2. execute 'invalidate metadata' to trigger catalog version upgrade.
   3. dump catalogd's JVM memory image by command 'jmap -dump:live,
      format=b,file=<path> <catalog_pid>'.
   4. count the 'org.apache.impala.catalog.Db' class's object number
      by analyze the dump file using Eclipse Memory Analyzer, and check
      whether the Db's object number match the actual.
 - Add FE test 'CatalogTableMetricsTest'.

Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java
A fe/src/main/java/org/apache/impala/catalog/monitor/TableLoadingTimeHistogram.java
M fe/src/main/java/org/apache/impala/common/Pair.java
A fe/src/test/java/org/apache/impala/catalog/monitor/CatalogTableMetricsTest.java
5 files changed, 242 insertions(+), 65 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/20096/5
-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 5
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13441/ : 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/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 5
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 03:15:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 7: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9466/


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 7
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 12:22:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 8: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 8
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 11:51:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 2:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/13395/ : Initial code review checks failed. See linked job for details on the failure.


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 2
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Jun 2023 00:47:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 8:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13466/ : 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/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 8
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 08:05:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 6: Code-Review+2

Nice work! Thanks for your contribution!


-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 6
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 08:10:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 4:

(7 comments)

looks good, some minor comments

http://gerrit.cloudera.org:8080/#/c/20096/4/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/20096/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@20
PS4, Line 20: import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P100;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P50;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P75;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P95;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P99;
import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.*;


http://gerrit.cloudera.org:8080/#/c/20096/4/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java
File fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java:

http://gerrit.cloudera.org:8080/#/c/20096/4/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java@27
PS4, Line 27: import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P100;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P50;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P75;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P95;
            : import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.P99;
change to "import static org.apache.impala.catalog.monitor.TableLoadingTimeHistogram.Quantile.*;"


http://gerrit.cloudera.org:8080/#/c/20096/4/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java@61
PS4, Line 61:         
nit: correct indent spaces here and below


http://gerrit.cloudera.org:8080/#/c/20096/4/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java@125
PS4, Line 125:   
nit: indent with 4 empty spaces


http://gerrit.cloudera.org:8080/#/c/20096/4/fe/src/main/java/org/apache/impala/catalog/monitor/TableLoadingTimeHistogram.java
File fe/src/main/java/org/apache/impala/catalog/monitor/TableLoadingTimeHistogram.java:

http://gerrit.cloudera.org:8080/#/c/20096/4/fe/src/main/java/org/apache/impala/catalog/monitor/TableLoadingTimeHistogram.java@28
PS4, Line 28:     
nit: correct indent spaces here and below


http://gerrit.cloudera.org:8080/#/c/20096/4/fe/src/main/java/org/apache/impala/catalog/monitor/TableLoadingTimeHistogram.java@34
PS4, Line 34: 8
why initial with 8?


http://gerrit.cloudera.org:8080/#/c/20096/4/fe/src/test/java/org/apache/impala/catalog/monitor/CatalogTableMetricsTest.java
File fe/src/test/java/org/apache/impala/catalog/monitor/CatalogTableMetricsTest.java:

http://gerrit.cloudera.org:8080/#/c/20096/4/fe/src/test/java/org/apache/impala/catalog/monitor/CatalogTableMetricsTest.java@33
PS4, Line 33:     
nit: correct indent spaces here and below



-- 
To view, visit http://gerrit.cloudera.org:8080/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 4
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Thu, 29 Jun 2023 13:55:26 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20096 )

Change subject: IMPALA-6876: Fix possible memory leak in CatalogTableMetrics after reset metadata.
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13442/ : 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/20096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91635cc40878cbb76b71ee7d81418ea753892c3
Gerrit-Change-Number: 20096
Gerrit-PatchSet: 6
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 03:41:56 +0000
Gerrit-HasComments: No