From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-f195.google.com (mail-lj1-f195.google.com [209.85.208.195]) (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 727A4445320 for ; Mon, 13 Jul 2020 12:02:08 +0300 (MSK) Received: by mail-lj1-f195.google.com with SMTP id d17so16399127ljl.3 for ; Mon, 13 Jul 2020 02:02:08 -0700 (PDT) Date: Mon, 13 Jul 2020 12:02:05 +0300 From: Cyrill Gorcunov Message-ID: <20200713090205.GE296695@grain> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH 1/1] recovery: handle local sync txns during recovery List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org On Sat, Jul 11, 2020 at 07:05:19PM +0200, Vladislav Shpilevoy wrote: > Recovery uses txn_commit_async() so as not to block the recovery > process when a synchronous transaction is met. They are either > committed later when CONFIRM is read, or stay in the limbo after > recovery. > > However txn_commit_async() assumed it is used for remote > transactions only, and had some assertions about that. One of them > crashed in case master restarted and had any synchronous > transaction in WAL. > > The patch makes txn_commit_async() not assume anything about > transaction's origin. > > Closes #5163 I might be missing something due to lack of strong understanding of our current qsync implementation. Still the patch looks reasonable and simple enough. Thus Reviewed-by: Cyrill Gorcunov