couchdb icon indicating copy to clipboard operation
couchdb copied to clipboard

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

Open pgj opened this issue 2 years ago • 7 comments
trafficstars

This is an alternative to #4710 based on the results of the related discussion.

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

Testing recommendations

The changes could be verified by the following commands:

make eunit apps=mango
make mango-test

This is a non-breaking change, so there should be no observable difference in the behavior for the users. When allow_fallback is set to false and use_index is set to some index, it may return HTTP 400 depending on the suitability of the given index.

Checklist

  • [x] Code is written and works correctly
  • [x] Changes are covered by tests
  • [x] Documentation changes were made in the src/docs folder

pgj avatar Oct 04 '23 16:10 pgj

Yes, I agree. @rnewson, @willholley, @ricellis please comment if you have the time.

pgj avatar Oct 05 '23 15:10 pgj

I was under the impression that allow_fallback was going to apply to all _find requests not just those with use_index. I know @willholley disagreed with that overloading, but I thought that was only related to the cluster toggles. I didn't think there was an objection to disallowing all_docs fallback as long as applied consistently.

ricellis avatar Oct 05 '23 16:10 ricellis

I do not want to respond on behalf of @willholley but in my understanding, in his respective comment he opposed to extending the allow_fallback mechanism to _all_docs: "I'm less keen on overloading it to removing the use of the _all_docs index". He also included a historical reference to suggest that it would not give us much: "Mango did originally prevent the _all_docs fallback unless the query contained a clause that would require it. [..] lots of users just blindly added a { _id: { "$gt": 0 } } clause to [..] work around the restriction."

pgj avatar Oct 09 '23 13:10 pgj

if allow_fallback: false will allow fallback, then it will not be merged.

rnewson avatar Oct 11 '23 16:10 rnewson

@rnewson do you mean that we shall extend this change to disabling the _all_docs fallback as well?

pgj avatar Oct 11 '23 16:10 pgj

Yes. Remember this an opt-in in the request body by the user. They are asking us to not fallback; they want an error instead of inefficient execution. I do not understand @willholley's objection to this.

rnewson avatar Oct 11 '23 16:10 rnewson

I don't think this issue / PR has anything to do with query efficiency - only fixing the unclear semantics of use_index.

I'm open to a discussion about if we want to / how to prevent use of inefficient indexes or, but that's much broader problem space in which I don't see _all_docs as any different to user-defined indexes.

willholley avatar Oct 11 '23 20:10 willholley