Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23760 )

Change subject: IMPALA-14612: Add global metrics for admission state map size
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/23760/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/23760/3//COMMIT_MSG@8
PS3, Line 8:
           : We need better observability for the admission state map to warn
           : about potential memory leaks.
           :
> Done. Updated the commit message.
Done


http://gerrit.cloudera.org:8080/#/c/23760/3//COMMIT_MSG@16
PS3, Line 16: the removal l
> We use FunctionGauge because it reads the map size directly. We don't need
If the metric page is refreshed hundreds of time per second, FunctionGauge will 
cause lock contention in the metric callback.
And it looks suspiciously complex to me that the callback function here 
requires obtaining metrics_lock_, tracking monotonic milis since 
ADMISSION_MAP_SIZE_METRIC_UPDATE_INTERVAL_MS, etc.

It sounds like updating AtomicHighWaterMarkGauge on each admission state map 
mutation is much better, complexity-wise. It is also can clearly show where the 
mutation sites are.

Are you avoiding any perf issue by creating new gauge?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie803aabf8d91b6381c5d0d7534cd9c9fc2166a73
Gerrit-Change-Number: 23760
Gerrit-PatchSet: 4
Gerrit-Owner: Yida Wu <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Yida Wu <[email protected]>
Gerrit-Comment-Date: Tue, 16 Dec 2025 00:16:29 +0000
Gerrit-HasComments: Yes

Reply via email to