Skip to content

Commit

Permalink
memcached: use own slab arena
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ligurio committed Apr 6, 2022
1 parent 0bf8fda commit f92475c
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 10 deletions.
11 changes: 11 additions & 0 deletions memcached/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ memcached_get_stat (struct memcached_service *);
struct memcached_service *
memcached_create(const char *, uint32_t);

void memcached_slab_arena_create();

void memcached_slab_cache_create();

int memcached_start (struct memcached_service *);
void memcached_stop (struct memcached_service *);
void memcached_free (struct memcached_service *);
Expand Down Expand Up @@ -386,6 +390,13 @@ local function memcached_get(name)
return memcached_services[name]
end

local function memcached_init()
memcached_internal.memcached_slab_arena_create()
memcached_internal.memcached_slab_cache_create()
end

memcached_init()

return {
create = memcached_create_instance,
get = memcached_get,
Expand Down
56 changes: 56 additions & 0 deletions memcached/internal/alloc.c
Original file line number Diff line number Diff line change
@@ -1,2 +1,58 @@
#include <tarantool/module.h>
#include <small/slab_cache.h>
#include <small/quota.h>

static struct slab_arena arena;
static struct slab_cache slabc;
static struct quota quota;

void
memcached_slab_arena_create()
{
static __thread bool inited = false;
if (inited) return;
size_t quota_size = QUOTA_MAX;
quota_init(&quota, quota_size);
/* Parameters are the same as in src/lib/core/memory.c:memory_init() */
const size_t SLAB_SIZE = 4 * 1024 * 1024;
slab_arena_create(&arena, &quota, 0, SLAB_SIZE, SLAB_ARENA_PRIVATE);
say_info("creating arena with %zu bytes...", quota_size);
inited = 1;
}

void
memcached_slab_arena_destroy()
{
static __thread bool freed = false;
if (freed) return;
say_info("destroying arena...");
slab_arena_destroy(&arena);
freed = 1;
}

void
memcached_slab_cache_create()
{
static __thread bool inited = false;
if (inited) return;
slab_cache_set_thread(&slabc);
slab_cache_create(&slabc, &arena);
say_info("allocate slab cache with slab size %u", arena.slab_size);
inited = 1;
}

void
memcached_slab_cache_destroy()
{
static __thread bool freed = false;
if (freed) return;
say_info("destroying slab cache");
slab_cache_destroy(&slabc);
freed = 1;
}

struct slab_cache *
memcached_slab_cache()
{
return &slabc;
}
15 changes: 15 additions & 0 deletions memcached/internal/alloc.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,19 @@
#ifndef ALLOC_H_INCLUDED
#define ALLOC_H_INCLUDED

void
memcached_slab_arena_create();

void
memcached_slab_arena_destroy();

void
memcached_slab_cache_create();

void
memcached_slab_cache_destroy();

struct slab_cache *
memcached_slab_cache();

#endif /* ALLOC_H_INCLUDED */
9 changes: 8 additions & 1 deletion memcached/internal/memcached.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include <small/ibuf.h>
#include <small/obuf.h>

#include "alloc.h"
#include "memcached.h"
#include "memcached_layer.h"
#include "error.h"
Expand All @@ -46,6 +47,12 @@
#include "expiration.h"
#include "mc_sasl.h"

__attribute__((destructor)) static void
destroy_allocations() {
memcached_slab_cache_destroy();
memcached_slab_arena_destroy();
}

static inline int
memcached_skip_request(struct memcached_connection *con) {
struct ibuf *in = con->in;
Expand Down Expand Up @@ -250,7 +257,7 @@ memcached_handler(struct memcached_service *p, int fd)
assert(0); /* unreacheable */
}

region_create(&con.gc, cord_slab_cache());
region_create(&con.gc, memcached_slab_cache());

/* read-write cycle */
con.cfg->stat.curr_conns++;
Expand Down
9 changes: 5 additions & 4 deletions memcached/internal/network.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <small/ibuf.h>
#include <small/obuf.h>

#include "alloc.h"
#include "memcached.h"
#include "constants.h"
#include "network.h"
Expand All @@ -31,8 +32,8 @@ iobuf_mempool_create()
{
static __thread bool inited = false;
if (inited) return;
mempool_create(&ibuf_pool, cord_slab_cache(), sizeof(struct ibuf));
mempool_create(&obuf_pool, cord_slab_cache(), sizeof(struct obuf));
mempool_create(&ibuf_pool, memcached_slab_cache(), sizeof(struct ibuf));
mempool_create(&obuf_pool, memcached_slab_cache(), sizeof(struct obuf));
inited = 1;
}

Expand All @@ -48,7 +49,7 @@ ibuf_new()
{
void *ibuf = mempool_alloc(&ibuf_pool);
if (ibuf == NULL) return NULL;
ibuf_create((struct ibuf *)ibuf, cord_slab_cache(), iobuf_readahead);
ibuf_create((struct ibuf *)ibuf, memcached_slab_cache(), iobuf_readahead);
return ibuf;
}

Expand All @@ -57,7 +58,7 @@ obuf_new()
{
void *obuf = mempool_alloc(&obuf_pool);
if (obuf == NULL) return NULL;
obuf_create((struct obuf *)obuf, cord_slab_cache(), iobuf_readahead);
obuf_create((struct obuf *)obuf, memcached_slab_cache(), iobuf_readahead);
return obuf;
}

Expand Down
34 changes: 29 additions & 5 deletions test/common/module_api.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,36 @@ package.cpath = './?.so;' .. package.cpath
local memcached = require('memcached')
local test = tap.test('memcached module api')

local function is_port_open(port)
local sock, _ = socket.tcp_connect('127.0.0.1', port)
-- 1. Open connection to memcached instance.
-- 2. Set a key 'key' to value 5.
-- 3. Get a value of key 'key'.
-- 4. Make sure value is equal to 5.
-- 5. Close connection.
local function check_memcached(port, timeout)
timeout = timeout or 0.5
local sock, err = socket.tcp_connect('127.0.0.1', port, timeout)
if sock == nil then
print(err)
return false
end
return true
sock:nonblock(true)

-- Set a value.
local cmd = 'set key 0 60 5\r\nvalue\r\n'
local send = sock:send(cmd)
assert(send > 0)

-- Get a value.
send = sock:send('get key\r\n')
assert(send > 0)
local read = sock:read(23)
local v = read:match('VALUE key 0 ([0-9])')
assert(v == '5')

-- Close connection.
sock:close()

return tonumber(v) == 5
end

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

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

-- memcached.server with created and started instances
Expand Down

0 comments on commit f92475c

Please sign in to comment.