Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(mango): add allow_fallback for user-specified indexes on _find #4792

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
mango: add allow_fallback for user-specified indexes on _find
It is not always beneficial for the performance if the Mango query
planner tries to assign an index to the selector.  User-specified
indexes may save the day, but since they are only hints for the
planner, fallbacks may still happen.

Introduce the `allow_fallback` flag which can be used to tell if
falling back to other indexes is acceptable when an index is
explicitly specified by the user.  When set to `false`, give up
on planning and return an HTTP 400 response right away.  This way
the user has the chance to learn about the requested but missing
index, optionally create it and try again.

By default, fallbacks are allowed to maintain backwards
compatibility.  It is possible to set `allow_fallback` to `true`
but currently it coincides with the default behavior hence becomes
a no-op in practice.

Fixes #4511
  • Loading branch information
pgj committed Oct 5, 2023
commit 3cd732ed0c90f443db258766a20b292b003d8d3b
9 changes: 8 additions & 1 deletion src/docs/src/api/database/find.rst
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@
is attempted. Therefore that is more like a hint. When
fallback occurs, the details are given in the ``warning``
field of the response. *Optional*
:<json boolean allow_fallback: Tell if it is allowed to fall back
to a valid index when requesting a query to use a specific
index that is not deemed usable. Default is ``true``. This
is meant to be used in combination with ``use_index`` and
setting ``allow_fallback`` to ``false`` can make the query
fail if the user-specified index is not suitable. *Optional*
:<json boolean conflicts: Include conflicted documents if ``true``.
Intended use is to easily find conflicted documents, without an
index or view. Default is ``false``. *Optional*
Expand Down Expand Up @@ -1492,7 +1498,8 @@ it easier to take advantage of future improvements to query planning
"stale": false,
"update": true,
"stable": false,
"execution_stats": false
"execution_stats": false,
"allow_fallback": true
},
"limit": 2,
"skip": 0,
Expand Down
104 changes: 95 additions & 9 deletions src/mango/src/mango_cursor.erl
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,22 @@ create(Db, Selector0, Opts, Kind) ->
{UsableIndexes, Trace} = mango_idx:get_usable_indexes(Db, Selector, Opts, Kind),
case maybe_filter_indexes_by_ddoc(UsableIndexes, Opts) of
[] ->
% use_index doesn't match a valid index - fall back to a valid one
create_cursor(Db, {UsableIndexes, Trace}, Selector, Opts);
% use_index doesn't match a valid index - determine how
% this shall be handled by the further settings
case allow_fallback(Opts) of
true ->
% fall back to a valid index
create_cursor(Db, {UsableIndexes, Trace}, Selector, Opts);
false ->
% return an error
Details =
case use_index(Opts) of
[] -> [];
[DesignId] -> [ddoc_name(DesignId)];
[DesignId, ViewName] -> [ddoc_name(DesignId), ViewName]
end,
?MANGO_ERROR({invalid_index, Details})
end;
UserSpecifiedIndex ->
create_cursor(Db, {UserSpecifiedIndex, Trace}, Selector, Opts)
end.
Expand Down Expand Up @@ -366,13 +380,21 @@ execute(#cursor{index = Idx} = Cursor, UserFun, UserAcc) ->
Mod = mango_idx:cursor_mod(Idx),
Mod:execute(Cursor, UserFun, UserAcc).

use_index(Opts) ->
{use_index, UseIndex} = lists:keyfind(use_index, 1, Opts),
UseIndex.

allow_fallback(Opts) ->
{allow_fallback, AllowFallback} = lists:keyfind(allow_fallback, 1, Opts),
AllowFallback.

maybe_filter_indexes_by_ddoc(Indexes, Opts) ->
case lists:keyfind(use_index, 1, Opts) of
{use_index, []} ->
case use_index(Opts) of
[] ->
[];
{use_index, [DesignId]} ->
[DesignId] ->
filter_indexes(Indexes, DesignId);
{use_index, [DesignId, ViewName]} ->
[DesignId, ViewName] ->
filter_indexes(Indexes, DesignId, ViewName)
end.

Expand Down Expand Up @@ -573,7 +595,9 @@ create_test_() ->
[
?TDEF_FE(t_create_regular, 10),
?TDEF_FE(t_create_user_specified_index, 10),
?TDEF_FE(t_create_invalid_user_specified_index, 10)
?TDEF_FE(t_create_invalid_user_specified_index, 10),
?TDEF_FE(t_create_invalid_user_specified_index_no_fallback_1, 10),
?TDEF_FE(t_create_invalid_user_specified_index_no_fallback_2, 10)
]
}.

Expand All @@ -589,7 +613,7 @@ t_create_regular(_) ->
filtered_indexes => sets:from_list(FilteredIndexes),
indexes_of_type => sets:from_list(IndexesOfType)
},
Options = [{use_index, []}],
Options = [{use_index, []}, {allow_fallback, true}],
meck:expect(mango_selector, normalize, [selector], meck:val(normalized_selector)),
meck:expect(
mango_idx,
Expand Down Expand Up @@ -648,7 +672,7 @@ t_create_invalid_user_specified_index(_) ->
filtered_indexes => sets:from_list(UsableIndexes),
indexes_of_type => sets:from_list(IndexesOfType)
},
Options = [{use_index, [<<"foobar">>]}],
Options = [{use_index, [<<"foobar">>]}, {allow_fallback, true}],
meck:expect(mango_selector, normalize, [selector], meck:val(normalized_selector)),
meck:expect(
mango_idx,
Expand All @@ -664,6 +688,68 @@ t_create_invalid_user_specified_index(_) ->
),
?assertEqual(view_cursor, create(db, selector, Options, target)).

t_create_invalid_user_specified_index_no_fallback_1(_) ->
IndexSpecial = #idx{type = <<"special">>, def = all_docs},
IndexView1 = #idx{type = <<"json">>, ddoc = <<"_design/view_idx1">>},
IndexView2 = #idx{type = <<"json">>, ddoc = <<"_design/view_idx2">>},
IndexView3 = #idx{type = <<"json">>, ddoc = <<"_design/view_idx3">>},
UsableIndexes = [IndexSpecial, IndexView1, IndexView2, IndexView3],
IndexesOfType = [IndexView1, IndexView2, IndexView3],
Trace1 = #{},
Trace2 =
#{
filtered_indexes => sets:from_list(UsableIndexes),
indexes_of_type => sets:from_list(IndexesOfType)
},
UseIndex = [<<"design">>, <<"foobar">>],
Options = [{use_index, UseIndex}, {allow_fallback, false}],
meck:expect(mango_selector, normalize, [selector], meck:val(normalized_selector)),
meck:expect(
mango_idx,
get_usable_indexes,
[db, normalized_selector, Options, target],
meck:val({UsableIndexes, Trace1})
),
meck:expect(
mango_cursor_view,
create,
[db, {IndexesOfType, Trace2}, normalized_selector, Options],
meck:val(view_cursor)
),
Exception = {mango_error, mango_cursor, {invalid_index, UseIndex}},
?assertThrow(Exception, create(db, selector, Options, target)).

t_create_invalid_user_specified_index_no_fallback_2(_) ->
IndexSpecial = #idx{type = <<"special">>, def = all_docs},
IndexView1 = #idx{type = <<"json">>, ddoc = <<"_design/view_idx1">>},
IndexView2 = #idx{type = <<"json">>, ddoc = <<"_design/view_idx2">>},
IndexView3 = #idx{type = <<"json">>, ddoc = <<"_design/view_idx3">>},
UsableIndexes = [IndexSpecial, IndexView1, IndexView2, IndexView3],
IndexesOfType = [IndexView1, IndexView2, IndexView3],
Trace1 = #{},
Trace2 =
#{
filtered_indexes => sets:from_list(UsableIndexes),
indexes_of_type => sets:from_list(IndexesOfType)
},
UseIndex = [],
Options = [{use_index, UseIndex}, {allow_fallback, false}],
meck:expect(mango_selector, normalize, [selector], meck:val(normalized_selector)),
meck:expect(
mango_idx,
get_usable_indexes,
[db, normalized_selector, Options, target],
meck:val({UsableIndexes, Trace1})
),
meck:expect(
mango_cursor_view,
create,
[db, {IndexesOfType, Trace2}, normalized_selector, Options],
meck:val(view_cursor)
),
Exception = {mango_error, mango_cursor, {invalid_index, UseIndex}},
?assertThrow(Exception, create(db, selector, Options, target)).

enhance_candidates_test() ->
Candidates1 = #{index => #{reason => [], usable => true}},
Candidates2 = #{index => #{reason => [reason1], usable => true}},
Expand Down
22 changes: 22 additions & 0 deletions src/mango/src/mango_error.erl
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,28 @@ info(mango_json_bookmark, {invalid_bookmark, BadBookmark}) ->
<<"invalid_bookmark">>,
fmt("Invalid bookmark value: ~s", [?JSON_ENCODE(BadBookmark)])
};
info(mango_cursor, {invalid_index, []}) ->
{
400,
<<"invalid_index">>,
<<"You must specify an index with the `use_index` parameter.">>
};
info(mango_cursor, {invalid_index, [DDocName]}) ->
{
400,
<<"invalid_index">>,
fmt("_design/~s specified by `use_index` could not be found or it is not suitable.", [
DDocName
])
};
info(mango_cursor, {invalid_index, [DDocName, ViewName]}) ->
{
400,
<<"invalid_index">>,
fmt("_design/~s, ~s specified by `use_index` could not be found or it is not suitable.", [
DDocName, ViewName
])
};
info(mango_cursor_text, {invalid_bookmark, BadBookmark}) ->
{
400,
Expand Down
6 changes: 6 additions & 0 deletions src/mango/src/mango_opts.erl
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,12 @@ validate_find({Props}) ->
{optional, true},
{default, false},
{validator, fun mango_opts:is_boolean/1}
]},
{<<"allow_fallback">>, [
{tag, allow_fallback},
{optional, true},
{default, true},
{validator, fun mango_opts:is_boolean/1}
]}
],
validate(Props, Opts).
Expand Down
1 change: 1 addition & 0 deletions src/mango/test/02-basic-find-test.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ def test_explain_options(self):
assert opts["stale"] == False
assert opts["update"] == True
assert opts["use_index"] == []
assert opts["allow_fallback"] == True

def test_sort_with_all_docs(self):
explain = self.db.find(
Expand Down
25 changes: 25 additions & 0 deletions src/mango/test/05-index-selection-test.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,31 @@ def test_explain_sort_reverse(self):
)
self.assertEqual(resp_explain["index"]["type"], "json")

def test_use_index_without_fallback(self):
with self.subTest(use_index="valid"):
docs = self.db.find(
{"manager": True}, use_index="manager", allow_fallback=False
)
assert len(docs) > 0

with self.subTest(use_index="invalid"):
try:
self.db.find(
{"manager": True}, use_index="invalid", allow_fallback=False
)
except Exception as e:
self.assertEqual(e.response.status_code, 400)
else:
raise AssertionError("did not fail on invalid index")

with self.subTest(use_index="empty"):
try:
self.db.find({"manager": True}, use_index=[], allow_fallback=False)
except Exception as e:
self.assertEqual(e.response.status_code, 400)
else:
raise AssertionError("did not fail due to missing use_index")


class JSONIndexSelectionTests(mango.UserDocsTests, IndexSelectionTests):
@classmethod
Expand Down
3 changes: 3 additions & 0 deletions src/mango/test/mango.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ def find(
update=True,
executionStats=False,
partition=None,
allow_fallback=None,
):
body = {
"selector": selector,
Expand All @@ -301,6 +302,8 @@ def find(
body["update"] = False
if executionStats == True:
body["execution_stats"] = True
if allow_fallback is not None:
body["allow_fallback"] = allow_fallback
body = json.dumps(body)
if partition:
ppath = "_partition/{}/".format(partition)
Expand Down