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/02/25 12:24:35 UTC

[GitHub] [couchdb] garrensmith opened a new pull request #2598: Fdb set view limits

garrensmith opened a new pull request #2598: Fdb set view limits
URL: https://github.com/apache/couchdb/pull/2598
 
 
   # Overview
   
   Limits the size that a key and value that can be indexed in a map function.
   If the key or value exceeds the size the document is not indexed for the design document and an error is logged.
   
   ## Testing recommendations
   
   `make eunit app=couch_views`
   
   ## Related Issues or Pull Requests
   
   <!-- If your changes affects multiple components in different
        repositories please put links to those issues or pull requests here.  -->
   
   ## 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] nickva commented on a change in pull request #2598: Fdb set view limits

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2598: Fdb set view limits
URL: https://github.com/apache/couchdb/pull/2598#discussion_r383978149
 
 

 ##########
 File path: src/couch_views/test/couch_views_indexer_test.erl
 ##########
 @@ -388,6 +388,142 @@ multipe_identical_keys_from_same_doc(Db) ->
         ], Out).
 
 
+handle_size_key_limits(Db) ->
+    ok = meck:new(config, [passthrough]),
+    ok = meck:expect(config, get_integer, fun(Section, _, Default) ->
+        case Section of
+            "couch_views" -> 15;
+            _ -> Default
+        end
+    end),
+
+    DDoc = create_ddoc(multi_emit_key_limit),
+    Docs = [doc(1)] ++ [doc(2)],
+
+    {ok, _} = fabric2_db:update_docs(Db, [DDoc | Docs], []),
+
+    {ok, Out} = couch_views:query(
+        Db,
+        DDoc,
+        <<"map_fun1">>,
+        fun fold_fun/2,
+        [],
+        #mrargs{}
+    ),
+
+    ?assertEqual([
+        {row, [
+            {id, <<"1">>},
+            {key, 1},
+            {value, 1}
+        ]}
+    ], Out),
+
+    {ok, Doc} = fabric2_db:open_doc(Db, <<"2">>),
+    Doc2 = Doc#doc {
+        body = {[{<<"val">>,3}]}
+    },
+    {ok, _} = fabric2_db:update_doc(Db, Doc2),
+
+    {ok, Out1} = couch_views:query(
+        Db,
+        DDoc,
+        <<"map_fun1">>,
+        fun fold_fun/2,
+        [],
+        #mrargs{}
+    ),
+
+    ?assertEqual([
+        {row, [
+            {id, <<"1">>},
+            {key, 1},
+            {value, 1}
+        ]},
+        {row, [
+            {id, <<"2">>},
+            {key, 3},
+            {value, 3}
+        ]}
+    ], Out1).
+
+
+handle_size_value_limits(Db) ->
+    ok = meck:new(config, [passthrough]),
+    ok = meck:expect(config, get_integer, fun(Section, _, Default) ->
+        case Section of
+            "couch_views" -> 15;
 
 Review comment:
   Let's be more specific by using keys as well not just a section. We could add more settings in the future and then the test might break unexpectedly.

----------------------------------------------------------------
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] nickva commented on issue #2598: Fdb set view limits

Posted by GitBox <gi...@apache.org>.
nickva commented on issue #2598: Fdb set view limits
URL: https://github.com/apache/couchdb/pull/2598#issuecomment-590959775
 
 
   Referring to the last discussion https://github.com/apache/couchdb/pull/2460#pullrequestreview-345631546 wonder if we could do anything else besides log error `couch_log`. End users might not have access to parse the logs.
   
   There was some discussion on the ML about emitting error rows vs regular rows. Would that be viable? Or I was thinking of having a stats count for errors, say number of attempted but failed emits and a general reason for error `size_exceeded`. 
   

----------------------------------------------------------------
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] nickva commented on a change in pull request #2598: Fdb set view limits

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2598: Fdb set view limits
URL: https://github.com/apache/couchdb/pull/2598#discussion_r383936390
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -249,6 +249,10 @@ iterations = 10 ; iterations for password hashing
 ; Settings for view indexing
 [couch_views]
 ; max_workers = 100
+; The maximum allowed key size emitted from a view for a document (in bytes)
+; key_size_limit = 8000
+; The maximum allowed value size emitted from a view for a document (in bytes)
+; key_size_limit = 64000
 
 Review comment:
   `value_size_limit` I think?
   
   The defaults are maximum as well? We could mention what happens if they are exceeded. I would guess we clip the config value to that max value/

----------------------------------------------------------------
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 merged pull request #2598: Fdb set view limits

Posted by GitBox <gi...@apache.org>.
garrensmith merged pull request #2598: Fdb set view limits
URL: https://github.com/apache/couchdb/pull/2598
 
 
   

----------------------------------------------------------------
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] nickva commented on a change in pull request #2598: Fdb set view limits

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2598: Fdb set view limits
URL: https://github.com/apache/couchdb/pull/2598#discussion_r383963656
 
 

 ##########
 File path: src/couch_views/src/couch_views_fdb.erl
 ##########
 @@ -130,26 +128,50 @@ write_doc(TxDb, Sig, ViewIds, Doc) ->
     ExistingViewKeys = get_view_keys(TxDb, Sig, DocId),
 
     clear_id_idx(TxDb, Sig, DocId),
-
-    lists:foreach(fun({ViewId, NewRows}) ->
-        update_id_idx(TxDb, Sig, ViewId, DocId, NewRows),
-
-        ExistingKeys = case lists:keyfind(ViewId, 1, ExistingViewKeys) of
-            {ViewId, TotalRows, TotalSize, EKeys} ->
-                RowChange = length(NewRows) - TotalRows,
-                SizeChange = calculate_row_size(NewRows) - TotalSize,
-                update_row_count(TxDb, Sig, ViewId, RowChange),
-                update_kv_size(TxDb, Sig, ViewId, SizeChange),
-                EKeys;
-            false ->
-                RowChange = length(NewRows),
-                SizeChange = calculate_row_size(NewRows),
-                update_row_count(TxDb, Sig, ViewId, RowChange),
-                update_kv_size(TxDb, Sig, ViewId, SizeChange),
-                []
-        end,
-        update_map_idx(TxDb, Sig, ViewId, DocId, ExistingKeys, NewRows)
-    end, lists:zip(ViewIds, Results)).
+    lists:foreach(fun({View, NewRows}) ->
+        #mrview{
+            map_names = MNames,
+            id_num = ViewId
+        } = View,
+
+        try
+            NewRowSize = calculate_row_size(NewRows),
+            update_id_idx(TxDb, Sig, ViewId, DocId, NewRows),
+
+            ExistingKeys = case lists:keyfind(ViewId, 1, ExistingViewKeys) of
+                {ViewId, TotalRows, TotalSize, EKeys} ->
+                    RowChange = length(NewRows) - TotalRows,
+                    SizeChange = NewRowSize - TotalSize,
+                    update_row_count(TxDb, Sig, ViewId, RowChange),
+                    update_kv_size(TxDb, Sig, ViewId, SizeChange),
+                    EKeys;
+                false ->
+                    RowChange = length(NewRows),
+                    SizeChange = NewRowSize,
+                    update_row_count(TxDb, Sig, ViewId, RowChange),
+                    update_kv_size(TxDb, Sig, ViewId, SizeChange),
+                    []
+            end,
+            update_map_idx(TxDb, Sig, ViewId, DocId, ExistingKeys, NewRows)
+        catch
+            throw:{size_exceeded, Type}  ->
+                case lists:keyfind(ViewId, 1, ExistingViewKeys) of
+                    false ->
+                        ok;
+                    ExistingViewKey ->
+                        remove_doc_from_idx(TxDb, Sig, DocId, ExistingViewKey)
+                end,
+                couch_log:error("Doc `~s` exceeded the ~s size for view `~s`"
+                    " and was not indexed.", [DocId, Type, MNames])
 
 Review comment:
   Could we add a db name as well?

----------------------------------------------------------------
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] nickva commented on a change in pull request #2598: Fdb set view limits

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2598: Fdb set view limits
URL: https://github.com/apache/couchdb/pull/2598#discussion_r383936390
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -249,6 +249,10 @@ iterations = 10 ; iterations for password hashing
 ; Settings for view indexing
 [couch_views]
 ; max_workers = 100
+; The maximum allowed key size emitted from a view for a document (in bytes)
+; key_size_limit = 8000
+; The maximum allowed value size emitted from a view for a document (in bytes)
+; key_size_limit = 64000
 
 Review comment:
   `value_size_limit` I think?
   
   The default are maximum as well? We could mention what happens if they are exceeded. I would guess we clip the config value to that max value/

----------------------------------------------------------------
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] nickva commented on a change in pull request #2598: Fdb set view limits

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2598: Fdb set view limits
URL: https://github.com/apache/couchdb/pull/2598#discussion_r383979743
 
 

 ##########
 File path: src/couch_views/test/couch_views_indexer_test.erl
 ##########
 @@ -388,6 +388,142 @@ multipe_identical_keys_from_same_doc(Db) ->
         ], Out).
 
 
+handle_size_key_limits(Db) ->
+    ok = meck:new(config, [passthrough]),
+    ok = meck:expect(config, get_integer, fun(Section, _, Default) ->
+        case Section of
+            "couch_views" -> 15;
+            _ -> Default
+        end
+    end),
+
+    DDoc = create_ddoc(multi_emit_key_limit),
+    Docs = [doc(1)] ++ [doc(2)],
+
+    {ok, _} = fabric2_db:update_docs(Db, [DDoc | Docs], []),
+
+    {ok, Out} = couch_views:query(
+        Db,
+        DDoc,
+        <<"map_fun1">>,
+        fun fold_fun/2,
+        [],
+        #mrargs{}
+    ),
+
+    ?assertEqual([
+        {row, [
 
 Review comment:
   If you want, to avoid repeating 5 lines for each row, could create a `row(Id, K, V)` function so then it would look like:
   
   ```?assertEqual(row(<<"1">>, 1, 1), Out)```

----------------------------------------------------------------
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] nickva commented on a change in pull request #2598: Fdb set view limits

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2598: Fdb set view limits
URL: https://github.com/apache/couchdb/pull/2598#discussion_r384643400
 
 

 ##########
 File path: src/couch_views/src/couch_views_fdb.erl
 ##########
 @@ -352,6 +378,28 @@ process_rows(Rows) ->
 
 
 calculate_row_size(Rows) ->
+    KeyLimit = key_size_limit(),
+    ValLimit = value_size_limit(),
+
     lists:foldl(fun({K, V}, Acc) ->
-        Acc + erlang:external_size(K) + erlang:external_size(V)
+        KeySize = erlang:external_size(K),
+        ValSize = erlang:external_size(V),
+
+        if KeySize =< KeyLimit -> ok; true ->
+            throw({size_exceeded, key})
+        end,
+
+        if ValSize =< ValLimit -> ok; true ->
+            throw({size_exceeded, value})
+        end,
+
+        Acc + KeySize + ValSize
     end, 0, Rows).
+
+
+key_size_limit() ->
+    config:get_integer("couch_views", "key_size_limit", 8000).
 
 Review comment:
   Let's define 8000 and 64000 as constants at the top.
   
   Also what do you think about actually clipping the user provided config to those maximums? 
   
   `max(config:get_integer(...), ?MAX_KEY_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] nickva commented on a change in pull request #2598: Fdb set view limits

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2598: Fdb set view limits
URL: https://github.com/apache/couchdb/pull/2598#discussion_r383945513
 
 

 ##########
 File path: src/couch_views/test/couch_views_indexer_test.erl
 ##########
 @@ -32,17 +29,19 @@ indexer_test_() ->
                 foreach,
                 fun foreach_setup/0,
                 fun foreach_teardown/1,
-                ?I_HEART_EUNIT([
-                    fun indexed_empty_db/1,
-                    fun indexed_single_doc/1,
-                    fun updated_docs_are_reindexed/1,
-                    fun updated_docs_without_changes_are_reindexed/1,
-                    fun deleted_docs_not_indexed/1,
-                    fun deleted_docs_are_unindexed/1,
-                    fun multipe_docs_with_same_key/1,
-                    fun multipe_keys_from_same_doc/1,
-                    fun multipe_identical_keys_from_same_doc/1
-                ])
+                [
+                    with([?TDEF(indexed_empty_db)]),
 
 Review comment:
   Can use the new `?TDEF_FE` macro from https://github.com/apache/couchdb/blob/prototype/fdb-layer/src/fabric/test/fabric2_changes_fold_tests.erl#L40 to avoid repeating the `with` line

----------------------------------------------------------------
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 #2598: Fdb set view limits

Posted by GitBox <gi...@apache.org>.
garrensmith commented on issue #2598: Fdb set view limits
URL: https://github.com/apache/couchdb/pull/2598#issuecomment-591291514
 
 
   Thanks for the review @nickva I've made all the changes
   
   In terms of logging the error. I've going with this approach for a two main reasons:
   
   1. It is the same as what views does already for when an error occurs while indexing a document in couch_js
   1. It should be straightforward for a user that is monitoring their logs to create some kind of event that is triggered when they get the size errors. 

----------------------------------------------------------------
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] nickva commented on a change in pull request #2598: Fdb set view limits

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2598: Fdb set view limits
URL: https://github.com/apache/couchdb/pull/2598#discussion_r384643883
 
 

 ##########
 File path: src/couch_views/test/couch_views_indexer_test.erl
 ##########
 @@ -388,6 +388,142 @@ multipe_identical_keys_from_same_doc(Db) ->
         ], Out).
 
 
+handle_size_key_limits(Db) ->
+    ok = meck:new(config, [passthrough]),
+    ok = meck:expect(config, get_integer, fun(Section, _, Default) ->
+        case Section of
+            "couch_views" -> 15;
+            _ -> Default
+        end
+    end),
+
+    DDoc = create_ddoc(multi_emit_key_limit),
+    Docs = [doc(1)] ++ [doc(2)],
+
+    {ok, _} = fabric2_db:update_docs(Db, [DDoc | Docs], []),
+
+    {ok, Out} = couch_views:query(
+        Db,
+        DDoc,
+        <<"map_fun1">>,
+        fun fold_fun/2,
+        [],
+        #mrargs{}
+    ),
+
+    ?assertEqual([
+        {row, [
 
 Review comment:
   Looks good

----------------------------------------------------------------
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] nickva commented on a change in pull request #2598: Fdb set view limits

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2598: Fdb set view limits
URL: https://github.com/apache/couchdb/pull/2598#discussion_r384638603
 
 

 ##########
 File path: src/couch_views/src/couch_views_fdb.erl
 ##########
 @@ -130,26 +128,54 @@ write_doc(TxDb, Sig, ViewIds, Doc) ->
     ExistingViewKeys = get_view_keys(TxDb, Sig, DocId),
 
     clear_id_idx(TxDb, Sig, DocId),
-
-    lists:foreach(fun({ViewId, NewRows}) ->
-        update_id_idx(TxDb, Sig, ViewId, DocId, NewRows),
-
-        ExistingKeys = case lists:keyfind(ViewId, 1, ExistingViewKeys) of
-            {ViewId, TotalRows, TotalSize, EKeys} ->
-                RowChange = length(NewRows) - TotalRows,
-                SizeChange = calculate_row_size(NewRows) - TotalSize,
-                update_row_count(TxDb, Sig, ViewId, RowChange),
-                update_kv_size(TxDb, Sig, ViewId, SizeChange),
-                EKeys;
-            false ->
-                RowChange = length(NewRows),
-                SizeChange = calculate_row_size(NewRows),
-                update_row_count(TxDb, Sig, ViewId, RowChange),
-                update_kv_size(TxDb, Sig, ViewId, SizeChange),
-                []
-        end,
-        update_map_idx(TxDb, Sig, ViewId, DocId, ExistingKeys, NewRows)
-    end, lists:zip(ViewIds, Results)).
+    lists:foreach(fun({View, NewRows}) ->
+        #mrview{
+            map_names = MNames,
+            id_num = ViewId
+        } = View,
+
+        try
+            NewRowSize = calculate_row_size(NewRows),
+            update_id_idx(TxDb, Sig, ViewId, DocId, NewRows),
+
+            ExistingKeys = case lists:keyfind(ViewId, 1, ExistingViewKeys) of
+                {ViewId, TotalRows, TotalSize, EKeys} ->
+                    RowChange = length(NewRows) - TotalRows,
+                    SizeChange = NewRowSize - TotalSize,
+                    update_row_count(TxDb, Sig, ViewId, RowChange),
+                    update_kv_size(TxDb, Sig, ViewId, SizeChange),
+                    EKeys;
+                false ->
+                    RowChange = length(NewRows),
+                    SizeChange = NewRowSize,
+                    update_row_count(TxDb, Sig, ViewId, RowChange),
+                    update_kv_size(TxDb, Sig, ViewId, SizeChange),
+                    []
+            end,
+            update_map_idx(TxDb, Sig, ViewId, DocId, ExistingKeys, NewRows)
+        catch
+            throw:{size_exceeded, Type}  ->
+                case lists:keyfind(ViewId, 1, ExistingViewKeys) of
+                    false ->
+                        ok;
+                    ExistingViewKey ->
+                        remove_doc_from_idx(TxDb, Sig, DocId, ExistingViewKey)
+                end,
+                #{
+                    name := DbName
+                } = TxDb,
+                couch_log:error("Db `~s` Doc `~s` exceeded the ~s size"
+                    "for view `~s` and was not indexed.",
 
 Review comment:
   Let's add space in front of `for` otherwise it looks like
   
   ```
   `Db `eunit-test-db-ace465025c62c6b0ac918fee38fd1963` Doc `2` exceeded the key sizefor view `map_fun1` and was not indexed.
   ```

----------------------------------------------------------------
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] nickva commented on a change in pull request #2598: Fdb set view limits

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2598: Fdb set view limits
URL: https://github.com/apache/couchdb/pull/2598#discussion_r384643724
 
 

 ##########
 File path: src/couch_views/src/couch_views_fdb.erl
 ##########
 @@ -130,26 +128,50 @@ write_doc(TxDb, Sig, ViewIds, Doc) ->
     ExistingViewKeys = get_view_keys(TxDb, Sig, DocId),
 
     clear_id_idx(TxDb, Sig, DocId),
-
-    lists:foreach(fun({ViewId, NewRows}) ->
-        update_id_idx(TxDb, Sig, ViewId, DocId, NewRows),
-
-        ExistingKeys = case lists:keyfind(ViewId, 1, ExistingViewKeys) of
-            {ViewId, TotalRows, TotalSize, EKeys} ->
-                RowChange = length(NewRows) - TotalRows,
-                SizeChange = calculate_row_size(NewRows) - TotalSize,
-                update_row_count(TxDb, Sig, ViewId, RowChange),
-                update_kv_size(TxDb, Sig, ViewId, SizeChange),
-                EKeys;
-            false ->
-                RowChange = length(NewRows),
-                SizeChange = calculate_row_size(NewRows),
-                update_row_count(TxDb, Sig, ViewId, RowChange),
-                update_kv_size(TxDb, Sig, ViewId, SizeChange),
-                []
-        end,
-        update_map_idx(TxDb, Sig, ViewId, DocId, ExistingKeys, NewRows)
-    end, lists:zip(ViewIds, Results)).
+    lists:foreach(fun({View, NewRows}) ->
+        #mrview{
+            map_names = MNames,
+            id_num = ViewId
+        } = View,
+
+        try
+            NewRowSize = calculate_row_size(NewRows),
+            update_id_idx(TxDb, Sig, ViewId, DocId, NewRows),
+
+            ExistingKeys = case lists:keyfind(ViewId, 1, ExistingViewKeys) of
+                {ViewId, TotalRows, TotalSize, EKeys} ->
+                    RowChange = length(NewRows) - TotalRows,
+                    SizeChange = NewRowSize - TotalSize,
+                    update_row_count(TxDb, Sig, ViewId, RowChange),
+                    update_kv_size(TxDb, Sig, ViewId, SizeChange),
+                    EKeys;
+                false ->
+                    RowChange = length(NewRows),
+                    SizeChange = NewRowSize,
+                    update_row_count(TxDb, Sig, ViewId, RowChange),
+                    update_kv_size(TxDb, Sig, ViewId, SizeChange),
+                    []
+            end,
+            update_map_idx(TxDb, Sig, ViewId, DocId, ExistingKeys, NewRows)
+        catch
+            throw:{size_exceeded, Type}  ->
+                case lists:keyfind(ViewId, 1, ExistingViewKeys) of
+                    false ->
+                        ok;
+                    ExistingViewKey ->
+                        remove_doc_from_idx(TxDb, Sig, DocId, ExistingViewKey)
+                end,
+                couch_log:error("Doc `~s` exceeded the ~s size for view `~s`"
+                    " and was not indexed.", [DocId, Type, MNames])
 
 Review comment:
   Works!
   
   ```
   Db `eunit-test-db-f486ff70c687455a009b04e44125e404` Doc `1` exceeded the value sizefor view `map_fun2` and was not indexed.
   ```

----------------------------------------------------------------
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] nickva commented on a change in pull request #2598: Fdb set view limits

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2598: Fdb set view limits
URL: https://github.com/apache/couchdb/pull/2598#discussion_r383979743
 
 

 ##########
 File path: src/couch_views/test/couch_views_indexer_test.erl
 ##########
 @@ -388,6 +388,142 @@ multipe_identical_keys_from_same_doc(Db) ->
         ], Out).
 
 
+handle_size_key_limits(Db) ->
+    ok = meck:new(config, [passthrough]),
+    ok = meck:expect(config, get_integer, fun(Section, _, Default) ->
+        case Section of
+            "couch_views" -> 15;
+            _ -> Default
+        end
+    end),
+
+    DDoc = create_ddoc(multi_emit_key_limit),
+    Docs = [doc(1)] ++ [doc(2)],
+
+    {ok, _} = fabric2_db:update_docs(Db, [DDoc | Docs], []),
+
+    {ok, Out} = couch_views:query(
+        Db,
+        DDoc,
+        <<"map_fun1">>,
+        fun fold_fun/2,
+        [],
+        #mrargs{}
+    ),
+
+    ?assertEqual([
+        {row, [
 
 Review comment:
   If you want, to avoid repeating 4 lines for each row, could create a `row(Id, K, V)` function so then it would look like:
   
   ```?assertEqual(row(<<"1">>, 1, 1), Out)```

----------------------------------------------------------------
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