From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id CA5951FF0EA for ; Fri, 14 Aug 2026 16:48:35 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 7A7CB226C2; Fri, 14 Aug 2026 16:48:35 +0200 (CEST) Message-ID: <927b348a-0a90-45f3-877e-a7339d5eb293@proxmox.com> Date: Fri, 14 Aug 2026 16:48:30 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 proxmox proxmox-backup 0/3] datastore: gc: defer and batch chunk atime updates To: Enrico Plantulli , pbs-devel@lists.proxmox.com References: <20260810050201.347124-1-plantulli@gmail.com> <20260811093722.735290-1-plantulli@gmail.com> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: <20260811093722.735290-1-plantulli@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1786718892772 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.190 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust RDNS_NONE 1.274 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: TSFB7WFJXLKWR4FUHKQZKU5KG2WIWRR2 X-Message-ID-Hash: TSFB7WFJXLKWR4FUHKQZKU5KG2WIWRR2 X-MailFrom: c.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 8/11/26 11:37 AM, Enrico Plantulli wrote: > Hi, > > this is v2 of the chunk metadata prefetch series, reworked along the > lines suggested in the review of v1: instead of one whole-store > readdir pass before phase 1, the chunk atime updates themselves are > now deferred into a bounded list and flushed sorted by digest, opening > and iterating each chunk directory right before its chunks are > touched, with the utimensat() calls going through the open directory > file descriptor. > > Changes since v1: > > * dropped the whole-store prefetch pass and the > prefetch_chunk_metadata() helper entirely; > * added deferred, batched atime updates behind the same opt-in > gc-chunk-metadata-prefetch tuning option (kept opt-in as requested: > backends that do not prefetch on readdir, e.g. network attached > storage, may regress); > * added a separate gc-prefetch-batch-size tuning option (default > 1048576, range 1024 - 16M) as an independent knob, as suggested, to > make benchmarking on different backends easy; > * the batch is used for filesystem backed datastores only; S3 keeps > the immediate path, since its in-use markers need per-chunk > handling anyway; > * a chunk found missing at flush time is handled as before (touch the > .bad companions, warn), but the warning can only name the digest, > not the referencing index file - noted in patch 2; > * GUI patch now covers both options. > > Why the rework was needed, empirically: I ran the v1 one-pass version > on the 72M chunk production datastore (A in the v1 thread). The pass > itself was fast (81.8M directory entries in 59m32s), but the warmed > metadata did not survive until a phase 1 that runs for hours reached > it: the GC worker was observed blocked in > zio_wait <- dbuf_read <- zap_get_leaf_byblk (and dnode_hold_impl), > re-reading from disk the very blocks the pass had loaded hours > earlier, with the ARC sitting at its adaptive target far below > c_max. External re-walks recovered the rate only modestly (~+30%). > Warming right before use removes that window entirely, which is what > this v2 does. > > The batch size bound also controls the readdir amplification: a flush > of N sorted entries spans up to min(N, 65536) directories, so about > N/65536 chunks are served per directory read. The default of 1M gives > ~16 used chunks per directory read on a full store; benchmarking > larger values is exactly what the separate option is for. > > Testing: compile-tested (cargo build, clippy with no new warnings, > fmt, cargo test) against current master of both repositories. I have > not yet run this exact v2 end-to-end in production; I will follow up > with figures from datastore A and B once it has. NAS-backed figures > would be very welcome from anyone with such a setup, as discussed. > > CLA: signed and sent to office@proxmox.com on Aug 10; office has been > in touch. Thanks for v2 of the patches, they have seen an improvement over the previous version but still require major revision. I left more details on the patches, but here some general comments as well. In general it would be desirable to only have one optional parameter to control the GC behavior for this, see comments on patch 1 for details. Also, while the patches improve GC performance in case of cold caches, they do also introduce a performance regression for hot cache case. Some results from my testing (5 GC runs each, dropped caches via `echo 3 > /proc/sys/vm/drop_caches`), measuring just phase 1 of GC: ZFS on SDD backed datastore: Chunk cache: hits 2671816, misses 924604 (hit ratio 74.29%) On-Disk usage: 2.077 TiB On-Disk chunks: 924604 Deduplication factor: 5.32 no readdir (caches dropped): 54.54 ± 0.28s no readdir (caches kept): 6.72 ± 0.02s with readdir (caches dropped, default batch size): 32.66 ± 0.16s with readdir (caches kept, default batch size): 9.82 ± 0.97s with readdir (caches dropped, max batch size): 32.65 ± 0.09s A particular observation is that the speedup for GC is observed even when the readdir() is not performed (result not included above, see comment and diff on patch 2). The major speed gain is therefore attributed to opening the chunk prefix directory in a structured way, allowing the utimensat() calls to be relative to that open file handle brings additional gains. This could further be optimized by only doing the additional directory open call if more than 1 chunk have to be touched within it, therefore reducing number of syscalls if not required. Another observation is that progress logging is currently broken, since now the index file reading is strongly decoupled from the atime updates. This should be improved upon. Also, please do provide some testing results from your side as well, getting additional datapoints here is desired.