Skip to content

Fix partitioned query database header version #1886

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 30, 2019
Merged

Fix partitioned query database header version #1886

merged 1 commit into from
Jan 30, 2019

Conversation

@jiangphcn
Copy link
Contributor

@jiangphcn jiangphcn commented Jan 30, 2019

Overview

Allow to upgrade database from version 7 to version 8 which supports the partitioned query. To do so, the database can be downgraded from version 8 to version 7 once there is need to downgrade.

Also support to upgrade database from version 6 to version 8 if database is not accessed for long time to skip version 7.

Testing recommendations

make check skip_deps+=couch_epi apps=couch tests=upgrade_test_

======================== EUnit ========================
Upgrade tests
Application crypto was left running!
  couchdb_views_tests:189: should_upgrade_legacy_view_files...[2.895 s] ok
[os_mon] cpu supervisor port (cpu_sup): Erlang has closed
  [done in 2.903 s]
Couch Bt Engine Upgrade tests
  couch_bt_engine_upgrade_tests:66: t_upgrade_without_purge_req...[0.130 s] ok
[os_mon] cpu supervisor port (cpu_sup): Erlang has closed
  couch_bt_engine_upgrade_tests:66: t_upgrade_without_purge_req...[0.132 s] ok
[os_mon] cpu supervisor port (cpu_sup): Erlang has closed
  couch_bt_engine_upgrade_tests:106: t_upgrade_with_1_purge_req...[0.151 s] ok
[os_mon] cpu supervisor port (cpu_sup): Erlang has closed
  couch_bt_engine_upgrade_tests:106: t_upgrade_with_1_purge_req...[0.136 s] ok
[os_mon] cpu supervisor port (cpu_sup): Erlang has closed
  couch_bt_engine_upgrade_tests:147: t_upgrade_with_N_purge_req...[0.147 s] ok
[os_mon] cpu supervisor port (cpu_sup): Erlang has closed
  couch_bt_engine_upgrade_tests:147: t_upgrade_with_N_purge_req...[0.138 s] ok
[os_mon] cpu supervisor port (cpu_sup): Erlang has closed
  couch_bt_engine_upgrade_tests:186: t_upgrade_with_1_purge_req_for_2_docs...[0.143 s] ok
[os_mon] cpu supervisor port (cpu_sup): Erlang has closed
  couch_bt_engine_upgrade_tests:186: t_upgrade_with_1_purge_req_for_2_docs...[0.129 s] ok
[os_mon] cpu supervisor port (cpu_sup): Erlang has closed
  [done in 3.737 s]
=======================================================
  All 9 tests passed.
==> rel (eunit)
==> db (eunit)

Related Issues or Pull Requests

#1789

Checklist

  • Code is written and works correctly;
  • Changes are covered by tests;
  • Documentation reflects the changes;

davisp
davisp reviewed Jan 30, 2019
View reviewed changes
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.

Tests and code changes look good but I'm not 100% on those eunit test changes. EUnit is kinda subtle so if there's obvious answers I'd accept that.

src/couch/test/couch_bt_engine_partition_downgrade_tests.erl Outdated


downgrade_test_() ->
downgrade_test() ->
Copy link
Member

@davisp davisp Jan 30, 2019

Choose a reason for hiding this comment

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

Are we sure that this is right? eunit deferred tests are weird so i can never tell off hand if the trailing underscore is necessary.

Copy link
Contributor Author

@jiangphcn jiangphcn Jan 30, 2019

Choose a reason for hiding this comment

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

my intention is not to let this downgrade_test_/0 to run because we remove the downgrade logic from PQ to non-PQ test.

Copy link
Member

@davisp davisp Jan 30, 2019

Choose a reason for hiding this comment

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

Aha. Then I'd just delete the test.

Copy link
Contributor Author

@jiangphcn jiangphcn Jan 30, 2019

Choose a reason for hiding this comment

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

deleted

src/couch/test/couch_bt_engine_upgrade_tests.erl


setup() ->
setup(_) ->
Copy link
Member

@davisp davisp Jan 30, 2019

Choose a reason for hiding this comment

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

Why did this change?

Copy link
Contributor Author

@jiangphcn jiangphcn Jan 30, 2019

Choose a reason for hiding this comment

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

because i used foreachx to perform test from 6 to 8, and 7 to 8 respectively now. So just follow the case in

couchdb/src/couch_replicator/test/couch_replicator_filtered_tests.erl

Line 49 in 103a062

setup(_) ->

Copy link
Member

@davisp davisp Jan 30, 2019

Choose a reason for hiding this comment

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

Gotchya. Sounds good. EUnit is hard.

davisp
davisp approved these changes Jan 30, 2019
View reviewed changes
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.

+1 once that downgrade test is removed.

@jiangphcn jiangphcn force-pushed the fix-pq-db-header-version branch 2 times, most recently from 6257930 to 70e8fa8 Compare January 30, 2019 12:22
@jiangphcn jiangphcn force-pushed the fix-pq-db-header-version branch from 70e8fa8 to 6d317e1 Compare January 30, 2019 12:24
@jiangphcn jiangphcn merged commit aba6633 into master Jan 30, 2019
@nickva nickva deleted the fix-pq-db-header-version branch December 18, 2019 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davisp davisp davisp approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jiangphcn @davisp