From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 793362F765 for ; Wed, 29 May 2019 14:24:44 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id CwUnpZxtHUKD for ; Wed, 29 May 2019 14:24:44 -0400 (EDT) Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 37A332F4D3 for ; Wed, 29 May 2019 14:24:44 -0400 (EDT) Received: by smtp3.mail.ru with esmtpa (envelope-from ) id 1hW3Fi-00010g-AK for tarantool-patches@freelists.org; Wed, 29 May 2019 21:24:42 +0300 Date: Wed, 29 May 2019 21:24:40 +0300 From: Konstantin Osipov Subject: [tarantool-patches] Re: [PATCH 3/5] vinyl: encapsulate reader thread selection logic in a helper function Message-ID: <20190529182440.GC6799@atlas> References: <82705410db4b7fa6c31be8c20366bbd3d99a83e1.1559142561.git.vdavydov.dev@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <82705410db4b7fa6c31be8c20366bbd3d99a83e1.1559142561.git.vdavydov.dev@gmail.com> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org * Vladimir Davydov [19/05/29 21:03]: > Page reading code is intermixed with the reader thread selection in the > same function, which makes it difficult to extend the former. So let's > introduce a helper function encapsulating a call on behalf of a reader > thread. > /** > + * Execute a task on behalf of a reader thread. > + * Calls free_cb on failure. > + */ > +static int > +vy_run_env_coio_call(struct vy_run_env *env, struct cbus_call_msg *msg, > + cbus_call_f func, cbus_call_f free_cb, double timeout) > +{ > + /* Optimization: use blocking I/O during WAL recovery. */ > + if (env->reader_pool == NULL) { > + if (func(msg) != 0) > + goto fail; > + return 0; > + } this would be a great api if not for this optimization. Today not involving free_cb on cbus_msg is harmless, but tomorrow may break some logic. Please consider designing an API in a way that would work with resources in the same way whether or not it is boot time or a coio call. The comment could also be extended, not reduced - e.g. explain why we consider this an optimization, when exactly we perform reads during recovery (for what statements, for example). -- Konstantin Osipov, Moscow, Russia