From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 13 May 2019 13:21:21 +0300 From: Vladimir Davydov Subject: Re: [PATCH] box/memtx: Allow to skip tuple memory from coredump Message-ID: <20190513102121.q5twlgfemojneiqm@esperanza> References: <20190507172611.8085-1-gorcunov@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190507172611.8085-1-gorcunov@gmail.com> To: Cyrill Gorcunov Cc: tml , Alexander Turenko List-ID: [Cc += Alexander] I don't quite like the name: memtx_memory_dontdump. We might want to extend it to work on vinyl memory as well one day - I don't think we'd want to introduce another cfg parameter for that. What about strip_core? Alexander, what do you think about the new configuration parameter name? Do you have a better name in mind? Cyrill, please see a few comments inline. On Tue, May 07, 2019 at 08:26:11PM +0300, Cyrill Gorcunov wrote: > In case if there are huge amount of tuples the whole > memory goes to coredump file even if we don't need it > for problem investigation. In result coredump may > blow up to gigabytes in size. > > Lets allow to exclude this memory from dumping via > box.cfg::memtx_memory_dontdump boolean parameter. > > Note that the tuple's arena is used not only for tuples > themselves but for memtx->index_extent_pool and > memtx->iterator_pool as well, so they are affected > too. > > Fixes #3509 Please write a doc bot request in the end of the commit message (after Fixes ...). Take a look at this for a reference: https://github.com/tarantool/tarantool/commit/22db9c264c9119d19a3130e6ad8e7fded0e16c6d > --- > https://github.com/tarantool/tarantool/issues/3509 > > Note this patch depends on small engine extension I've sent > earlier which is not yet merged thus for review only. > > src/box/box.cc | 5 +++++ > src/box/lua/load_cfg.lua | 2 ++ > src/box/memtx_engine.c | 4 ++-- > src/box/memtx_engine.h | 9 ++++++--- > src/box/tuple.c | 4 ++-- > src/box/tuple.h | 2 +- > src/box/vy_mem.c | 2 +- > 7 files changed, 19 insertions(+), 9 deletions(-) Please run all tests. I think box/cfg will break after this. > > diff --git a/src/box/box.cc b/src/box/box.cc > index 7828f575b..e66b5200c 100644 > --- a/src/box/box.cc > +++ b/src/box/box.cc > @@ -1670,6 +1670,10 @@ box_free(void) > static void > engine_init() > { > + uint32_t slab_flags = SLAB_ARENA_PRIVATE; > + if (cfg_geti("memtx_memory_dontdump")) > + slab_flags |= SLAB_ARENA_DONTDUMP; > + Interpreting dontdump option here and converting it to slab_flags kinda breaks encapsulation. I'd pass cfg parameter to memtx_engine_new and let it interpret it. > /* > * Sic: order is important here, since > * memtx must be the first to participate > @@ -1681,6 +1685,7 @@ engine_init() > cfg_geti("force_recovery"), > cfg_getd("memtx_memory"), > cfg_geti("memtx_min_tuple_size"), > + slab_flags, > cfg_getd("slab_alloc_factor")); > engine_register((struct engine *)memtx); > box_set_memtx_max_tuple_size(); > diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua > index 5530b2caa..8d4073539 100644 > --- a/src/box/lua/load_cfg.lua > +++ b/src/box/lua/load_cfg.lua > @@ -25,6 +25,7 @@ end > local default_cfg = { > listen = nil, > memtx_memory = 256 * 1024 *1024, > + memtx_memory_dontdump = false, > memtx_min_tuple_size = 16, > memtx_max_tuple_size = 1024 * 1024, > slab_alloc_factor = 1.05, > @@ -88,6 +89,7 @@ local default_cfg = { > local template_cfg = { > listen = 'string, number', > memtx_memory = 'number', > + memtx_memory_dontdump = 'boolean', > memtx_min_tuple_size = 'number', > memtx_max_tuple_size = 'number', > slab_alloc_factor = 'number', > diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c > index 210f3c22c..ff70cd98f 100644 > --- a/src/box/memtx_engine.c > +++ b/src/box/memtx_engine.c > @@ -1011,7 +1011,7 @@ memtx_engine_gc_f(va_list va) > struct memtx_engine * > memtx_engine_new(const char *snap_dirname, bool force_recovery, > uint64_t tuple_arena_max_size, uint32_t objsize_min, > - float alloc_factor) > + uint32_t slab_flags, float alloc_factor) > { > struct memtx_engine *memtx = calloc(1, sizeof(*memtx)); > if (memtx == NULL) { > @@ -1066,7 +1066,7 @@ memtx_engine_new(const char *snap_dirname, bool force_recovery, > /* Initialize tuple allocator. */ > quota_init(&memtx->quota, tuple_arena_max_size); > tuple_arena_create(&memtx->arena, &memtx->quota, tuple_arena_max_size, > - SLAB_SIZE, "memtx"); > + SLAB_SIZE, slab_flags, "memtx"); > slab_cache_create(&memtx->slab_cache, &memtx->arena); > small_alloc_create(&memtx->alloc, &memtx->slab_cache, > objsize_min, alloc_factor); > diff --git a/src/box/memtx_engine.h b/src/box/memtx_engine.h > index 8f4ce7cdd..0546924a4 100644 > --- a/src/box/memtx_engine.h > +++ b/src/box/memtx_engine.h > @@ -189,7 +189,8 @@ memtx_engine_schedule_gc(struct memtx_engine *memtx, > struct memtx_engine * > memtx_engine_new(const char *snap_dirname, bool force_recovery, > uint64_t tuple_arena_max_size, > - uint32_t objsize_min, float alloc_factor); > + uint32_t objsize_min, uint32_t slab_flags, > + float alloc_factor); > > int > memtx_engine_recover_snapshot(struct memtx_engine *memtx, > @@ -257,12 +258,14 @@ memtx_index_def_change_requires_rebuild(struct index *index, > static inline struct memtx_engine * > memtx_engine_new_xc(const char *snap_dirname, bool force_recovery, > uint64_t tuple_arena_max_size, > - uint32_t objsize_min, float alloc_factor) > + uint32_t objsize_min, uint32_t slab_flags, > + float alloc_factor) > { > struct memtx_engine *memtx; > memtx = memtx_engine_new(snap_dirname, force_recovery, > tuple_arena_max_size, > - objsize_min, alloc_factor); > + objsize_min, slab_flags, > + alloc_factor); > if (memtx == NULL) > diag_raise(); > return memtx; > diff --git a/src/box/tuple.c b/src/box/tuple.c > index c325f58c9..b2640dc83 100644 > --- a/src/box/tuple.c > +++ b/src/box/tuple.c > @@ -324,7 +324,7 @@ tuple_init(field_name_hash_f hash) > void > tuple_arena_create(struct slab_arena *arena, struct quota *quota, > uint64_t arena_max_size, uint32_t slab_size, > - const char *arena_name) > + uint32_t slab_flags, const char *arena_name) > { > /* > * Ensure that quota is a multiple of slab_size, to > @@ -336,7 +336,7 @@ tuple_arena_create(struct slab_arena *arena, struct quota *quota, > arena_name); > > if (slab_arena_create(arena, quota, prealloc, slab_size, > - MAP_PRIVATE) != 0) { > + slab_flags) != 0) { > if (errno == ENOMEM) { > panic("failed to preallocate %zu bytes: Cannot "\ > "allocate memory, check option '%s_memory' in box.cfg(..)", prealloc, > diff --git a/src/box/tuple.h b/src/box/tuple.h > index d261d4cc9..ec579e95e 100644 > --- a/src/box/tuple.h > +++ b/src/box/tuple.h > @@ -69,7 +69,7 @@ tuple_free(void); > void > tuple_arena_create(struct slab_arena *arena, struct quota *quota, > uint64_t arena_max_size, uint32_t slab_size, > - const char *arena_name); > + uint32_t slab_flags, const char *arena_name); > > void > tuple_arena_destroy(struct slab_arena *arena); > diff --git a/src/box/vy_mem.c b/src/box/vy_mem.c > index a4fae26e2..de893858a 100644 > --- a/src/box/vy_mem.c > +++ b/src/box/vy_mem.c > @@ -54,7 +54,7 @@ vy_mem_env_create(struct vy_mem_env *env, size_t memory) > /* Vinyl memory is limited by vy_quota. */ > quota_init(&env->quota, QUOTA_MAX); > tuple_arena_create(&env->arena, &env->quota, memory, > - SLAB_SIZE, "vinyl"); > + SLAB_SIZE, SLAB_ARENA_PRIVATE, "vinyl"); I think it's safe to skip dumping this arena, too. > lsregion_create(&env->allocator, &env->arena); > env->tree_extent_size = 0; > } > -- > 2.20.1 >