-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Cache metastore (table and partition) stats by default #15811
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
cc @electrum |
b0a2b41 to
da55b27
Compare
findepi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Please make sure not to rebase wrt to current master when applying changes. No need for fixup commits.
plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastore.java
Outdated
Show resolved
Hide resolved
plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastore.java
Outdated
Show resolved
Hide resolved
...rino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastoreConfig.java
Outdated
Show resolved
Hide resolved
...rino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastoreConfig.java
Outdated
Show resolved
Hide resolved
.../trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/SharedHiveMetastoreCache.java
Outdated
Show resolved
Hide resolved
.../trino-hive/src/test/java/io/trino/plugin/hive/metastore/cache/TestCachingHiveMetastore.java
Outdated
Show resolved
Hide resolved
plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastore.java
Outdated
Show resolved
Hide resolved
...erg/src/main/java/io/trino/plugin/iceberg/catalog/hms/IcebergHiveMetastoreCatalogModule.java
Outdated
Show resolved
Hide resolved
da55b27 to
492fa07
Compare
492fa07 to
b643176
Compare
lib/trino-collect/src/main/java/io/trino/collect/cache/EmptyCache.java
Outdated
Show resolved
Hide resolved
lib/trino-collect/src/main/java/io/trino/collect/cache/EmptyCache.java
Outdated
Show resolved
Hide resolved
plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastore.java
Outdated
Show resolved
Hide resolved
...erg/src/main/java/io/trino/plugin/iceberg/catalog/hms/IcebergHiveMetastoreCatalogModule.java
Outdated
Show resolved
Hide resolved
b643176 to
00bfd86
Compare
findepi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
cdacf39 to
9173c1e
Compare
9173c1e to
71c952a
Compare
|
On top of #15864 |
71c952a to
b183975
Compare
Please ping me when the other one is merged. Thanks! |
Split caching into metadata and stats cache. Stats pulling puts significant pressure on metastore. However, stats don't have to be always up to date in order to get good query plance. Therefore, stats can be cached by default.
b183975 to
689e248
Compare
|
@findepi rebased |
plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastore.java
Show resolved
Hide resolved
| .hdfsEnvironment(hdfsEnvironment) | ||
| .build())) | ||
| .executor(executor) | ||
| .metadataCacheEnabled(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why using a different setup than the production default here?
add a code comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure. Cache was enabled here "forever"
Some deployments might had hive.metastore-cache-ttl already set. trinodb#15811 introduced new config hive.metastore-stats-cache-ttl which could be lower (by default) for such deployments. However, hive.metastore-cache-ttl should take precedense over hive.metastore-stats-cache-ttl as it's more generic (affects whole metastore cache).
Some deployments might had hive.metastore-cache-ttl already set. #15811 introduced new config hive.metastore-stats-cache-ttl which could be lower (by default) for such deployments. However, hive.metastore-cache-ttl should take precedense over hive.metastore-stats-cache-ttl as it's more generic (affects whole metastore cache).
RELEASE NOTES: