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

Possible race condition using emqttd_cm #486

Closed
obi458 opened this issue Mar 30, 2016 · 9 comments
Closed

Possible race condition using emqttd_cm #486

obi458 opened this issue Mar 30, 2016 · 9 comments
Assignees
Milestone

Comments

@obi458
Copy link

obi458 commented Mar 30, 2016

  1. register using cast(async)
  2. lookup is sync

So it is possible that the client is being registered(async) while for example
on_client_subscribe(ClientId, TopicTable, _Env), i need the mqtt_client record,
emqttd_cm:lookup(ClientId)(sync) gives undefined!

@emqplus
Copy link
Contributor

emqplus commented Mar 30, 2016

@obi458, Agree that there is a possible race condition issue. I will check the design later. But notice that you should not lookup the 'mqtt_client' record in on_client_subscribe for the function is called by session process. The session and client may be on different nodes in a cluster.

@emqplus emqplus modified the milestones: 0.17.1, 1.0 Mar 30, 2016
@emqplus emqplus self-assigned this Mar 30, 2016
@obi458
Copy link
Author

obi458 commented Mar 30, 2016

Ok, i have only "bridge cluster", but in an "erlang cluster" this could be a problem with the
sessions, i've only the clientid and can't get the mqtt_client record.

I think a "global" MQTT Client Manager should solve it!

@emqplus
Copy link
Contributor

emqplus commented Mar 30, 2016

Hi @obi458, please checkout the latest '0.17' branch and verify this issue. Thanks.

@obi458
Copy link
Author

obi458 commented Mar 31, 2016

Works, the 120 seconds timeout is your experience or a pessimistic value?

@emqplus
Copy link
Contributor

emqplus commented Apr 1, 2016

The timeout is a tradeoff to ensure the broker could handle about 1k+ TCP connections per second.

@obi458, would you like to benchmark the fix? The tool: https://github.com/emqtt/emqtt_benchmark

./emqtt_bench_sub -h server -p 1883 -c 50000 -i 1 -t test/%i -q 1

@obi458
Copy link
Author

obi458 commented Apr 1, 2016

Processing....
BTW, the documentation for tuning is for linux, missing for OS X!

@obi458
Copy link
Author

obi458 commented Apr 1, 2016

Dashboard_50000_171.pdf

emqplus pushed a commit that referenced this issue Apr 3, 2016
1.0 - update design guide and fix issue #486
@emqplus
Copy link
Contributor

emqplus commented Apr 13, 2016

Hi @obi458, could we close the issue now?

@emqplus emqplus closed this as completed Apr 13, 2016
@obi458
Copy link
Author

obi458 commented Apr 13, 2016

Closed!

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

No branches or pull requests

2 participants