davidradl commented on code in PR #26253:
URL: https://github.com/apache/flink/pull/26253#discussion_r1981836587
##########
docs/layouts/shortcodes/generated/forst_configuration.html:
##########
@@ -30,7 +30,7 @@
<td><h5>state.backend.forst.cache.reserve-size</h5></td>
<td style="word-wrap: break-word;">256 mb</td>
<td>MemorySize</td>
- <td>The reserved size of cache, when set to a positive number.
Meaning that the cache will reserve the specified size of disk space. This
option and the 'state.backend.forst.cache.size-based-limit' option can be set
simultaneously, the smaller cache limit will be used as the upper limit. The
default value is '256 mb', meaning the disk will be reserved that much and
remaining of which can be used for cache.</td>
+ <td>The reserved size of cache, when set to a positive number.
Meaning that the cache will reserve the specified size of disk space. This
option and the 'state.backend.forst.cache.size-based-limit' option can be set
simultaneously, the smaller cache limit will be used as the upper limit. If the
specified file system of cache directory does not support reading the remaining
space, the cache will not be able to reserve the specified space. Thus this
option will be ignored. The default value is '256 mb', meaning the disk will be
reserved that much and remaining of which can be used for cache.</td>
Review Comment:
nits:
1) `when set to a positive number` does not read as a sentence. Does this
value have to be a positive number or does 0 means something? I would suggest
linking to the definition of Memory size in the docs - ideally there should be
an extra anchor so the link would point to the memorySize in page
[apache.org/flink/flink-docs-release-1.20/docs/deployment/config/
](apache.org/flink/flink-docs-release-1.20/docs/deployment/config/ )
2) the smaller cache limit will be used as the upper limit - is bit
confusing. `reserve-size` to me reads as a lower (reserved) boundary.
`size-based-limit` implies an upper limit.
3) `Thus this option will be ignored.` As there is no context in this
sentence, I would remove this sentence.
4) It seems weird that the smallest size wins - it would be more intuitive
to have defined precedence between these configuration fields.
5) `The default value is '256 mb', meaning the disk will be reserved that
much and remaining of which can be used for cache` Does not read well. This is
the reserved size of the cache - so how can the remaining be used for cache?
6) It is is difficult to understand the difference between this config
option and the size based cache option.
Maybe a picture would help?
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
