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 5BE9E1FF0B2 for ; Mon, 24 Aug 2026 11:39:31 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 060E92159F; Mon, 24 Aug 2026 11:39:31 +0200 (CEST) Message-ID: <1186c9b2-f08e-4db1-9c8c-e415f256f835@proxmox.com> Date: Mon, 24 Aug 2026 11:39:23 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Christian Ebner Subject: Re: [RFC] add max-depth to the datastore snapshots API endpoint To: sales@mooresolutions.co.nz, pbs-devel References: <1a026d2b773.4f0fd0cd237366.1462259800663760854@mooresolutions.co.nz> Content-Language: en-US, de-DE In-Reply-To: <1a026d2b773.4f0fd0cd237366.1462259800663760854@mooresolutions.co.nz> 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: 1787564334281 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.581 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) POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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: 3AZTDSOARE7Q2NHPNI75PIIRBZ73EJEO X-Message-ID-Hash: 3AZTDSOARE7Q2NHPNI75PIIRBZ73EJEO 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: Hi! On 8/22/26 2:30 AM, thomas moore wrote: > Hi all > > This is a proposal for discussion, not a patch. The > developer documentation asks that plans be discussed before > starting work, so I'd like to check whether this would be > acceptable before writing it. > > GET /api2/json/admin/datastore/{store}/snapshots returns only > the snapshots in the requested namespace, so a client wanting > a datastore-wide view has to call > /admin/datastore/{store}/namespace first and then issue one > snapshots request per namespace. I ran into this writing a > third-party monitoring client. > > Tested against PBS 4.2: > > /admin/datastore/pbs/snapshots -> 485 snapshots (root > only) > > /admin/datastore/pbs/snapshots?ns=test -> 1 snapshot > > /admin/datastore/pbs/snapshots?max-depth=7 -> 400 > parameter verification failed - 'max-depth': schema does not > allow additional properties > > Proposal: accept an optional max-depth using the > existing NS_MAX_DEPTH_SCHEMA, defaulting to 0 so behaviour is > unchanged for current callers. > > This looks anticipated in the code. As of > 5d95fc20e, src/api2/admin/datastore.rs lines 571, 575 and 583 > each carry a "// FIXME: Recursion" on the group > construction in list_snapshots_blocking, with a further one at > 565 about filtering by owner before collecting. > ListAccessibleBackupGroups already takes max_depth and applies > the per-namespace privilege and owner checks during iteration, > and get_snapshots_count uses it in roughly the shape list_snapshots > would need. > > I'd send it as two patches: the refactor > onto ListAccessibleBackupGroups with no behavioural change, then > max-depth on top. I can test on a 4.2 instance including with a > token holding DatastoreAudit on the root namespace only, to > confirm namespaces the token cannot see stay absent from a > recursive listing. > > > > > > Questions for the list > > 1. Is adding max-depth to the snapshots endpoint acceptable in > principle? Yes, although there has already been some work on this [0], implementing a streaming API which should already cover your case as well. This however is not applied yet, but might be picked up again. Nevertheless, may I ask what your goal is and what limitation which is not already covered by the current API you are running into? Does performing the additional requests cause to much latency for your usecase? > 2. Is ListAccessibleBackupGroups the right vehicle here, or is > there a reason list_snapshots does not already use it? list_snapshots_blocking() is rather heavy weight and performance critical, especially for datastores with lots of groups/snapshots on slow (spin rust) storage. So performance and memory analysis is a must here. Further, current behavior must be retained also with respect to group listing by type and backup id only, respectively. But again, I think patch [0] already would cover what you are asking for. > 3. Snapshot entries carry no namespace field, so a recursive > listing is ambiguous unless the client tracks it per request. Adding > one would mean a new field on SnapshotListItem, which is also > decoded from remote instances in server/sync.rs and server/push.rs While not that critical, adding the additional field does have implications for the response payload size, especially for deeply nested datastores with lots of snapshots. > If you'd rather this were solved another way or not at all I'm > happy to hear it before writing anything. > > Regards Thomas Moore [0] https://lore.proxmox.com/pbs-devel/20251008134344.3512958-8-d.csapak@proxmox.com/ Best regards, Chris