Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19863 )

Change subject: IMPALA-11941: Support Java 17 in Impala
......................................................................


Patch Set 26:

(1 comment)

> I also added metrics and compared jamm vs ehcache for a few
 > operations after running 
 > https://github.com/apache/impala/blob/master/tests/custom_cluster/test_local_catalog.py#L472-L518.
 > They're almost identical:
 >
 > Jamm:
 > catalog.cache.entry-99th-size        14.10 KB        99th percentile size of
 > Impalad Catalog cache entries.
 > catalog.cache.entry-median-size      376.00 B        Median size of Impalad
 > Catalog cache entries.
 > catalog.cache.entry-stddev-size      2648.229397     Standard deviation for
 > size of Impalad Catalog cache entries.
 >
 > Ehcache:
 > catalog.cache.entry-99th-size        14.10 KB        99th percentile size of
 > Impalad Catalog cache entries.
 > catalog.cache.entry-median-size      376.00 B        Median size of Impalad
 > Catalog cache entries.
 > catalog.cache.entry-stddev-size      2639.141904     Standard deviation for
 > size of Impalad Catalog cache entries.
 >
 > I can push up the additional changes to allow switching back to
 > ehcache and adding these metrics (not sure stddev is worth it when
 > 99th percentile is more readable).

I agree that stddev might not be very useful. median and 99th percentile are 
useful. Is max useful?

http://gerrit.cloudera.org:8080/#/c/19863/26/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:

http://gerrit.cloudera.org:8080/#/c/19863/26/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@2060
PS26, Line 2060:       if (entrySize_ != null) {
               :         entrySize_.update(size);
               :       }
So, the histogram and metrics are about everything that has been evaluated by 
the weigher. This is different from the current contents of the cache, because 
nothing happens on eviction.

The metric is still useful, but maybe we need to be clear about it in the 
description.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic378896f572e030a3a019646a96a32a07866a737
Gerrit-Change-Number: 19863
Gerrit-PatchSet: 26
Gerrit-Owner: Michael Smith <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Laszlo Gaal <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Wed, 21 Jun 2023 22:11:09 +0000
Gerrit-HasComments: Yes

Reply via email to