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

MRView's seq btree needs custom sorter function. #592

Closed
eiri opened this issue Jun 9, 2017 · 3 comments
Closed

MRView's seq btree needs custom sorter function. #592

eiri opened this issue Jun 9, 2017 · 3 comments

Comments

@eiri
Copy link
Member

eiri commented Jun 9, 2017

Expected Behavior

During work on #560 it was found that there are bug in mrview's seq btree leading to inconsistent behaviour on query with since parameter.

It is expected that when querying view_changes_since we should get changes feed starting from next update sequence than provided at since.

Current Behavior

Response of couch_mrview:view_changes_since/7 either include or exclude entry for since update sequence, depending on a view's key data type.

Possible Solution

The issue here is that seq_btree using tuples{UpdSeq, ViewKey} as the keys and ViewKey can be any mapped from JSON data type: binary, integer or boolean. We are using erlang native collision to find first key to start from, so we can't construct consistent low limit key.

We need to specify custom less function on seq_btree creation and normalize keys in it.

Steps to Reproduce (for bugs)

Since http end-point /{db}/_design/{ddoc}/_view_changes not exposed at the moment it's a bit hard to reproduce without changing existing changes_since_test_ test suite to create view with integer keys instead of binaries.

Context

It's a low priority issue, the function in question not exposed to any interface.

@AlexanderKaraberov
Copy link
Contributor

AlexanderKaraberov commented Jul 31, 2018

Hi @eiri
I don't want to escalate and create a separate issue for a behaviour which just seems confusing to me and probably is not a bug at all therefore I decided to ask here.
So I have a document in the db which I delete either via DELETE /db/doc?rev=1-aa... or via PUT db/doc?rev=1-aa... -d "{_deleted":true}".
And now I want to "undelete" it via PUT db/doc?rev=2-aa... -d "{_deleted":false}". This was working fine in the CouchDB 1.x and is something what I'd expect to work in 2.x too, because when updating an existing document, the current document revision must be included in the document for an obvious reason (updater has to know a node in the revision tree). But now in CouchDB 2.x I receive:

{"error":"conflict","reason":"Document update conflict."}

This works fine if I'm not including document revision in the request for the case when I "undelete" the doc. So, the actual logic still works and I see all the corresponding revisions via revs_info=true. My question is this an expected behaviour? And if yes what were the reasons behind this? I think this has something to do with the new logic in the couch_db_updater related to the merging of revision trees.

@eiri
Copy link
Member Author

eiri commented Aug 1, 2018

@AlexanderKaraberov Ah, this happened before of my time, so it took a bit of digging through the old commits to see what's going on here.

Yes, this is an intentional change and it was done to address this bug. Short version is that the previous version of merge_rev_trees relied on equality of the old rev tree and the merged rev tree in order to know when the update is a document recreation. If body of the deleted document been compacted away then at recreation you've been getting into situation when the old rev tree and the new rev tree different in content but identical in structure.

To address that merge_rev_trees was essentially rewritten completely in this commit. So now when you are trying to recreate with rev you are ending up in this codepath, because your rev's depth bigger than 1.

This is all a bit esoteric, so I hope it makes sense to you.

@eiri
Copy link
Member Author

eiri commented Aug 1, 2018

After discussion dev@ mail list we decided to just remove this implementation of view's changes and redoing it later from a scratch.

@eiri eiri closed this as completed Aug 1, 2018
@eiri eiri added the wontfix label Aug 1, 2018
nickva pushed a commit to nickva/couchdb that referenced this issue Sep 7, 2022
* Fix spelling/typo mistake in /_up endpoint description

* Bump version number to 3.1.1 (apache#592)

* Jenkins: do not alwaysPull true

Co-authored-by: Jonathan Hall <[email protected]>
nickva pushed a commit to nickva/couchdb that referenced this issue Sep 7, 2022
* Bump version number to 3.1.1 (apache#592)

* Fix spelling/typo mistake in /_up endpoint description

* Document pagination API

* Address problems identified durring review

* Fix typo

* Fix build failures

Co-authored-by: Joan Touzet <[email protected]>
Co-authored-by: Jonathan Hall <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants