Fix query limit

Previously, we set a default limit that was an effective infinity (2^28). It
seems back in the 32 bit days that was the Erlang's largest small integer [1].
However, that turned out to be too low and it surprised a user when it
truncated their all_docs output skipping some of the data. Fix that by
increasing the limit to a larger "infinity" (highest 64 bit Erlang small
integer [1]).

We did have a "query_limit" config parameter to customize the limit, however
that turned out to be broken and did not take effect when the user tried it for
all_docs, so fix that as well. Fix that and use a test to ensure the limit gets
reduced appropriately. To make the setting more user friendly, allow `infinity`
as the value.

Also, in the case of all_docs, we validated args and applied the limit check
twice: once in the coordinator and another time on each worker, which wasted
CPU resources and made things a bit confusing. To fix that, remove the
validation from the common worker code in couch_mrview and validate once:
either on the coordinator side, or local (port 5986) callback, right in the
http callback.

[1] https://www.erlang.org/doc/system/memory.html

Fix #5176

Author: nickva

rel/overlay/etc/default.ini

@@ -512,8 +512,8 @@ authentication_db = _users
 ; Timeout for how long a response from a busy view group server can take.
 ; "infinity" is also a valid configuration value.
 ;group_info_timeout = 5000
-;query_limit = 268435456
-;partition_query_limit = 268435456
+;query_limit = infinity
+;partition_query_limit = infinity
 
 ; Configure what to use as the db tag when selecting design doc couchjs
 ; processes. The choices are:

src/chttpd/src/chttpd_show.erl

@@ -264,7 +264,8 @@ handle_view_list(Req, Db, DDoc, LName, {ViewDesignName, ViewName}, Keys) ->
         },
         case ViewName of
             <<"_all_docs">> ->
-                fabric:all_docs(Db, Options, CB, Acc, QueryArgs);
+                QueryArgs1 = fabric_util:validate_all_docs_args(Db, QueryArgs),
+                fabric:all_docs(Db, Options, CB, Acc, QueryArgs1);
             _ ->
                 fabric:query_view(
                     Db,

src/chttpd/test/eunit/chttpd_view_test.erl

@@ -48,7 +48,8 @@
 
 setup() ->
     Hashed = couch_passwords:hash_admin_password(?PASS),
-    ok = config:set("admins", ?USER, ?b2l(Hashed), _Persist = false),
+    ok = config:set("admins", ?USER, ?b2l(Hashed), false),
+    ok = config:set("query_server_config", "query_limit", "100", false),
     Db = ?tempdb(),
     ok = create_db(Db),
     ok = create_docs(Db),
@@ -57,7 +58,8 @@ setup() ->
 
 teardown(Db) ->
     ok = fabric:delete_db(Db),
-    ok = config:delete("admins", ?USER, _Persist = false).
+    ok = config:delete("admins", ?USER, false),
+    ok = config:delete("query_server_config", "query_limit", false).
 
 view_test_() ->
     {
@@ -111,7 +113,11 @@ all_docs_test_() ->
                     ?TDEF_FE(t_all_docs_with_key_non_existent_docs),
                     ?TDEF_FE(t_all_docs_with_keys_non_existent_docs),
                     ?TDEF_FE(t_all_docs_with_key_deleted_docs),
-                    ?TDEF_FE(t_all_docs_with_keys_deleted_docs)
+                    ?TDEF_FE(t_all_docs_with_keys_deleted_docs),
+                    ?TDEF_FE(t_all_docs_with_limit),
+                    ?TDEF_FE(t_all_docs_with_skip),
+                    ?TDEF_FE(t_all_docs_with_limit_and_skip),
+                    ?TDEF_FE(t_all_docs_with_configured_query_limit)
                 ]
             }
         }
@@ -370,6 +376,66 @@ t_view_map_reduce_with_keys_and_group(Db) ->
     ?assertEqual(Res, Res1),
     ?assertEqual(Code, Code1).
 
+t_all_docs_with_limit(Db) ->
+    {Code, Res} = req(get, url(Db, "_all_docs", "limit=2")),
+    ?assertEqual(200, Code),
+    ?assertMatch(
+        #{
+            <<"offset">> := 0,
+            <<"total_rows">> := 4,
+            <<"rows">> := [
+                #{<<"id">> := <<"_design/ddoc">>},
+                #{<<"id">> := <<"a">>}
+            ]
+        },
+        Res
+    ).
+
+t_all_docs_with_skip(Db) ->
+    {Code, Res} = req(get, url(Db, "_all_docs", "skip=2")),
+    ?assertEqual(200, Code),
+    ?assertMatch(
+        #{
+            <<"offset">> := 2,
+            <<"total_rows">> := 4,
+            <<"rows">> := [
+                #{<<"id">> := <<"b">>},
+                #{<<"id">> := <<"c">>}
+            ]
+        },
+        Res
+    ).
+
+t_all_docs_with_limit_and_skip(Db) ->
+    {Code, Res} = req(get, url(Db, "_all_docs", "skip=2&limit=1")),
+    ?assertEqual(200, Code),
+    ?assertMatch(
+        #{
+            <<"offset">> := 2,
+            <<"total_rows">> := 4,
+            <<"rows">> := [
+                #{<<"id">> := <<"b">>}
+            ]
+        },
+        Res
+    ).
+
+t_all_docs_with_configured_query_limit(Db) ->
+    config:set("query_server_config", "query_limit", "2", false),
+    {Code, Res} = req(get, url(Db, "_all_docs")),
+    ?assertEqual(200, Code),
+    ?assertMatch(
+        #{
+            <<"offset">> := 0,
+            <<"total_rows">> := 4,
+            <<"rows">> := [
+                #{<<"id">> := <<"_design/ddoc">>},
+                #{<<"id">> := <<"a">>}
+            ]
+        },
+        Res
+    ).
+
 %%%%%%%%%%%%%%%%%%%% Utility Functions %%%%%%%%%%%%%%%%%%%%
 url(Db) ->
     Addr = config:get("chttpd", "bind_address", "127.0.0.1"),

src/couch_mrview/include/couch_mrview.hrl

@@ -54,7 +54,8 @@
     view_states=nil
 }).
 
--define(MAX_VIEW_LIMIT, 16#10000000).
+% Largest 64 bit small int: 2^59-1
+-define(MAX_VIEW_LIMIT, 576460752303423487).
 
 -record(mrargs, {
     view_type,

src/couch_mrview/src/couch_mrview.erl

@@ -279,20 +279,20 @@ query_all_docs(Db, Args) ->
     query_all_docs(Db, Args, fun default_cb/2, []).
 
 query_all_docs(Db, Args, Callback, Acc) when is_list(Args) ->
-    query_all_docs(Db, to_mrargs(Args), Callback, Acc);
+    Args1 = couch_mrview_util:validate_all_docs_args(Db, to_mrargs(Args)),
+    query_all_docs(Db, Args1, Callback, Acc);
 query_all_docs(Db, Args0, Callback, Acc) ->
     Sig = couch_util:with_db(Db, fun(WDb) ->
         {ok, Info} = couch_db:get_db_info(WDb),
         couch_index_util:hexsig(couch_hash:md5_hash(?term_to_bin(Info)))
     end),
     Args1 = Args0#mrargs{view_type = map},
-    Args2 = couch_mrview_util:validate_all_docs_args(Db, Args1),
     {ok, Acc1} =
-        case Args2#mrargs.preflight_fun of
+        case Args1#mrargs.preflight_fun of
             PFFun when is_function(PFFun, 2) -> PFFun(Sig, Acc);
             _ -> {ok, Acc}
         end,
-    all_docs_fold(Db, Args2, Callback, Acc1).
+    all_docs_fold(Db, Args1, Callback, Acc1).
 
 query_view(Db, DDoc, VName) ->
     Args = #mrargs{extra = [{view_row_map, true}]},

src/couch_mrview/src/couch_mrview_http.erl

@@ -229,7 +229,7 @@ do_all_docs_req(Req, Db, Keys, NS) ->
     ETagFun = fun(Sig, Acc0) ->
         check_view_etag(Sig, Acc0, Req)
     end,
-    Args = Args1#mrargs{preflight_fun = ETagFun},
+    Args2 = Args1#mrargs{preflight_fun = ETagFun},
     {ok, Resp} = couch_httpd:etag_maybe(Req, fun() ->
         Max = chttpd:chunked_response_buffer_size(),
         VAcc0 = #vacc{db = Db, req = Req, threshold = Max},
@@ -241,7 +241,8 @@ do_all_docs_req(Req, Db, Keys, NS) ->
         ),
         IsAdmin = is_admin(Db),
         Callback = get_view_callback(DbName, UsersDbName, IsAdmin),
-        couch_mrview:query_all_docs(Db, Args, Callback, VAcc0)
+        Args3 = couch_mrview_util:validate_all_docs_args(Db, Args2),
+        couch_mrview:query_all_docs(Db, Args3, Callback, VAcc0)
     end),
     case is_record(Resp, vacc) of
         true -> {ok, Resp#vacc.resp};

src/couch_mrview/src/couch_mrview_show.erl

@@ -218,15 +218,16 @@ handle_view_list(Req, Db, DDoc, LName, VDDoc, VName, Keys) ->
             false -> {ok, Acc0#lacc{etag = ETag}}
         end
     end,
-    Args = Args0#mrargs{preflight_fun = ETagFun},
+    Args1 = Args0#mrargs{preflight_fun = ETagFun},
     couch_httpd:etag_maybe(Req, fun() ->
         couch_query_servers:with_ddoc_proc(Db, DDoc, fun(QServer) ->
             Acc = #lacc{db = Db, req = Req, qserver = QServer, lname = LName},
             case VName of
                 <<"_all_docs">> ->
-                    couch_mrview:query_all_docs(Db, Args, fun list_cb/2, Acc);
+                    Args2 = couch_mrview_util:validate_all_docs_args(Db, Args1),
+                    couch_mrview:query_all_docs(Db, Args2, fun list_cb/2, Acc);
                 _ ->
-                    couch_mrview:query_view(Db, VDDoc, VName, Args, fun list_cb/2, Acc)
+                    couch_mrview:query_view(Db, VDDoc, VName, Args1, fun list_cb/2, Acc)
             end
         end)
     end).

src/couch_mrview/src/couch_mrview_util.erl

@@ -522,11 +522,11 @@ apply_limit(ViewPartitioned, Args) ->
             {false, _} -> "query_limit"
         end,
 
-    MaxLimit = config:get_integer(
-        "query_server_config",
-        LimitType,
-        ?MAX_VIEW_LIMIT
-    ),
+    MaxLimit =
+        case config:get("query_server_config", LimitType, "infinity") of
+            "infinity" -> ?MAX_VIEW_LIMIT;
+            Val -> list_to_integer(Val)
+        end,
 
     % Set the highest limit possible if a user has not
     % specified a limit
@@ -545,7 +545,7 @@ apply_limit(ViewPartitioned, Args) ->
     end.
 
 validate_all_docs_args(Db, Args0) ->
-    Args = validate_args(Args0),
+    Args = validate_args(Args0#mrargs{view_type = map}),
 
     DbPartitioned = couch_db:is_partitioned(Db),
     Partition = get_extra(Args, partition),
@@ -557,7 +557,7 @@ validate_all_docs_args(Db, Args0) ->
             Args1 = apply_limit(true, Args),
             apply_all_docs_partition(Args1, Partition);
         _ ->
-            Args
+            apply_limit(false, Args)
     end.
 
 validate_args(Args) ->

src/mango/src/mango_cursor_view.erl

@@ -246,7 +246,8 @@ execute(#cursor{db = Db, index = Idx, execution_stats = Stats} = Cursor0, UserFu
                 case mango_idx:def(Idx) of
                     all_docs ->
                         CB = fun ?MODULE:handle_all_docs_message/2,
-                        fabric:all_docs(Db, DbOpts, CB, Cursor, Args);
+                        Args1 = fabric_util:validate_all_docs_args(Db, Args),
+                        fabric:all_docs(Db, DbOpts, CB, Cursor, Args1);
                     _ ->
                         CB = fun ?MODULE:handle_message/2,
                         % Normal view