From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 30 May 2019 11:42:36 +0300 From: Vladimir Davydov Subject: Re: [PATCH 0/5] Hand over key lookup in a page to vinyl reader thread Message-ID: <20190530084236.vnigc6vufmyl45o6@esperanza> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: To: tarantool-patches@freelists.org List-ID: On Wed, May 29, 2019 at 06:12:46PM +0300, Vladimir Davydov wrote: > Vladimir Davydov (5): > vinyl: factor out function to lookup key in page > vinyl: pass page info by reference to reader thread > vinyl: encapsulate reader thread selection logic in a helper function > vinyl: do not allow to cancel a fiber reading a page > vinyl: lookup key in reader thread Pushed to master after adding fiber_testcancel and rewriting the comment to patch 4 as follows: vinyl: do not allow to cancel a fiber reading a page To handle fiber cancellation during page read we need to pin all objects referenced by vy_page_read_task. Currently, there's the only such object, vy_run. It has reference counting so pinning it is trivial. However, to move page lookup to a reader thread, we need to also reference key def, tuple format, and key. Format and key have reference counting, but key def doesn't - we typically copy it. Copying it in this case is too heavy. Actually, cancelling a fiber manually or on timeout while it's reading disk doesn't make much sense with PCIE attached flash drives. It used to be reasonable with rotating disks, since a rotating disk controller could retry reading a block indefinitely on read failure. It is still relevant to Network Attached Storage. On the other hand, NAS has never been tested, and what isn't tested, can and should be removed. For complex SQL queries we'll be forced to rethink timeout handling anyway. That being said, let's simply drop this functionality.