From owner-freebsd-hackers@freebsd.org Tue Nov 21 17:38:17 2017 Return-Path: Delivered-To: freebsd-hackers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id CA9ABDF4F1F for ; Tue, 21 Nov 2017 17:38:17 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-qk0-x229.google.com (mail-qk0-x229.google.com [IPv6:2607:f8b0:400d:c09::229]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 8199A775EF for ; Tue, 21 Nov 2017 17:38:17 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: by mail-qk0-x229.google.com with SMTP id j202so13146289qke.10 for ; Tue, 21 Nov 2017 09:38:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=58f8TuzYc7OMHgUl7xnYpQXODdCK8zmGK6v6aUBGcN8=; b=a65KJnvCpXQt1BFxOSWyhmXuWijjLu+enRNoLcGm8+E6O/8ibrLpFB+xL4AUT/ITys 5zazmR47AEN6zEoqbilmyOrWRmDq+2dvyNUCCG9yd8rCQeFEMq1k7S9NLhctEyGXdczD SMp7EdlrfNZYhddX8yqUsj5Qjp+JkdN+lBQK+xQfRlhxAMEL5p5Y1IyFO6GLL7Gv1ojW neUKBCkBc0pCA0gKa3ERwAilwbPFp87zaXlrSv4BZmng82FX7Zs91I//KKY/d9V+qFbh denB+PHPT2qzS/K5jradjTN8x6QjVB6w/l/BTzlOxsXuDdgNO7s8X7PYuFzms/+zWPCc xI+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=58f8TuzYc7OMHgUl7xnYpQXODdCK8zmGK6v6aUBGcN8=; b=UXY6gufDF3a6FLAhBnhp4rxYHYJh13SXeZunWlqLzv6drwUONmZIRco815ioFTA7k9 OlrRyPj811I4WcgILGvV7CEXCHuA3TLEZCVyj7vjyALf7xdYczvoXybeVPDK4Q3dFVbR /5N9wFfnm2O1zP7pUDL8w0YRId9xOKUECWxZI5Fssdh1DKCCHyO/4LAvOGbeMVKQtuDP 1/d0IE/2EAejTuAKce06hGnO3VD3biwVfg7amxZ2daWlFXVI28EdO9lsPpzRzCwS+dal Q9TPtk+txPTeB7Om8JOyY70KpxDP2GSbYdPrVLlDVKrPMxTZb7uqFTfg7PUcgwxMMj6N mBdw== X-Gm-Message-State: AJaThX5VDrsQGU742uHSJShaP8zTpSjLfMQl9eFyxBrMPJxQa6TK1zaj O04qBZCxN3MOPhYH/WzCCkfGvg== X-Google-Smtp-Source: AGs4zMbi9AeoagYmhapcLTmIQxkWW7ULKD7igVVIm6hiVblNnF2F8D31RCwgJ9d8IO3l5aD/+dy87A== X-Received: by 10.55.34.135 with SMTP id i129mr25841236qki.86.1511285896398; Tue, 21 Nov 2017 09:38:16 -0800 (PST) Received: from raichu (toroon0560w-lp140-01-69-159-38-22.dsl.bell.ca. [69.159.38.22]) by smtp.gmail.com with ESMTPSA id h21sm1144178qte.44.2017.11.21.09.38.15 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 21 Nov 2017 09:38:15 -0800 (PST) Sender: Mark Johnston Date: Tue, 21 Nov 2017 12:38:13 -0500 From: Mark Johnston To: Andre Albsmeier Cc: freebsd-hackers@freebsd.org Subject: Re: gmirror synchronising is very slow due to frequent metadata updates Message-ID: <20171121173813.GB4126@raichu> References: <20171119103241.GA20588@voyager> <20171120033828.GA1959@bish> <20171120053409.GA57536@gate> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171120053409.GA57536@gate> User-Agent: Mutt/1.9.1 (2017-09-22) X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Nov 2017 17:38:17 -0000 On Mon, Nov 20, 2017 at 06:34:09AM +0100, Andre Albsmeier wrote: > On Sun, 19-Nov-2017 at 22:38:28 -0500, Mark Johnston wrote: > > We should probably decrease the update interval based on the size of a > > mirror. For mirrors larger than say, 1GB, we might just update the > > metadata block once per 1% of the synchronization operation's progress. > > I think best would be to have it updated every seconds. > I think of very fast or very slow drives, or drives which are used > heavily during rebuild (where the effective rebuild speed is quite > low)... I think that's reasonable. Could you give the patch below a try? It adds a sysctl which configures the frequency of metadata updates during synchronization. It'd be nice to be able to specify this parameter (as well as idletime) on a per-gmirror basis, with the default value still being taken from the sysctl, but that can be implemented separately. diff --git a/sys/geom/mirror/g_mirror.c b/sys/geom/mirror/g_mirror.c index 4a5f542c1de5..47d11f3ce465 100644 --- a/sys/geom/mirror/g_mirror.c +++ b/sys/geom/mirror/g_mirror.c @@ -69,6 +69,10 @@ SYSCTL_UINT(_kern_geom_mirror, OID_AUTO, disconnect_on_failure, CTLFLAG_RWTUN, static u_int g_mirror_syncreqs = 2; SYSCTL_UINT(_kern_geom_mirror, OID_AUTO, sync_requests, CTLFLAG_RDTUN, &g_mirror_syncreqs, 0, "Parallel synchronization I/O requests."); +static u_int g_mirror_sync_period = 5; +SYSCTL_UINT(_kern_geom_mirror, OID_AUTO, sync_update_period, CTLFLAG_RWTUN, + &g_mirror_sync_period, 0, + "Metadata update period during synchroniztion, in seconds"); #define MSLEEP(ident, mtx, priority, wmesg, timeout) do { \ G_MIRROR_DEBUG(4, "%s: Sleeping %p.", __func__, (ident)); \ @@ -463,6 +467,7 @@ g_mirror_init_disk(struct g_mirror_softc *sc, struct g_provider *pp, disk->d_sync.ds_consumer = NULL; disk->d_sync.ds_offset = md->md_sync_offset; disk->d_sync.ds_offset_done = md->md_sync_offset; + disk->d_sync.ds_update_ts = time_uptime; disk->d_genid = md->md_genid; disk->d_sync.ds_syncid = md->md_syncid; if (errorp != NULL) @@ -1457,10 +1462,11 @@ g_mirror_sync_request(struct bio *bp) if (bp != NULL && bp->bio_offset < offset) offset = bp->bio_offset; } - if (sync->ds_offset_done + (MAXPHYS * 100) < offset) { - /* Update offset_done on every 100 blocks. */ + if (g_mirror_sync_period > 0 && + time_uptime - sync->ds_update_ts > g_mirror_sync_period) { sync->ds_offset_done = offset; g_mirror_update_metadata(disk); + sync->ds_update_ts = time_uptime; } return; } diff --git a/sys/geom/mirror/g_mirror.h b/sys/geom/mirror/g_mirror.h index 3b9664035c98..be46f7b43e4c 100644 --- a/sys/geom/mirror/g_mirror.h +++ b/sys/geom/mirror/g_mirror.h @@ -108,6 +108,7 @@ struct g_mirror_disk_sync { off_t ds_offset; /* Offset of next request to send. */ off_t ds_offset_done; /* Offset of already synchronized region. */ + time_t ds_update_ts; /* Time of last metadata update. */ u_int ds_syncid; /* Disk's synchronization ID. */ u_int ds_inflight; /* Number of in-flight sync requests. */ struct bio **ds_bios; /* BIOs for synchronization I/O. */