You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2020/03/04 21:35:34 UTC
[GitHub] [couchdb] davisp opened a new pull request #2636: Clean up view
size limit enforcement
davisp opened a new pull request #2636: Clean up view size limit enforcement
URL: https://github.com/apache/couchdb/pull/2636
## Overview
Clean up view size limit enforcement
Both a performance and style cleanup. This reduces the number of ets
calls by roughly `2 * 2 * $num_docs * $num_views`. The first factor of
two is because this avoids re-calculating the size twice for both the id
and map indexes. The second factor of two is because we have to lookup
two different settings.
Stylistically this also moves the logic out of the fdb modules. Keeping
key/value structure and update logic is already pretty complicated so we
should avoid mixing external application logic into those modules as
much as possible for our sanity.
The behavior is slightly changed vs the original patch as well.
Originally only the view that contained the errant key or value was
affected. However, pre-existing behavior where a document fails to be
mapped correctly resulted in it being excluded from all view indexes
defined in the design document. That behavior has been restored.
## Testing recommendations
make check-fdb
## Checklist
- [x] Code is written and works correctly
- [x] Changes are covered by tests
- [x] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
- [ ] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] davisp merged pull request #2636: Clean up view size
limit enforcement
Posted by GitBox <gi...@apache.org>.
davisp merged pull request #2636: Clean up view size limit enforcement
URL: https://github.com/apache/couchdb/pull/2636
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] iilyak commented on a change in pull request #2636: Clean
up view size limit enforcement
Posted by GitBox <gi...@apache.org>.
iilyak commented on a change in pull request #2636: Clean up view size limit enforcement
URL: https://github.com/apache/couchdb/pull/2636#discussion_r388213029
##########
File path: src/couch_views/src/couch_views_indexer.erl
##########
@@ -29,6 +29,8 @@
% TODO:
% * Handle timeouts of transaction and other errors
+-define(KEY_SIZE_LIMIT, 8000).
Review comment:
Maybe rename it to `DEFAULT_KEY_SIZE` and `DEFAULT_VALUE_SIZE_LIMIT`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] davisp commented on a change in pull request #2636: Clean
up view size limit enforcement
Posted by GitBox <gi...@apache.org>.
davisp commented on a change in pull request #2636: Clean up view size limit enforcement
URL: https://github.com/apache/couchdb/pull/2636#discussion_r388438691
##########
File path: src/couch_views/src/couch_views_indexer.erl
##########
@@ -297,8 +299,13 @@ write_docs(TxDb, Mrst, Docs, State) ->
last_seq := LastSeq
} = State,
- lists:foreach(fun(Doc) ->
- couch_views_fdb:write_doc(TxDb, Sig, Views, Doc)
+ ViewIds = [View#mrview.id_num || View <- Views],
+ KeyLimit = key_size_limit(),
+ ValLimit = value_size_limit(),
+
+ lists:foreach(fun(Doc0) ->
+ Doc1 = calculate_kv_sizes(Mrst, Doc0, KeyLimit, ValLimit),
+ couch_views_fdb:write_doc(TxDb, Sig, ViewIds, Doc1)
Review comment:
Started to try moving this but I didn't like how it moved extra logic into the fdb module. Given that the loop is the number of views on a design document the performance cost is insignificant so leaving the logic for pulling out the view id here seems better.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] garrensmith commented on a change in pull request #2636:
Clean up view size limit enforcement
Posted by GitBox <gi...@apache.org>.
garrensmith commented on a change in pull request #2636: Clean up view size limit enforcement
URL: https://github.com/apache/couchdb/pull/2636#discussion_r388337918
##########
File path: src/couch_views/src/couch_views_indexer.erl
##########
@@ -297,8 +299,13 @@ write_docs(TxDb, Mrst, Docs, State) ->
last_seq := LastSeq
} = State,
- lists:foreach(fun(Doc) ->
- couch_views_fdb:write_doc(TxDb, Sig, Views, Doc)
+ ViewIds = [View#mrview.id_num || View <- Views],
+ KeyLimit = key_size_limit(),
+ ValLimit = value_size_limit(),
+
+ lists:foreach(fun(Doc0) ->
+ Doc1 = calculate_kv_sizes(Mrst, Doc0, KeyLimit, ValLimit),
+ couch_views_fdb:write_doc(TxDb, Sig, ViewIds, Doc1)
Review comment:
I prefer that we pass in the Views here rather than the ViewId's. The extra loop to extract the ViewIds seems unnecessary
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] garrensmith commented on issue #2636: Clean up view size
limit enforcement
Posted by GitBox <gi...@apache.org>.
garrensmith commented on issue #2636: Clean up view size limit enforcement
URL: https://github.com/apache/couchdb/pull/2636#issuecomment-595268224
> +1. Not sure what's up with Jenkins, but `make check-fdb` passes locally.
I don't think Jenkins is configured yet to run FDB
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] davisp commented on a change in pull request #2636: Clean
up view size limit enforcement
Posted by GitBox <gi...@apache.org>.
davisp commented on a change in pull request #2636: Clean up view size limit enforcement
URL: https://github.com/apache/couchdb/pull/2636#discussion_r388437350
##########
File path: src/couch_views/src/couch_views_indexer.erl
##########
@@ -29,6 +29,8 @@
% TODO:
% * Handle timeouts of transaction and other errors
+-define(KEY_SIZE_LIMIT, 8000).
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services