<HTML><BODY><div><div>Hi!</div><div> </div><div>Answers below.</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">Воскресенье, 20 декабря 2020, 19:13 +03:00 от Vladislav Shpilevoy <v.shpilevoy@tarantool.org>:<br> <div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_16084808171336135662_BODY">> > diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c<br>> > index 73b4c450eb..cc431ea816 100644<br>> > --- a/src/box/memtx_space.c<br>> > +++ b/src/box/memtx_space.c<br>> > @@ -327,8 +327,9 @@ memtx_space_execute_replace(struct space *space, struct txn *txn,<br>> > struct memtx_space *memtx_space = (struct memtx_space *)space;<br>> > struct txn_stmt *stmt = txn_current_stmt(txn);<br>> > enum dup_replace_mode mode = dup_replace_mode(request->type);<br>> > - stmt->new_tuple = memtx_tuple_new(space->format, request->tuple,<br>> > - request->tuple_end);<br>> > + stmt->new_tuple = space->format->vtab.tuple_new(space->format,<br>> > + request->tuple,<br>> > + request->tuple_end);<br>><br>> 2. Seems like an expensive change. You added +2 pointer<br>> dereferences to a hot path.<br>><br>> Last time when you worked on that I proposed to make space_truncate<br>> use box.begin + quota disable + box_upsert + quota enable + commit.<br>> So there are no yields between quota enable and disable. And no changes<br>> in the code not related to truncation. Why didn't it work?<br>><br>> That does work, but then it was also discussed that we should use only<br>> huge slabs on disabled quota, so that quota can shrink back when freed.<br><br>Such discussion results are far from something trivial and obvious. Please,<br>specify that in the commit message and in the comments if possible.</div></div></div></div></blockquote></div><div>Right, i will repush with more info on this.</div><div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div><br>Also I don't understand why can't you do that on the slab arena<br>level. Start allocating only 'huge' slabs if the quota is full, but<br>disabled.</div></div></div></div></blockquote></div><div>Yes, allocation in case of out of quota can be done on more</div><div>internal level.</div><div>But the problem is exactly in freeing you mentioned further.</div><div>We need to free in the right way: If we are allocating with</div><div>mempool, we need to use mempool_find() properly, while</div><div>if we are allocating ‘huge’ slab, we need to free it accordingly.</div><div>So the information about the allocation has to be stored</div><div>somehow in case we are going to alter it depending on the</div><div>current state. I don’t see convenient place where we can store</div><div>it, looks like it will be even worse to store such information</div><div>in struct tuple itself.</div><div>This leads to the solution, where allocation and deletion can</div><div>only being altered per space, thus all the tuples in _truncate</div><div>space are being allocated through malloc() and freed accordingly.</div><div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div><br>> It also requires specific freeing for those tuples, that is why both new() and<br>> delete() are now used from vtab, which has specific version for truncate space.<br><br>Why do you need special freeing? memtx_tuple_delete() does not allocate<br>anything.</div></div></div></div></blockquote></div><div>Commented above.</div><div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div><br>> There is an option to use only specific delete() from vtab and for allocation<br>> patch space_truncate() instead of new(), although it looks more strange.<br><br>Didn't understand what you mean.</div></div></div></div></blockquote></div><div>I mean that allocation and deletion is now altered for _truncate space.</div><div>Technically, we don’t to change the way we allocate on smalloc() and</div><div>tuple_new() levels. We can patch space_truncate() instead. But we will</div><div>still need to patch tuple_delete() for the correct freeing. This way doesn’t</div><div>look good.</div><div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div><br>><br>><br>> > if (stmt->new_tuple == NULL)<br>> > return -1;<br>> > tuple_ref(stmt->new_tuple);<br>><br>> <br>> <br>> --<br>> Ilya Kosarev<br>> </div></div></div></div></blockquote> <div> </div><div data-signature-widget="container"><div data-signature-widget="content"><div>--<br>Ilya Kosarev</div></div></div><div> </div></div></BODY></HTML>