Add call points of the new metrics #18351
Conversation
| public static void setCacheStorageSupplier(DoubleSupplier supplier) { | ||
| sCacheStorageSupplier = supplier; | ||
| } |
There was a problem hiding this comment.
allow clearing this static reference when the page meta store is closed. Otherwise, it cannot be GCed because the metrics system is holding a strong reference to it. Typically the meta store can hold up to several million PageInfo objects which consumes a considerable amount of memory.
There was a problem hiding this comment.
sounds good. where do you suggest adding the clearing?
| @@ -130,6 +131,7 @@ public static CacheManager create(AlluxioConfiguration conf, | |||
| if (isNettyDataTransmissionEnable) { | |||
| options.setIsAsyncWriteEnabled(false); | |||
| } | |||
| MultiDimensionalMetricsSystem.setCacheStorageSupplier(pageMetaStore::bytes); | |||
There was a problem hiding this comment.
the reference to page meta store is captured here
There was a problem hiding this comment.
yes. and i tracked the call stack, the PageMetaStore will be put in a LocalCacheFileSystem, which is kept in the FileSystem.Factory.FILESYSTEM_CACHE as a singleton. so i'm not sure if it's necessary to release the reference. after all the singleton will live until the process exits.
| @@ -107,6 +108,7 @@ public long append(ByteBuf buf) throws IOException { | |||
| bytesWritten += bytesLeftInPage; | |||
| } | |||
|
|
|||
| MultiDimensionalMetricsSystem.UFS_DATA_ACCESS.labelValues("write").observe(bytesWritten); | |||
There was a problem hiding this comment.
This is not UFS_DATA_ACCESS since PagedFileWriter only writes into the worker's storage. Should be CACHED_DATA_WRITE.
There was a problem hiding this comment.
my bad. i tracked the ufs parameter and lost in the wrong place.
Kai-Zhang
force-pushed
the
new-metrics
branch
from
54c3a4d
to
084edd1
Compare
|
alluxio-bot, merge this please |
### What changes are proposed in this pull request? Add call points of the new metrics ### Why are the changes needed? The new metrics are already defined in the previous pr, and need to be called in this pr. ### Does this PR introduce any user facing changes? no pr-link: Alluxio#18351 change-id: cid-1b35a87f41a1f836005c368881378329f4c77b25
What changes are proposed in this pull request?
Add call points of the new metrics
Why are the changes needed?
The new metrics are already defined in the previous pr, and need to be called in this pr.
Does this PR introduce any user facing changes?
no