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

fix(docs): amend description of use_index on /{db}/_find #4774

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

pgj
Copy link
Contributor

@pgj pgj commented Sep 24, 2023

The semantics of Mango's use_index query parameter has changed over time but this has not been reflected in the documentation, which causes a lot of confusion.

The use_index parameter was introduced in 1b0426a to force the index selection to a specific index. Unfortunately, this did not work out well in practice, so 743bd88 added a fallback mechanism to make it less brittle. With that, use_index became only a "hint" not an "instruction".

@pgj pgj requested a review from willholley September 24, 2023 20:21
@pgj pgj mentioned this pull request Sep 24, 2023
4 tasks
@rnewson
Copy link
Member

rnewson commented Sep 25, 2023

The discussion in #4710 is ongoing with advocates for fixing use_index to work the way the documentation says.

I think we'll need a vote on it to decide.

@pgj
Copy link
Contributor Author

pgj commented Sep 25, 2023

@rnewson please note that I have included references to the commit history to support that the way use_index currently works is not a bug but a feature, a mitigation to the valid problem.

The bug here is that the documentation has not been updated. Also, it happened a while (7 years) ago and independently of this was a good design decision or not, it is now part of CouchDB and there may be integrations on that work under these assumptions. Especially that after the expected change CouchDB would immediately start emitting HTTP 400s instead of HTTP 200s in those scenarios. How would this work out in a multi-version cluster?

In #4710 I was proposing ways to change the behavior of use_index in backward-compatible ways (i.e. rename use_index to index_hint and introduce force_index to restore the old behavior, also the toggles) but it was rejected. While I hear you and understand all your concerns and assume good intentions behind your comments, I do not think restoring use_index to the old, assumed good behavior right away is the good solution. Or I might be misunderstanding something?

@rnewson
Copy link
Member

rnewson commented Sep 25, 2023

@pgj I think we're all confused. I appreciate you checking through historical comments.

To resolve this I think it needs to move to the dev@ mailing list. Is this something you are comfortable doing? I'm happy to do it in your stead otherwise. If you wish to proceed, I suggest a [DISCUSS] thread that references this ticket and any relevant history, a brief description of use_index and how it acts in practice (with the fallback), and list out the suggested paths to address this (roughly, 1) accept the current behaviour and adjust the documentation 2) change the behaviour to not allow the fallback and clarify this in docs and release notes as potentially a breaking change 3) a combination of 1 and introducing a new way to get the strict use_index)

@pgj
Copy link
Contributor Author

pgj commented Sep 25, 2023

Yes, I am happy to discuss that on dev@. Please give me a day or so to prepare the topic starter email.

@pgj
Copy link
Contributor Author

pgj commented Sep 26, 2023

@rnewson given that commenters are about to agree regarding #4710, do you still think that launching a discussion and potentially a vote on dev@ would be necessary?

@rnewson
Copy link
Member

rnewson commented Sep 26, 2023

I think no longer needed.

@pgj
Copy link
Contributor Author

pgj commented Sep 26, 2023

Excellent.

The semantics of Mango's `use_index` query parameter has changed
over time but this has not been reflected in the documentation,
which causes a lot of confusion.

The `use_index` parameter was introduced in 1b0426a to force the
index selection to a specific index.  Unfortunately, this did not
work out well in practice, so 743bd88 added a fallback mechanism
to make it less brittle.  With that, `use_index` became only a
"hint" not an "instruction".
@pgj pgj force-pushed the fix/docs/_find-use_index branch from fc0c1dc to 38aaea0 Compare October 4, 2023 12:39
@pgj
Copy link
Contributor Author

pgj commented Oct 4, 2023

Based on the recent conclusion of #4710, @rnewson are you fine with having this PR merged?

@pgj
Copy link
Contributor Author

pgj commented Oct 4, 2023

Thanks @rnewson !

@pgj pgj merged commit aaf9005 into apache:main Oct 4, 2023
@pgj pgj deleted the fix/docs/_find-use_index branch October 4, 2023 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants