Skip to content

Expose JMX metrics for TrinoFileSystemCache#12024

Merged
findepi merged 1 commit intotrinodb:masterfrom
posulliv:trino-fs-cache-jmx-metrics
Jun 8, 2022
Merged

Expose JMX metrics for TrinoFileSystemCache#12024
findepi merged 1 commit intotrinodb:masterfrom
posulliv:trino-fs-cache-jmx-metrics

Conversation

@posulliv
Copy link
Copy Markdown
Contributor

Description

The Hive connector in Trino has it's own implementation of Hadoop's FileSystemCache. The size of this cache defaults to 1000 but there is no way to see what the current size of the cache is.

In deployments where HDFS impersonation is enabled with many distinct users connecting to Trino, this cache can get large.

This PR exposes some statistics on the cache via JMX to help Trino admins know if the cache is getting close to capacity.

Is this change a fix, improvement, new feature, refactoring, or other?

New JMX statistics for TrinoFileSystemCache.

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

Update to the Hive connector.

How would you describe this change to a non-technical end user or system administrator?

This change exposes new JMX stats which will be available at trino.plugin.hive.fs:name=<catalog_name>,type=trinofilesystemcache.

Related issues, pull requests, and links

Documentation

(X ) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( X) No release notes entries required.
( ) Release notes entries required with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@posulliv
Copy link
Copy Markdown
Contributor Author

posulliv commented May 3, 2022

@hashhar sorry to bug you but do you know who a good person to ping for a review of this might be?

@hashhar
Copy link
Copy Markdown
Member

hashhar commented May 10, 2022

@findinpath @homar can you PTAL (or find someone eligible). Piotr can do the final review before merge.

@findinpath findinpath self-requested a review May 10, 2022 06:50
Copy link
Copy Markdown
Member

@homar homar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me but @findepi needs to take a look

@findepi
Copy link
Copy Markdown
Member

findepi commented May 13, 2022

@phd3 @Felicity-3786 PTAL (re #12364)

@findinpath
Copy link
Copy Markdown
Contributor

Do we want to enable clearing the cache via JMX ?
I can imagine this may be interesting for debugging purposes for exotic behaviors.

@findepi
Copy link
Copy Markdown
Member

findepi commented May 13, 2022

Do we want to enable clearing the cache via JMX ?

i don't know the implications of doing that, so wouldn't approve such a change.

@findepi
Copy link
Copy Markdown
Member

findepi commented May 27, 2022

@posulliv there are some unapplied comments. Do you want to update the PR?

@posulliv posulliv force-pushed the trino-fs-cache-jmx-metrics branch from bbab13a to ed69cd8 Compare May 31, 2022 14:52
@posulliv
Copy link
Copy Markdown
Contributor Author

posulliv commented May 31, 2022

@findepi I have applied all feedback now.

@posulliv posulliv force-pushed the trino-fs-cache-jmx-metrics branch from ed69cd8 to 462a89d Compare May 31, 2022 17:19
@posulliv posulliv requested review from findepi and findinpath May 31, 2022 17:21
@posulliv posulliv force-pushed the trino-fs-cache-jmx-metrics branch from 462a89d to 72bdb9a Compare June 3, 2022 15:51
@findepi
Copy link
Copy Markdown
Member

findepi commented Jun 3, 2022

@posulliv please rebase, the CI didn't run

@posulliv posulliv force-pushed the trino-fs-cache-jmx-metrics branch from 72bdb9a to 809b073 Compare June 6, 2022 14:22
@posulliv posulliv force-pushed the trino-fs-cache-jmx-metrics branch from 809b073 to aa0a593 Compare June 7, 2022 00:06
@findepi findepi force-pushed the trino-fs-cache-jmx-metrics branch from aa0a593 to a0092c1 Compare June 7, 2022 15:48
@findepi findepi added the no-release-notes This pull request does not require release notes entry label Jun 7, 2022
@findepi findepi merged commit fa0de52 into trinodb:master Jun 8, 2022
@github-actions github-actions bot added this to the 385 milestone Jun 8, 2022
@posulliv posulliv deleted the trino-fs-cache-jmx-metrics branch June 8, 2022 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed no-release-notes This pull request does not require release notes entry

Development

Successfully merging this pull request may close these issues.

6 participants