Document the hive.fs.cache.max-size property#11261
Conversation
posulliv
force-pushed
the
hive-fs-cache-docs
branch
from
8fb0dd3 to
d7d2dd4
Compare
posulliv
force-pushed
the
hive-fs-cache-docs
branch
from
d7d2dd4 to
1580c85
Compare
posulliv
force-pushed
the
hive-fs-cache-docs
branch
from
1580c85 to
d6ca2cd
Compare
| with a leading 0. If set to 'skip', permissions of newly | ||
| created directories will not be set by Trino. | ||
|
|
||
| ``hive.fs.cache.max-size`` Maximum number of cached file system objects. 1000 |
There was a problem hiding this comment.
technical question .. and potentially necessary link.. is this about Hive object storage caching or something else? We should link to the relevant docs..
There was a problem hiding this comment.
Good question. It is unrelated to hive object storage caching. It is the max size of the TrinoFileSystemCache which is an implementation of the Hadoop interface FileSystemCache.
This cache is used to speed up requests to get FileSystem objects.
I don't know any docs we could link to for explaining in more detail I'm afraid.
The reason I wanted to document this is because we hit this error sometimes on long running Trino clusters:
io.trino.spi.TrinoException: FileSystem max cache size has been reached: 1000
To increase this cache size limit and avoid hitting this error, you need to change the hive.fs.cache.max-size property.
There was a problem hiding this comment.
Thanks for the explanation. So it seems like there is another caching going on .. on the workers only I assume. Maybe @hashhar @findepi or others can chime in with any other relevant info we should add. From my perspective this is already a worth improvement and could be merged. but more info might be better..
There was a problem hiding this comment.
This is not caching of data. @electrum will know more.
There was a problem hiding this comment.
Thanks @findepi .. we definitely need to figure out more .. and if people hit this limit often .. maybe we need to raise the default value as well ..
There was a problem hiding this comment.
I've only hit this limit once. It's not common in my experience to hit this limit.
There was a problem hiding this comment.
since it's uncommon to hit this why document at all? People change values and then end up with broken setups. 🤔
There was a problem hiding this comment.
Because you want to at least enable the uncommon usage.. by your logic we should not have this as a modifiable setting at all @hashhar .. all our settings should have reasonable defaults and not need changing .. but we still need to enable the users that require changing a setting and document them. The alternative is that users are required to look things up in the log or the source code or just cant fix a problem.. thats much worse than having some documentation
There was a problem hiding this comment.
the default is "reasonable" since according to Padraig he's only ever needed to do it once.
I'm fine with documenting this but with the number of advanced toggles we have (either for testing or people who know what they are doing) documenting them all would just increase burden on both readers of the docs and the people keeping them up to date.
There was a problem hiding this comment.
Sure.. but thats what it takes .. documentation should be complete .. all alternatives are worse
mosabua
left a comment
mosabua
left a comment
There was a problem hiding this comment.
I honestly don't know if this is very helpful as it stands but it is definitely better than not having the property documented.
|
Can we get this merged? |
4 participants
Description
Documentation update to document a property.
No code changes.
Related issues, pull requests, and links
Documentation
( ) No documentation is needed.
(X ) 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: