Respect hive.metastore-stats-cache-ttl is explicitly set #19862
Conversation
findepi
added
the
no-release-notes
Remove the logic that would ignore `hive.metastore-stats-cache-ttl` set by the user, if the chosen value was less than the `hive.metastore-cache-ttl` value. The logic's purpose was considered necessary to enable statistics cache by default, but the same goal can be achieved differently, by applying default value (fixed duration or `hive.metastore-cache-ttl` whichever is greater) only if the parameter was not set explicitly.
findepi
force-pushed
the
findepi/respect-hive-metastore-stats-cache-ttl-is-explicitly-set-05b7fe
branch
from
58f41d3
to
ca83b6a
Compare
| .setStatsCacheTtl(new Duration(135, MILLISECONDS)) | ||
| .setMetastoreCacheTtl(new Duration(1111, DAYS)) |
There was a problem hiding this comment.
Can we disallow this configuration ? I can't think of a good reason why someone would want statistics metadata ttl to be shorter than other metadata ttl.
There was a problem hiding this comment.
Yes we can. Neither I can think of a good reason why someone would want that. OTOH, these are separate configuration toggles, so unclear what we gain by disallowing certain configurations which just work otherwise.
There was a problem hiding this comment.
Disallowing it prevents users from making a mistake in the configuration and saves debugging time for us.
There was a problem hiding this comment.
There are two toggles
- A
hive.metastore-cache-ttl - B
hive.metastore-stats-cache-ttl
if B < A, we can call it a mistake and reject. Do we have guarantee this is a mistake?
Or we risk that we gonna need to spend time explaining why we thought this is a good idea to reject such configurations?
BTW I think this discussion isn't very important. The whole problem comes from the fact that separate hive.metastore-stats-cache-ttl toggle was introduced so that it can be enabled by default. This PR doesn't change anything with respect to this main issue -- being enabled by default; default behavior doesn't change. So I guess we're spending time about some edge case.
There was a problem hiding this comment.
discussed offline and concluded this will be as it is for now at least
|
Thanks @raunaqmorarka @sopel39 for your detailed review! |
findepi
deleted the
findepi/respect-hive-metastore-stats-cache-ttl-is-explicitly-set-05b7fe
branch
Remove the logic that would ignore
hive.metastore-stats-cache-ttlset by the user, if the chosen value was less than thehive.metastore-cache-ttlvalue.The logic's purpose was considered necessary to enable statistics cache by default, but the same goal can be achieved differently, by applying default value (fixed duration or
hive.metastore-cache-ttlwhichever is greater) only if the parameter was not set explicitly.