Skip to content

Commit cae3fa3

Browse files
committed
memcached: use own slab arena
Historically memcached module uses slab cache created by Tarantool. Fibers get access to it via call of function cord_slab_cache() that is a part of Tarantool C API. The problem is happen when memcached and Tarantool use different versions of Small library where ABI is broken. Issues #59 and #96 are examples of such problems. We decided use independent slab arena in memcached module to avoid such problems in future and this patch implements it. Below I'll describe what is important in switching to own arena. memcached module uses different types of Small's allocators (ibuf and obuf used for every network connection, one slab cache used per fiber). There are two important things with used Small's allocators: 1. some allocators depends on other and sequence of allocations should be correct. README in small source code repository has a clear description of relationships between allocators [1]. 2. some allocators are thread-safe and others not. slab arena is shared between all memcached instances, when slab cache allocated on top of slab arena is not thread-safe and must be used in scope of fiber. memcached performs memory allocations and deallocations on different stages and I would list all these stages explicitly: - memcached module initialization - memcached module deinitialization - memcached instance gc - memcached instance initialization - memcached instance start - memcached instance stop - memcached instance deinitialization Most part of these cases covered in tests test/common/instance_api.test.lua and test/common/module_api.test.lua that were added in previous commits. The exception here is gc that was checked manually. In tests function is_port_open() has been replaced to memcached minimalistic client that do set value, get value with the same key and compare both values. This check is more rigorous than check port openess. 1. https://github.com/tarantool/small Fixes #96
1 parent c463b64 commit cae3fa3

File tree

6 files changed

+124
-10
lines changed

6 files changed

+124
-10
lines changed

memcached/init.lua

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ memcached_get_stat (struct memcached_service *);
8282
struct memcached_service *
8383
memcached_create(const char *, uint32_t);
8484

85+
void memcached_slab_arena_create();
86+
87+
void memcached_slab_cache_create();
88+
8589
int memcached_start (struct memcached_service *);
8690
void memcached_stop (struct memcached_service *);
8791
void memcached_free (struct memcached_service *);
@@ -386,6 +390,13 @@ local function memcached_get(name)
386390
return memcached_services[name]
387391
end
388392

393+
local function memcached_init()
394+
memcached_internal.memcached_slab_arena_create()
395+
memcached_internal.memcached_slab_cache_create()
396+
end
397+
398+
memcached_init()
399+
389400
return {
390401
create = memcached_create_instance,
391402
get = memcached_get,

memcached/internal/alloc.c

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,58 @@
11
#include <tarantool/module.h>
22
#include <small/slab_cache.h>
3+
#include <small/quota.h>
4+
5+
static struct slab_arena arena;
6+
static struct slab_cache slabc;
7+
static struct quota quota;
8+
9+
void
10+
memcached_slab_arena_create()
11+
{
12+
static __thread bool inited = false;
13+
if (inited) return;
14+
size_t quota_size = QUOTA_MAX;
15+
quota_init(&quota, quota_size);
16+
/* Parameters are the same as in src/lib/core/memory.c:memory_init() */
17+
const size_t SLAB_SIZE = 4 * 1024 * 1024;
18+
slab_arena_create(&arena, &quota, 0, SLAB_SIZE, SLAB_ARENA_PRIVATE);
19+
say_info("creating arena with %zu bytes...", quota_size);
20+
inited = 1;
21+
}
22+
23+
void
24+
memcached_slab_arena_destroy()
25+
{
26+
static __thread bool freed = false;
27+
if (freed) return;
28+
say_info("destroying arena...");
29+
slab_arena_destroy(&arena);
30+
freed = 1;
31+
}
32+
33+
void
34+
memcached_slab_cache_create()
35+
{
36+
static __thread bool inited = false;
37+
if (inited) return;
38+
slab_cache_set_thread(&slabc);
39+
slab_cache_create(&slabc, &arena);
40+
say_info("allocate slab cache with slab size %u", arena.slab_size);
41+
inited = 1;
42+
}
43+
44+
void
45+
memcached_slab_cache_destroy()
46+
{
47+
static __thread bool freed = false;
48+
if (freed) return;
49+
say_info("destroying slab cache");
50+
slab_cache_destroy(&slabc);
51+
freed = 1;
52+
}
53+
54+
struct slab_cache *
55+
memcached_slab_cache()
56+
{
57+
return &slabc;
58+
}

memcached/internal/alloc.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,19 @@
11
#ifndef ALLOC_H_INCLUDED
22
#define ALLOC_H_INCLUDED
33

4+
void
5+
memcached_slab_arena_create();
6+
7+
void
8+
memcached_slab_arena_destroy();
9+
10+
void
11+
memcached_slab_cache_create();
12+
13+
void
14+
memcached_slab_cache_destroy();
15+
16+
struct slab_cache *
17+
memcached_slab_cache();
18+
419
#endif /* ALLOC_H_INCLUDED */

memcached/internal/memcached.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include <small/ibuf.h>
3838
#include <small/obuf.h>
3939

40+
#include "alloc.h"
4041
#include "memcached.h"
4142
#include "memcached_layer.h"
4243
#include "error.h"
@@ -46,6 +47,12 @@
4647
#include "expiration.h"
4748
#include "mc_sasl.h"
4849

50+
__attribute__((destructor)) static void
51+
destroy_allocations() {
52+
memcached_slab_cache_destroy();
53+
memcached_slab_arena_destroy();
54+
}
55+
4956
static inline int
5057
memcached_skip_request(struct memcached_connection *con) {
5158
struct ibuf *in = con->in;
@@ -250,7 +257,7 @@ memcached_handler(struct memcached_service *p, int fd)
250257
assert(0); /* unreacheable */
251258
}
252259

253-
region_create(&con.gc, cord_slab_cache());
260+
region_create(&con.gc, memcached_slab_cache());
254261

255262
/* read-write cycle */
256263
con.cfg->stat.curr_conns++;

memcached/internal/network.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <small/ibuf.h>
1515
#include <small/obuf.h>
1616

17+
#include "alloc.h"
1718
#include "memcached.h"
1819
#include "constants.h"
1920
#include "network.h"
@@ -31,8 +32,8 @@ iobuf_mempool_create()
3132
{
3233
static __thread bool inited = false;
3334
if (inited) return;
34-
mempool_create(&ibuf_pool, cord_slab_cache(), sizeof(struct ibuf));
35-
mempool_create(&obuf_pool, cord_slab_cache(), sizeof(struct obuf));
35+
mempool_create(&ibuf_pool, memcached_slab_cache(), sizeof(struct ibuf));
36+
mempool_create(&obuf_pool, memcached_slab_cache(), sizeof(struct obuf));
3637
inited = 1;
3738
}
3839

@@ -48,7 +49,7 @@ ibuf_new()
4849
{
4950
void *ibuf = mempool_alloc(&ibuf_pool);
5051
if (ibuf == NULL) return NULL;
51-
ibuf_create((struct ibuf *)ibuf, cord_slab_cache(), iobuf_readahead);
52+
ibuf_create((struct ibuf *)ibuf, memcached_slab_cache(), iobuf_readahead);
5253
return ibuf;
5354
}
5455

@@ -57,7 +58,7 @@ obuf_new()
5758
{
5859
void *obuf = mempool_alloc(&obuf_pool);
5960
if (obuf == NULL) return NULL;
60-
obuf_create((struct obuf *)obuf, cord_slab_cache(), iobuf_readahead);
61+
obuf_create((struct obuf *)obuf, memcached_slab_cache(), iobuf_readahead);
6162
return obuf;
6263
}
6364

test/common/module_api.test.lua

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,36 @@ package.cpath = './?.so;' .. package.cpath
66
local memcached = require('memcached')
77
local test = tap.test('memcached module api')
88

9-
local function is_port_open(port)
10-
local sock, _ = socket.tcp_connect('127.0.0.1', port)
9+
-- 1. Open connection to memcached instance.
10+
-- 2. Set a key 'key' to value 5.
11+
-- 3. Get a value of key 'key'.
12+
-- 4. Make sure value is equal to 5.
13+
-- 5. Close connection.
14+
local function check_memcached(port, timeout)
15+
timeout = timeout or 0.5
16+
local sock, err = socket.tcp_connect('127.0.0.1', port, timeout)
1117
if sock == nil then
18+
print(err)
1219
return false
1320
end
14-
return sock:sysconnect(0, port) and true or false
21+
sock:nonblock(true)
22+
23+
-- Set a value.
24+
local cmd = 'set key 0 60 5\r\nvalue\r\n'
25+
local send = sock:send(cmd)
26+
assert(send > 0)
27+
28+
-- Get a value.
29+
send = sock:send('get key\r\n')
30+
assert(send > 0)
31+
local read = sock:read(23)
32+
local v = read:match('VALUE key 0 ([0-9])')
33+
assert(v == '5')
34+
35+
-- Close connection.
36+
sock:close()
37+
38+
return tonumber(v) == 5
1539
end
1640

1741
if type(box.cfg) == 'function' then
@@ -39,7 +63,7 @@ local mc_1 = memcached.create(mc_1_name, tostring(mc_1_port), {
3963
space_name = mc_1_space_name
4064
})
4165
test:isnt(mc_1, nil, '1st memcached instance object is not nil')
42-
test:is(is_port_open(mc_1_port), true, '1st memcached instance is started')
66+
test:is(check_memcached(mc_1_port), true, 'connected to 1st memcached instance')
4367
mc_1:stop()
4468

4569
-- memcached.create(): instance 2
@@ -52,7 +76,7 @@ local mc_2 = memcached.create(mc_2_name, tostring(mc_2_port), {
5276
space_name = mc_2_space_name
5377
})
5478
test:isnt(mc_2, nil, '2nd memcached instance object is not nil')
55-
test:is(is_port_open(mc_2_port), true, '2nd memcached instance is started')
79+
test:is(check_memcached(mc_2_port), true, 'connected to 2nd memcached instance')
5680
mc_2:stop()
5781

5882
-- memcached.server with created and started instances

0 commit comments

Comments
 (0)