From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-f196.google.com (mail-lj1-f196.google.com [209.85.208.196]) (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 6443E469719 for ; Sat, 15 Feb 2020 21:51:51 +0300 (MSK) Received: by mail-lj1-f196.google.com with SMTP id d10so14329858ljl.9 for ; Sat, 15 Feb 2020 10:51:51 -0800 (PST) Date: Sat, 15 Feb 2020 21:51:49 +0300 From: Konstantin Osipov Message-ID: <20200215185149.GF19902@atlas> References: <20200214140339.4085-1-gorcunov@gmail.com> <20200214140339.4085-4-gorcunov@gmail.com> <20200215173741.GC19902@atlas> <20200215182459.GD2527@uranus> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200215182459.GD2527@uranus> Subject: Re: [Tarantool-patches] [PATCH v10 3/4] box/applier: prevent nil dereference on applier rollback List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov Cc: tml * Cyrill Gorcunov [20/02/15 21:26]: > > It would be nice to explain in the comment why you want to > > preserve the original error in the fiber here: because when later > > this fiber is joined in (add call site here), we may want to check > > its diagnostics area. > > You know I think we might need to drop replicaset.diag completely > and use applier.diag instead. I don't mind, but that could be too intrusive for a bug fix. I still don't see why you can't patch the crash site - i.e. the place which joins the fiber with broken diag? Let me remind you that this is just a regression from the parallel applier patch, and not a big one - just a coding bug. So let's fix the coding bug first and refactor the patch second. > understand this code yet, so I need more time to come with some > clean solution (which I plan to implement on the top). > > > > > > /* Broadcast the rollback event across all appliers. */ > > > trigger_run(&replicaset.applier.on_rollback, event); > > > /* Rollback applier vclock to the committed one. */ -- Konstantin Osipov, Moscow, Russia