Conversation
gz
left a comment
There was a problem hiding this comment.
maybe also needs a Ctrl+F for min_storage_rows in the web-console folder to adjust any reference to it if you havent dont that already
| /// This can be an approximation, such as the size of an on-disk file for a | ||
| /// stored batch. | ||
| /// | ||
| /// Implementations of this function can be expensive because they might |
There was a problem hiding this comment.
It's not obvious to me why it can't be computed and stored in the meta-data of the Vec* types when building the batch?
There was a problem hiding this comment.
It can be. I just don't see enough benefit. It complicates the merger implementations, which are already hard enough for me to understand (and to have any confidence in those incremental implementations I'd have to write careful tests), and it adds a cost to every builder and merger. it would only have a benefit in a case where we're about to visit every key-value pair anyway (a merge) and in that case we're throwing away the source batches anyway (because the merge consumes them). Also, we know that in-memory source batches are relatively small by the standard of the threshold, because otherwise they wouldn't be in memory.
If this turns out to be a mistake, then it'll show up in a profile. (On the other hand, the cost of incrementally maintaining the size won't ever show up in a profile because it'll be hidden inside larger code.)
A row can be any old size but a byte is well-defined. Signed-off-by: Ben Pfaff <blp@feldera.com>
A row can be any old size but a byte is well-defined.
See issue #1812.
Is this a user-visible change (yes/no): no