From f13501eb3ffcb4bd315693e53c54aa396e6b78cd Mon Sep 17 00:00:00 2001 From: Gabor Pali Date: Wed, 4 Oct 2023 18:38:56 +0200 Subject: [PATCH] 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 --- src/docs/src/api/database/find.rst | 9 +- src/mango/src/mango_cursor.erl | 106 ++++++++++++++++++++-- src/mango/src/mango_error.erl | 22 +++++ src/mango/src/mango_opts.erl | 6 ++ src/mango/test/02-basic-find-test.py | 1 + src/mango/test/05-index-selection-test.py | 25 +++++ src/mango/test/mango.py | 3 + 7 files changed, 162 insertions(+), 10 deletions(-) diff --git a/src/docs/src/api/database/find.rst b/src/docs/src/api/database/find.rst index c707a1c1e0f..7efdbe916af 100644 --- a/src/docs/src/api/database/find.rst +++ b/src/docs/src/api/database/find.rst @@ -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* + : {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 + [] -> + []; + UseIndex -> + [DesignId | Rest] = UseIndex, + [ddoc_name(DesignId) | Rest] + end, + ?MANGO_ERROR({invalid_index, Details}) + end; UserSpecifiedIndex -> create_cursor(Db, {UserSpecifiedIndex, Trace}, Selector, Opts) end. @@ -366,13 +382,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. @@ -573,7 +597,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) ] }. @@ -589,7 +615,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, @@ -648,7 +674,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, @@ -664,6 +690,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}}, diff --git a/src/mango/src/mango_error.erl b/src/mango/src/mango_error.erl index f1c343854f3..989e9fa1245 100644 --- a/src/mango/src/mango_error.erl +++ b/src/mango/src/mango_error.erl @@ -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, diff --git a/src/mango/src/mango_opts.erl b/src/mango/src/mango_opts.erl index 96aa0eb42ef..1d3ded1701f 100644 --- a/src/mango/src/mango_opts.erl +++ b/src/mango/src/mango_opts.erl @@ -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). diff --git a/src/mango/test/02-basic-find-test.py b/src/mango/test/02-basic-find-test.py index 6544df8489f..9a701b06db8 100644 --- a/src/mango/test/02-basic-find-test.py +++ b/src/mango/test/02-basic-find-test.py @@ -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( diff --git a/src/mango/test/05-index-selection-test.py b/src/mango/test/05-index-selection-test.py index a4c15608a09..c4f245bdb27 100644 --- a/src/mango/test/05-index-selection-test.py +++ b/src/mango/test/05-index-selection-test.py @@ -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 diff --git a/src/mango/test/mango.py b/src/mango/test/mango.py index 2dff18f4006..c03e7db0964 100644 --- a/src/mango/test/mango.py +++ b/src/mango/test/mango.py @@ -248,6 +248,7 @@ def find( update=True, executionStats=False, partition=None, + allow_fallback=None, ): body = { "selector": selector, @@ -267,6 +268,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)