kbendick edited a comment on pull request #3543:
URL: https://github.com/apache/iceberg/pull/3543#issuecomment-985818457


   > Just to resonate with what Russell said, I have also heard a few customers 
got confused about the caching behaviors, and not enabling caching directly 
would likely be a more straight-forward solution for users, compared to have a 
30 seconds expiration that we have to then explain potential behaviors of query 
based on this expiration time.
   > 
   > Overall I think it's worth adding the feature, Trino and Presto also has 
something similar like `hive.metastore-refresh-interval`. I would personally 
just do not set a default for that config, given it is an optional config 
anyway.
   > 
   > Given the fact that the PR is also getting quite messy with all the 
changes, I would say let's first add the `cache-enabled` as a catalog property 
with `true` as default, which is the default behavior today. Then we can add 
time-based expiration and review it separately in more details.
   
   So you think I should open a new PR that just has a flag for 
`expiration-enabled`, @jackye1995? Or can we keep the current 
`cache.expiration-interval-ms` but turn it off by default (set to -1 or zero)?
   
   I would be in favor of adding this (or a subset of it), but keeping it 
turned off by default for now. As it will need to be added to the other 
catalogs. This would also give people time to test it out.
   
   There is a `cache-enabled` flag already fwiw.
   
   It's also worth noting that Spark has added a similar feature flag, 
`spark.sql.metadataCacheTTLSeconds`, where it expires table relations from the 
session catalog's cache after write. This was added in Spark 3.1: 
https://github.com/apache/spark/blob/38115cb907ec93151382260cda327330e78ca340/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala#L163-L172
   
   It is admittedly disabled by default though (but that's probably more for 
backwards compatibility purposes): 
https://github.com/apache/spark/blob/fba61ad68bb12b22055d5d475e95e2681685eed7/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/StaticSQLConf.scala#L242-L252


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to