[Tarantool-patches] [PATCH v6] memtx: fix out of memory handling for rtree
Sergey Ostanevich
sergos at tarantool.org
Tue Feb 18 15:46:12 MSK 2020
Hi!
Thanks for the patch!
I wonder why do you use additional by-ref argument to pass an integer
result instead of redefining the interface into:
int
rtree_insert(struct rtree *tree, struct rtree_rect *rect, record_t obj)
instead, so that all uses of a form:
int err = 0;
...
rtree_insert(&index->tree, &rect, new_tuple, &err);
if (err == 1) {
diag_set(OutOfMemory, sizeof(struct rtree),
"rtree", "rtree_page");
return -1;
}
into a much simpler:
if (rtree_insert(&index->tree, &rect, new_tuple, &err)) {
diag_set(OutOfMemory, sizeof(struct rtree),
"rtree", "rtree_page");
return -1;
}
Sergos
On 11 Feb 15:42, Nikita Pettik wrote:
> On 11 Feb 13:51, Olga Arkhangelskaia wrote:
> > Hi Nikita! Thanks for the review.
> >
> > I have fixed everything you have pointed out. Diff can be found below.
>
> LGTM
>
> > 10.02.2020 14:18, Nikita Pettik пишет:
> > > On 10 Feb 10:47, Olga Arkhangelskaia wrote:
> > diff --git a/src/box/index.cc b/src/box/index.cc
> > -index 4e4867118..c2fc00867 100644
> > +index 4e4867118..750838bef 100644
> > --- a/src/box/index.cc
> > +++ b/src/box/index.cc
> > -@@ -733,6 +733,13 @@ int
> > +@@ -733,6 +733,8 @@ int
> > generic_index_build_next(struct index *index, struct tuple *tuple)
> > {
> > struct tuple *unused;
> > -+ /*
> > -+ * Note this is not no-op call in case of rtee index:
> > -+ * reserving 0 bytes is required during rtree recovery.
> > -+ * For details see memtx_rtree_index_reserve().
> > -+ */
> > + if (index_reserve(index, 0) != 0)
> > + return -1;
> >
> > diff --git a/src/box/memtx_rtree.c b/src/box/memtx_rtree.c
> > -index 8badad797..612fcb2a9 100644
> > +index 8badad797..a5cae8f45 100644
> > --- a/src/box/memtx_rtree.c
> > +++ b/src/box/memtx_rtree.c
> > -@@ -242,6 +242,24 @@ memtx_rtree_index_replace(struct index *base, struct
> > tuple *old_tuple,
> > +@@ -242,6 +242,23 @@ memtx_rtree_index_replace(struct index *base, struct
> > tuple *old_tuple,
> > return 0;
> > }
> >
> > @@ -82,14 +109,13 @@
> > +memtx_rtree_index_reserve(struct index *base, uint32_t size_hint)
> > +{
> > + /*
> > -+ * In case of rtree we use reserve to make sure that
> > -+ * memory allocation will not fail during any operation
> > -+ * on rtree, because there is no error handling in the
> > -+ * rtree lib.
> > -+ */
> > ++ * In case of rtree we use reserve to make sure that memory
> > allocation
> > ++ * will not fail durig any operationin rtree, because there is no
> > ++ * error handling in rtree lib.
> > ++ */
> > + (void)size_hint;
> > + ERROR_INJECT(ERRINJ_INDEX_RESERVE, {
> > -+ diag_set(OutOfMemory, MEMTX_EXTENT_SIZE, "mempool", "new
> > slab");
> > ++ diag_set(OutOfMemory, MEMTX_EXTENT_SIZE, "slab allocator",
> > "memtx extent");
> > + return -1;
> > + });
> > + struct memtx_engine *memtx = (struct memtx_engine *)base->engine;
> >
> > diff --git a/test/box/errinj.test.lua b/test/box/errinj.test.lua
> > -index 03c088677..63c8b19fe 100644
> > +index 03c088677..598e7da33 100644
> > --- a/test/box/errinj.test.lua
> > +++ b/test/box/errinj.test.lua
> > -@@ -620,3 +620,31 @@ box.error.injection.set('ERRINJ_RELAY_TIMEOUT', 0.5)
> > +@@ -620,3 +620,30 @@ box.error.injection.set('ERRINJ_RELAY_TIMEOUT', 0.5)
> > box.error.injection.get('ERRINJ_RELAY_TIMEOUT')
> > box.error.injection.set('ERRINJ_RELAY_TIMEOUT', 0)
> > box.error.injection.get('ERRINJ_RELAY_TIMEOUT')
> > +
> > +--
> > -+-- gh-4619: make sure that if OOM takes place during rtree recovery,
> > -+-- Tarantool instance will fail gracefully.
> > ++-- gh-4619-RTREE-doesn't-handle-OOM-properly
> > +--
> > +test_run:cmd('create server rtree with script = "box/lua/cfg_rtree.lua"')
> > +test_run:cmd("start server rtree")
More information about the Tarantool-patches
mailing list