-
Notifications
You must be signed in to change notification settings - Fork 428
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
Multi tenancy init #3061
Multi tenancy init #3061
Conversation
for testing run: test-runner.sh --skip-cover --skip-small-tests --preset pgsql_mnesia --db pgsql -- service_domain_db ./rebar3 ct --suite mongoose_domain_core_SUITE
test-runner.sh --skip-cover --skip-small-tests --preset pgsql_mnesia --db pgsql --dev-nodes mim1 mim2 -- dynamic_domains_pm
Codecov Report
@@ Coverage Diff @@
## multi-tenancy-phase-1 #3061 +/- ##
=========================================================
+ Coverage 78.09% 78.69% +0.59%
=========================================================
Files 374 386 +12
Lines 31219 31610 +391
=========================================================
+ Hits 24382 24877 +495
+ Misses 6837 6733 -104
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added some comments
src/domain/service_domain_db.erl
Outdated
State = #{last_event_id => LastEventId, | ||
check_for_updates_interval => 30000}, | ||
{ok, handle_check_for_updates(State)}. | ||
gen_server:cast(self(),initial_loading), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will unblock execution, introducing new set of errors (i.e. domain is not on the node yet).
I highly recommend to not continue execution, unless every domain is on MongooseIM side.
We even don't allow that when syncing mnesia.
TLDR: you will have issues here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about unblocking initialisation of other modules/services.
Mostly like we will have to introduce some REST API check to ensure that node is ready for adding to load balancer.
Anyway it's relatively easy to roll back that change if required later.
mostly functions renaming and whitespaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good changes! Just a few comments from me.
6503c50
to
9743f99
Compare
4d5cd7a
to
86e8fd8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also had a quick look, it seems good!
this PR introduces the next changes:
check_for_updates/2
functionmongoose_domain_core
ifservice_domain_db
crashes on initial loadingservice_domain_db
goes out of syncAlso now it's possible to implement "hooks" for domains adding/removing if there's such a need