From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [94.100.177.104]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 0AAEF46970E for ; Thu, 19 Dec 2019 17:45:11 +0300 (MSK) References: <20191219134126.25839-1-arkholga@tarantool.org> <20191219141445.GA26708@atlas> From: Olga Arkhangelskaia Message-ID: <4350dbf5-6fcd-634b-fc28-6eb2df288ca0@tarantool.org> Date: Thu, 19 Dec 2019 17:45:10 +0300 MIME-Version: 1.0 In-Reply-To: <20191219141445.GA26708@atlas> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-US Subject: Re: [Tarantool-patches] [PATCH v3] memtx: fix out of memory handling for rtree List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Konstantin Osipov , tarantool-patches@dev.tarantool.org On 19/12/2019 17:14, Konstantin Osipov wrote: > * Olga Arkhangelskaia [19/12/19 16:44]: >> When tarantool tries to recover rtree from a snapshot and memtx_memory value >> is lower than it has been when the snapshot was created, server suffers from >> segmentation fault. This happens because there is no out of memory error >> handling in rtree lib. In another words, we do not check the result of >> malloc operation. >> The execution flow in case of recovery uses different way and the secondary >> keys are build in batches. There is no separate implementations for rtree of >> build_next function. And the way with no checks and reservation is used >> (insex_replace). The patch adds memtx_rtree_build_next implementation to >> check and insert keys. Although this gives us no additional optimization as in >> case of memtx_tree, the memory reservation prevents tarantool from segmentation >> fault. >> If there is not enough memory to be reserved - server will fail >> gently with the "Failed to allocate" error message. > Why not add memtx_index_extent_reserve() to > generic_index_build_next(), this would be a shorter version which > does the same thing? > > I can't agree - everything that has memtx_ should be in memtx, because generic is used by vinyl, and sysview. And the only way I see do it generic - is to use index_reserve? Did you mean index_reserve, that hasĀ  memtx_index_extent_reserve( inside? In this case it is not shorter.