Skip to content

Commit

Permalink
fix(*) use dedicated shm for rate-limiting plugins
Browse files Browse the repository at this point in the history
Addresses the issue discussed in #3124 and #3241. This is part of a
series of fixes to address those errors.

Context
-------
In the `local` mode of the rate-limiting plugins, storing the
rate-limiting counters in the same shm used by Kong's database cache is
too invasive for the underlying shm, especially when the rate-limiting
plugins are used with a `seconds` precision.

On top of exhausting the database cache slots, this approach also
generates some form of fragmentation in the shm. This is due to the
side-by-side storage of values with sizes of different orders of
magnitude (JSON strings vs. an incremented double) and the LRU eviction
mechanism. When the shm is full and LRU kicks-in, it is highly probable
that several rate-limiting counters will be evicted (due to their
proliferation), thus not freeing enough space to store the retrieved
data, causing a `no memory` error to be reported by the shm.

Solution
--------
Declaring shms that are only used by some plugins is not very elegant.
Now, all users (even those not using rate-limiting plugins) have to pay
a memory cost (although small).
Unfortunately, and in the absence of a more dynamic solution to shm
configuration such as a more dynamic templating engine, or a
`configure_by_lua` phase, this is the safest solution.

Size rationale
--------------
Running a script generating similar keys and storing similar values
(double) indicates that an shm with 12Mb should be able to store about
~48,000 of those values at once. It is important to remind ourselves
that one Consumer/IP address might use more than one key (in fact, one
per period configured on the plugin), and both the rate-limiting and
response-ratelimiting plugins at once, and they use the same shms.

Even considering the above statements, ~48,000 keys per node seems
somewhat reasonable, considering keys of `second` precision will most
likely fill up the shm and be candidates for LRU eviction. Our concern
lies instead around long-lived limits (and thus, keys) set by the user.

Additionally, a future improvement upon this will be the setting of the
`init_ttl` argument for the rate-limiting keys, which will help **quite
considerably** in reducing the footprint of the plugins on the shm. As
of this day, this feature has been contributed to ngx_lua but not
released yet:

    openresty/lua-nginx-module#1226

Again, this limit only applies when using the **local** strategy, which
also likely means that a load-balancer is distributing traffic to a pool
of Kong nodes with some sort of consistent load-balancing technique.
Thus considerably reducing the number of concurrent Consumers a given
node needs to handle at once.

See also
--------
Another piece of the fixes for the `no memory` errors resides in the
behavior of the database caching module upon a full shm. See:

    thibaultcha/lua-resty-mlcache#41

This patch reduces the likeliness of a full shm (by a lot!), but does
not remove it. The above patch ensures a somewhat still sane behavior
would the shm happen to be full again.

Fix #3124
Fix #3241
  • Loading branch information
thibaultcha committed Mar 20, 2018
1 parent d6f7904 commit e462456
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 2 deletions.
1 change: 1 addition & 0 deletions kong/constants.lua
Original file line number Diff line number Diff line change
Expand Up @@ -73,5 +73,6 @@ return {
"kong_process_events",
"kong_cluster_events",
"kong_healthchecks",
"kong_rate_limiting_counters",
},
}
2 changes: 1 addition & 1 deletion kong/plugins/rate-limiting/policies/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ local redis = require "resty.redis"
local policy_cluster = require "kong.plugins.rate-limiting.policies.cluster"
local reports = require "kong.core.reports"
local ngx_log = ngx.log
local shm = ngx.shared.kong_cache
local shm = ngx.shared.kong_rate_limiting_counters

local pairs = pairs
local fmt = string.format
Expand Down
2 changes: 1 addition & 1 deletion kong/plugins/response-ratelimiting/policies/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ local redis = require "resty.redis"
local policy_cluster = require "kong.plugins.response-ratelimiting.policies.cluster"
local reports = require "kong.core.reports"
local ngx_log = ngx.log
local shm = ngx.shared.kong_cache
local shm = ngx.shared.kong_rate_limiting_counters

local pairs = pairs
local fmt = string.format
Expand Down
1 change: 1 addition & 0 deletions kong/templates/nginx_kong.lua
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ lua_shared_dict kong_cache ${{MEM_CACHE_SIZE}};
lua_shared_dict kong_process_events 5m;
lua_shared_dict kong_cluster_events 5m;
lua_shared_dict kong_healthchecks 5m;
lua_shared_dict kong_rate_limiting_counters 12m;
> if database == "cassandra" then
lua_shared_dict kong_cassandra 5m;
> end
Expand Down
1 change: 1 addition & 0 deletions spec/fixtures/custom_nginx.template
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ http {
lua_shared_dict kong_process_events 5m;
lua_shared_dict kong_cluster_events 5m;
lua_shared_dict kong_healthchecks 5m;
lua_shared_dict kong_rate_limiting_counters 12m;
> if database == "cassandra" then
lua_shared_dict kong_cassandra 5m;
> end
Expand Down

0 comments on commit e462456

Please sign in to comment.