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

Mnesia migration #2561

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Mnesia migration #2561

wants to merge 4 commits into from

Conversation

vkatsuba
Copy link
Contributor

@vkatsuba vkatsuba commented Dec 10, 2019

Proposed changes include:

  • CTL for migration from Mnesia to RDMS

How To:
Migrate pubsub_nodes:

$ mongooseimctl migrate mnesia rdbms pubsub_nodes

Migrate pubsub_subscriptions:

$ mongooseimctl migrate mnesia rdbms pubsub_subscriptions

Migrate pubsub_affiliations:

$ mongooseimctl migrate mnesia rdbms pubsub_affiliations

Migrate pubsub_items:

$ mongooseimctl migrate mnesia rdbms pubsub_items

Migrate users:

$ mongooseimctl migrate mnesia rdbms users

Migrate vcard_search:

$ mongooseimctl migrate mnesia rdbms vcard_search

Migrate vcard:

$ mongooseimctl migrate mnesia rdbms vcard

Migrate event_pusher_push_subscription:

$ mongooseimctl migrate mnesia rdbms event_pusher_push_subscription

Migrate rosterusers:

$ mongooseimctl migrate mnesia rdbms rosterusers

Migrate roster_version:

$ mongooseimctl migrate mnesia rdbms roster_version

Migrate rostergroups:

$ mongooseimctl migrate mnesia rdbms rostergroups

Migrate last:

$ mongooseimctl migrate mnesia rdbms last

Migrate private_storage:

$ mongooseimctl migrate mnesia rdbms private_storage

Migrate offline_message:

$ mongooseimctl migrate mnesia rdbms offline_message

Migrate muc_light_rooms:
NOTES: Looks like before migration of muc_light_config and muc_light_occupants tables need to know room ID from muc_light_rooms, by this reason when run migrate mnesia rdbms muc_light_rooms will be migrated tables: muc_light_rooms, muc_light_config, muc_light_occupants

$ mongooseimctl migrate mnesia rdbms muc_light_rooms

Migrate all:

$ mongooseimctl migrate mnesia rdbms all

Copy link
Contributor

@michalwski michalwski left a comment

Choose a reason for hiding this comment

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

That's a huge effort and functionality we've been missing for long time already. Thanks a lot for that.
I managed to do partial review.

src/admin_extra/service_admin_extra_migration.erl Outdated Show resolved Hide resolved
src/admin_extra/service_admin_extra_migration.erl Outdated Show resolved Hide resolved
src/admin_extra/service_admin_extra_migration.erl Outdated Show resolved Hide resolved
src/admin_extra/service_admin_extra_migration.erl Outdated Show resolved Hide resolved
src/admin_extra/service_admin_extra_migration.erl Outdated Show resolved Hide resolved
src/admin_extra/service_admin_extra_migration.erl Outdated Show resolved Hide resolved
src/admin_extra/service_admin_extra_migration.erl Outdated Show resolved Hide resolved
src/admin_extra/service_admin_extra_migration.erl Outdated Show resolved Hide resolved
src/admin_extra/service_admin_extra_migration.erl Outdated Show resolved Hide resolved
src/admin_extra/service_admin_extra_migration.erl Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Dec 14, 2019

Codecov Report

Merging #2561 (6b0903f) into master (53935c5) will increase coverage by 0.96%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2561      +/-   ##
==========================================
+ Coverage   78.02%   78.99%   +0.96%     
==========================================
  Files         374      348      -26     
  Lines       31190    29647    -1543     
==========================================
- Hits        24337    23419     -918     
+ Misses       6853     6228     -625     
Impacted Files Coverage Δ
src/sasl/cyrsasl_scram.erl 68.11% <0.00%> (-27.44%) ⬇️
src/metrics/mongoose_metrics_mam_hooks.erl 66.66% <0.00%> (-20.00%) ⬇️
src/rdbms/rdbms_queries_mssql.erl 85.71% <0.00%> (-14.29%) ⬇️
src/auth/ejabberd_auth_internal.erl 57.14% <0.00%> (-13.77%) ⬇️
src/ejabberd.erl 45.00% <0.00%> (-10.00%) ⬇️
src/mongoose_lib.erl 73.68% <0.00%> (-8.54%) ⬇️
src/jlib.erl 82.84% <0.00%> (-6.69%) ⬇️
src/auth/ejabberd_auth.erl 61.42% <0.00%> (-6.28%) ⬇️
src/shaper.erl 94.11% <0.00%> (-5.89%) ⬇️
src/config/mongoose_config_parser.erl 74.29% <0.00%> (-5.71%) ⬇️
... and 277 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53935c5...73f4ce0. Read the comment docs.

@vkatsuba vkatsuba changed the title WIP: Mnesia migration Mnesia migration Dec 17, 2019
Copy link
Contributor

@michalwski michalwski left a comment

Choose a reason for hiding this comment

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

Thanks for the update I had 2 more comments. Also we usually don't merge new functionality to master without tests. I don't expect you to work on tests now, you did huge work already. I'll try accommodate some time for writing tests for these migrations in January.

src/admin_extra/service_admin_extra_migration.erl Outdated Show resolved Hide resolved
src/admin_extra/service_admin_extra_migration.erl Outdated Show resolved Hide resolved
@michalwski
Copy link
Contributor

Hi @vkatsuba, sorry it took me so long to respond.
Regarding the approach to tests, I think the following would the simplest and enough approach:

  • It makes sens to run the tests only on these jobs which have a RDBMS backend configured. You can detect that by calling mongoose_helper:is_rdbms_enabled(XMPPHost).
  • The test could do sth like this:
    • use the mod_*_mnesia module to generate some data in Mnesia
    • run appropriate migration function
    • use the mod_*_rdbms and verify if the migration succeeded. Basically if functions from mod_*_rdbms returns the same values as corresponding function from mod_*_mnesia
    • remember to call mod_*_mnesia:init in order to create the mnesia table
    • remember to remove mnesia tables after the test

Let me know what do you think about it.

@vkatsuba vkatsuba force-pushed the mnesia-migration branch 2 times, most recently from 9e23cdd to e876cc2 Compare July 22, 2020 11:25
@Neustradamus

This comment was marked as spam.

@vkatsuba
Copy link
Contributor Author

@michalwski: Please look it :)

Still need add CT. I will try to do it soonish.

@vkatsuba vkatsuba force-pushed the mnesia-migration branch 6 times, most recently from 76b2594 to edaddc0 Compare December 22, 2020 16:10
@vkatsuba vkatsuba force-pushed the mnesia-migration branch 12 times, most recently from 59867fc to 007c951 Compare December 24, 2020 19:51
@vkatsuba vkatsuba force-pushed the mnesia-migration branch 2 times, most recently from e224b8e to 2b675d3 Compare December 25, 2020 00:17
Added migration for: pubsub_items migration, users, vcard, vcard_search, event_pusher_push_subscription, rosterusers, roster_versio, last, rostergroups, muc_light_occupants, muc_light_config, migrate_muc_light_rooms, migrate_offline_message, migrate_private_storage
…ubscriptions, migrate_pubsub_affiliations, migrate_pubsub_items, migrate_users, migrate_vcard_search, migrate_vcard, migrate_event_pusher_push_subscription
@Neustradamus

This comment was marked as spam.

@vkatsuba
Copy link
Contributor Author

@vkatsuba: Any news?

Hi @Neustradamus. Current implementation a little bit outdated and should be updated with using latest tables structure. Still in progress. Hopefully in the next few weeks I can take the time to update and complete the current implementation. Thank you for reminding.

@Neustradamus

This comment was marked as spam.

@vkatsuba
Copy link
Contributor Author

vkatsuba commented Aug 2, 2022

Hi @Neustradamus. Sorry but I don't have a lot of time for now for finalize this version for MIM, however if you need include this functionality into your project you can always ping me and I will try to help you will adaptation of this code for your project.

Regards,
-V

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants