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

Add new shared subscription dispatch strategy #1823

Merged
merged 2 commits into from Sep 22, 2018

Conversation

@spring2maz
Copy link
Contributor

spring2maz commented Sep 15, 2018

Fixes #1467 #1486
'random' was already there before this change
Added three new strategies: 'sticky', 'round_robin', and 'hash'
'round_robin' is made default
and 'sticky' is the cheapest

@coveralls

This comment has been minimized.

Copy link

coveralls commented Sep 15, 2018

Pull Request Test Coverage Report for Build 3340

  • 28 of 29 (96.55%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+1.2%) to 56.432%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/emqx_shared_sub.erl 28 29 96.55%
Totals Coverage Status
Change from base Build 3332: 1.2%
Covered Lines: 2360
Relevant Lines: 4182

💛 - Coveralls
@spring2maz spring2maz requested review from emqplus and gilbertwong96 Sep 15, 2018
@spring2maz spring2maz force-pushed the add-shared-sub-strategy branch from d855e0b to 7a87581 Sep 15, 2018
@emqplus emqplus self-assigned this Sep 17, 2018
@emqplus emqplus added the Enhancement label Sep 17, 2018
@emqplus emqplus added this to the 3.0-beta.3 milestone Sep 17, 2018
@spring2maz spring2maz force-pushed the add-shared-sub-strategy branch 4 times, most recently from a96feb5 to 20f06ad Sep 19, 2018
spring2maz added 2 commits Sep 13, 2018
'random' was already there before this change
Added two new strategies: 'sticky' and 'round_robin'
'sticky' is made default as it is the cheapest
Before this change, eqmx_mock_client uses a shared ets table
to store last received message, this causes troulbe when we
want to start / stop two or more clients in one test case
the ets table gets owned by the first spanwed client and
gets closed when the owner client dies.

Now it keeps the last received message in process state
and a gen_server call is added to retrieve it for verification

Along with this change in emqx_mock_client, it made possible
to write test case to verify the actual subscriber pid
used in shared subscription strategy, so test cases were
added (and modified) to verify different strategies
@spring2maz spring2maz force-pushed the add-shared-sub-strategy branch from 20f06ad to 3b92479 Sep 22, 2018
@turtleDeng turtleDeng merged commit 925e98a into emqx30 Sep 22, 2018
3 checks passed
3 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+1.2%) to 56.432%
Details
@gbrehmer

This comment has been minimized.

Copy link

gbrehmer commented Sep 25, 2018

@spring2maz sorry I didn't look into the implementation too much, but it is also possible to favor online clients (offline clients with session should be ignored if possible = other online clients exists)?

@spring2maz spring2maz deleted the add-shared-sub-strategy branch Sep 25, 2018
@spring2maz

This comment has been minimized.

Copy link
Contributor Author

spring2maz commented Sep 25, 2018

Hi. @gbrehmer
persistent sessions are indeed treated equally regardless of connection status.
could you please create an Issue for discussion? we'll see what we can do from there.

@gbrehmer

This comment has been minimized.

Copy link

gbrehmer commented Sep 25, 2018

thanks for response, I will do later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.