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

Remove vestiges of view-based _changes feed from codebase #2324

Merged
merged 7 commits into from
Dec 3, 2019

Conversation

eiri
Copy link
Member

@eiri eiri commented Nov 22, 2019

Overview

This PR removes all bits and pieces of view_changes from mrview, couch_changes and fabric applications, removes /_view_changes end-point and removes seq, kseq and log btrees from mrview file structure. Migration code for 1.2 views was modified to support transparent migration from 2.x views.

Testing recommendations

All the tests should pass. Sinice this is removing functionality tests for which been disabled anyway, it's hard to validate it otherwise.

Additional manual testing could be done with creation of views on CouchDB 2.x with seq_indexed and / or keyseq_indexed options set and then accessing them on this CouchDB version. Here is a setup I've used for testing, it could help to get up to speed with that.

Related Issues or Pull Requests

Closes #2167

Checklist

@eiri eiri requested review from davisp and nickva and removed request for nickva November 22, 2019 06:32
Copy link
Member

@kocolosk kocolosk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is some excellent work @eiri . My only comment is to improve the logging whenever we attempt an online upgrade of a 2.x index file. Otherwise it all looks good!

@@ -1168,10 +965,10 @@ maybe_update_index_file(State) ->
end.

update_index_file(State) ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest upgrading the logging here to notice level and including more details about the index file that we're attempting to update.

Copy link
Member

@davisp davisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! +1

We'll probably want to make a note about index upgrades from 1.x to 3.x not being transparent somewhere (unless we already have a disclaimer for not jumping from 1.x to 3.x).

@eiri
Copy link
Member Author

eiri commented Dec 3, 2019

@kocolosk @davisp Thank you for the reviews! I've added details to upgrade logs and fixed a small upgrade bug, added a commit right after upgrade to make sure we have new sig in header on disk. Will to merge once travis allows and then add a note to upgrade docs on views 1.x incompatibility.

@eiri eiri merged commit 87edbae into master Dec 3, 2019
@eiri eiri deleted the 2167-no-view-changes branch December 3, 2019 23:47
@wohali wohali mentioned this pull request Dec 4, 2019
4 tasks
nickva added a commit that referenced this pull request Dec 11, 2019
The test broke during the removal of view-based changes PR

Issue: #2324
@nickva nickva mentioned this pull request Dec 11, 2019
davisp pushed a commit that referenced this pull request Dec 11, 2019
The test broke during the removal of view-based changes PR

Issue: #2324
nickva added a commit that referenced this pull request Dec 11, 2019
The test broke during the removal of view-based changes PR

Issue: #2324
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.

Remove vestiges of view-based _changes feed
3 participants