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

zone.external.publish_limit cannot work #1847

Closed
liulangzhe opened this issue Sep 21, 2018 · 6 comments
Assignees
Labels
BUG
Milestone

Comments

@liulangzhe
Copy link

@liulangzhe liulangzhe commented Sep 21, 2018

hi , when
## zone.external.publish_limit = 10,100
is work , bug when
zone.external.publish_limit = 10,100 is not work

Error:
CRASH REPORT Process <0.1734.0> with 0 neighbours crashed with reason: no function clause matching emqx_connection:init_limiter("10,100") line 158
18:49:24.005 [error] Supervisor 'esockd_connection_sup - <0.1588.0>' had child connection started with emqx_connection:start_link([{max_conn_rate,1000},{zone,external}]) at <0.1734.0> exit with reason no function clause matching emqx_connection:init_limiter("10,100") line 158 in context connection_crashed

this is limit my publish rate ?

@gilbertwong96

This comment has been minimized.

Copy link
Contributor

@gilbertwong96 gilbertwong96 commented Sep 21, 2018

OK, I will check it.

@gilbertwong96 gilbertwong96 self-assigned this Sep 21, 2018
@emqplus emqplus changed the title zone.external.publish_limit not work zone.external.publish_limit cannot work Sep 22, 2018
@emqplus emqplus self-assigned this Sep 22, 2018
@emqplus emqplus added the BUG label Sep 22, 2018
@emqplus emqplus added this to the 3.0-beta.3 milestone Sep 22, 2018
emqplus added a commit that referenced this issue Sep 22, 2018
Change the config of 'zone.$name.publish_limit'
@emqplus emqplus changed the title zone.external.publish_limit cannot work `zone.external.publish_limit` cannot work Sep 22, 2018
@emqplus emqplus changed the title `zone.external.publish_limit` cannot work zone.external.publish_limit cannot work Sep 22, 2018
@emqplus

This comment has been minimized.

Copy link
Contributor

@emqplus emqplus commented Sep 22, 2018

@liulangzhe Fixed in PR: #1856

emqplus added a commit that referenced this issue Sep 22, 2018
Change the config of 'zone.$name.publish_limit'
@emqplus

This comment has been minimized.

Copy link
Contributor

@emqplus emqplus commented Sep 25, 2018

@liulangzhe Please download the 3.0-beta.3 release to verify the issue: http://emqtt.io/downloads

@emqplus emqplus closed this Sep 25, 2018
@EthsonLiu

This comment has been minimized.

Copy link

@EthsonLiu EthsonLiu commented Jun 11, 2019

@emqplus

centos-7 + emqx-centos7-3.2-beta.2 问题依然存在。


然后我试了下 3.0-beta.3 (以下测试均在 qos = 0 的情况下),可以是可以,但是有一个现象,不知道是刻意还是 bug。

我配置文件改的是 1 min 5 条,即zone.external.publish_limit = 5,1m,用 mqtt.fx 测试的时候,连续发送五条,停顿一会(差不多 1s),再连续发送两条,那边显示共收到 6 条,等待 5-7s 后,第七条数据也收到了。

然后我又测试了下,连续发送 5 条,停顿 1s,再连续发送 5 条,那边显示收到 6 条,等待 5-7s,收到了剩余的 4 条。

按我的理解,既然限制了上报的频率,那么多上报的数据,应该直接被丢掉,而不是应该被保存。

我上边的测试是否属于 emqx 的正常处理逻辑?

@tradingtrace

This comment has been minimized.

Copy link
Contributor

@tradingtrace tradingtrace commented Jun 11, 2019

I'm a newcomer to EMQX, just starting to read the code. Please correct me if I have any mistakes.

For example zone.external.publish_limit = 12,1m, it will be converted to 2 variables:

  1. Rate = 0.2 ( 12 / 60), which means how many PUBLISH packets would be processed once a second, but this is not accurate.
  2. Burst = 12
  3. And there is another parameter defined in code ACTIVE_N = 100

Only TCP and SSL connections obey the publish_limit. I didn't find it in WS/WSS connections.

When a TCP/SSL connection is ready, it begins to process the received data messages from socket (a data message is not a packet, maybe a part of a packet or more than one packet). Every time It allows 100 (ACTIVE_N) data messages to pass in, which may contain PUBLISH packets or other type packets.

There is also a counter named incoming_pubs, it will increase 1 when a PUBLISH packet is processed.

The connection will check the publish limit when every 100(ACTIVE_N) data messages are processed.

For example, the incoming_pubs counter is 20 ( let us call it Tokens), and we also have Rate = 0.2, Burst = 12, and the current time is Now, and the time of the last check is Lastime, and here is the check function:

-spec(check(pos_integer(), integer(), bucket()) -> {non_neg_integer(), bucket()}).
check(Tokens, Now, Bucket = #bucket{burst   = Burst,
                                    tokens  = Remaining,
                                    rate    = Rate,
                                    lastime = Lastime}) ->
    Limit = min(Burst, Remaining + round((Rate * (Now - Lastime)) / 1000)),
    case Limit >= Tokens of
        true  -> %% Tokens available
            {0, Bucket#bucket{tokens = Limit - Tokens, lastime = Now}};
        false -> %% Tokens not enough
            Pause = round((Tokens - Remaining)*1000/Rate),
            {Pause, Bucket#bucket{tokens = 0, lastime = Now}}
    end.

If Tokens do not run out of Limit, nothing will happen. If Token is greater than Limit, the connection will just block the socket a while without dropping any packets.

And also, if you can submit more than 12(Burst) PUBLISH packets together within 100(ACTIVE_N) data messages, all of them will be processed before the next socket blocking.

@HJianBo

This comment has been minimized.

Copy link
Member

@HJianBo HJianBo commented Jun 12, 2019

@tradingtrace
Cool! your analysis is exactly correct. I think this explanation could be a standard documentation for publish_limit :)

The publish_limit can not supported on WS/WSS connection, because it implement depend on cowboy that is not provide an interface to control socket recv/send operation.

Thank you very much for your interest in this project code. Welcome to submit some contributions if it is possible!

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