From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f66.google.com (mail-lf1-f66.google.com [209.85.167.66]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id A3E4346970E for ; Thu, 19 Dec 2019 17:20:09 +0300 (MSK) Received: by mail-lf1-f66.google.com with SMTP id v201so4450498lfa.11 for ; Thu, 19 Dec 2019 06:20:09 -0800 (PST) Date: Thu, 19 Dec 2019 17:14:45 +0300 From: Konstantin Osipov Message-ID: <20191219141445.GA26708@atlas> References: <20191219134126.25839-1-arkholga@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191219134126.25839-1-arkholga@tarantool.org> 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: Olga Arkhangelskaia Cc: tarantool-patches@dev.tarantool.org * 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? -- Konstantin Osipov, Moscow, Russia