Calculate ExternalSize Correctly for Views #608
Conversation
| @@ -609,7 +612,6 @@ make_header(State) -> | |||
| log_btree=LogBtree, | |||
| views=Views | |||
| } = State, | |||
|
|
|||
| end | ||
| end, | ||
| {ok, lists:foldl(SumFun, 0, Views)}. | ||
|
|
||
|
|
||
| sum_btree_sizes(nil, _) -> | ||
| sum_btree_sizes(null, _) -> |
There was a problem hiding this comment.
This nil/null change strikes me as possibly problematic for the non KV trees.
There was a problem hiding this comment.
@davisp: I changed reduced_external_size/1 to return 0 for cases when an ExternalSize is not returned. Previously, the value returned when there was no ExternalSize existed was null. So here I was accounting for the situation when it was null. Now that I changed the value 0, it seems the only possible values here are 0 or some other number. It can't be be nil or null? I think it's probably due to my lack of understanding of non KV trees.
|
|
||
| kv_external_size(KVList, Reduction) -> | ||
| lists:foldl(fun([[Key, _], Value], Acc) -> | ||
| size(term_to_binary(Key)) + size(term_to_binary(Value)) + Acc |
There was a problem hiding this comment.
Use ?term_size/1 instead of size(term_to_binary(Term)) for these.
| case couch_btree:full_reduce(Tree) of | ||
| {ok, {_, _, Size}} -> Size; | ||
| % return null for versions of the reduce function without Size | ||
| {ok, {_, _}} -> null |
There was a problem hiding this comment.
You're crossing streams a bit here. This returns null when there's no external size. But below your reduce function returns 0 when there's no external size. This means that with no write, we'll get null, but the first change will give us a number. We should pick one or the other.
|
hmmm, pre compaction and post compaction is slightly off by a few hundred bytes, but the compression method does not matter. need to figure out why |
|
looks like |
| @@ -1038,22 +1047,32 @@ get_user_reds(Reduction) -> | |||
| element(2, Reduction). | |||
|
|
|||
|
|
|||
| get_external_reds(Reduction) when tuple_size(Reduction) == 2 -> | |||
There was a problem hiding this comment.
Minor nit, call this get_external_size_reds. external_reds could be a bit misleading here.
tonysun83
force-pushed
the
3430-external-size-views
branch
from
19a827b
to
da5e714
Compare
Previously, we were calculating the ExternalSize for views by summing up all the nodes in the btree. Furthermore, this was the compressed size. Now we modify the reduce function to return an ExternalSize for uncompressed values in the KVList. PR: #608 COUCHDB-3430
tonysun83
force-pushed
the
3430-external-size-views
branch
from
da5e714
to
6d06dcf
Compare
Previously, we were calculating the ExternalSize for views by summing up all the nodes in the btree. Furthermore, this was the compressed size. Now we modify the reduce function to return an ExternalSize for uncompressed values in the KVList. PR: #608 COUCHDB-3430
Previously, we were calculating the ExternalSize for views by summing up all the nodes in the btree. Furthermore, this was the compressed size. Now we modify the reduce function to return an ExternalSize for uncompressed values in the KVList. PR: apache#608 COUCHDB-3430
* Description of Standalone vs Cluster Added a short description of Standalone and Clustermode which should make clearer what the purposes of the modes are.
Overview
Previously, we were calculating the ExternalSize for views by summing
up all the nodes in the btree. Furthermore, this was the compressed
size. Now we modify the reduce function to return an ExternalSize for
uncompressed values in the KVList.
Testing recommendations
Performance tests were run to compare External Size Info pre-fix and post-fix.
GitHub issue number
PR is number
Related Pull Requests
Checklist