From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 830DCEF18 for ; Thu, 28 Sep 2023 09:04:48 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6423F12B3E for ; Thu, 28 Sep 2023 09:04:48 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Thu, 28 Sep 2023 09:04:47 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 10B314442B for ; Thu, 28 Sep 2023 09:04:47 +0200 (CEST) Date: Thu, 28 Sep 2023 09:04:46 +0200 From: Wolfgang Bumiller To: Christian Ebner Cc: pbs-devel@lists.proxmox.com, m.carrara@proxmox.com Message-ID: References: <20230922071621.12670-1-c.ebner@proxmox.com> <20230922071621.12670-8-c.ebner@proxmox.com> <526287774.5120.1695817218589@webmail.proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <526287774.5120.1695817218589@webmail.proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.099 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [RFC pxar 7/20] fix #3174: encoder: add helper to incr encoder pos X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Sep 2023 07:04:48 -0000 On Wed, Sep 27, 2023 at 02:20:18PM +0200, Christian Ebner wrote: > > > On 27.09.2023 14:07 CEST Wolfgang Bumiller wrote: > > > > > > 'incr' :S > > > > On Fri, Sep 22, 2023 at 09:16:08AM +0200, Christian Ebner wrote: > > > Adds a helper to allow to increase the encoder position by a given > > > size. This is used to increase the position when adding an appendix > > > section to the pxar stream, as these bytes are never encoded directly > > > but rather referenced by already existing chunks. > > > > Exposing this seems like a weird choice to me. Why exactly is this > > needed? Why don't we instead expose methods to actually write the > > appendix section instead? > > This is needed in order to increase the byte offset of the encoder itself. > The appendix section is a list of chunks which are injected in the chunk > stream on upload, but never really consumed by the encoder and subsequently > the chunker itself. So there is no direct writing of the appendix section to > the stream. > > By adding the bytes, consistency with the rest of the pxar archive is assured, > as these chunks/bytes are present during decoding. Ah so we inject the *contents* of the old pxar archive by way of sending the chunks a writing "layer" above. Initially I thought the archive would contain chunk ids, but this makes more sense. And is unfortunate for the API :-) Maybe consider marking the position modification as `unsafe fn`, though? I mean it is a foot gun to break the resulting archive with, after all ;-) But this means we don't have a direct way of creating incremental pxars without a PBS context, doesn't it? Would it make sense to have a method here which returns a Writer to the `EncoderOutput` where we could in theory also just "dump in" contents of another actual pxar file (where the byte counting happens implicitly), which also has an extra `unsafe fn add_out_of_band_bytes()` to do the raw byte count modification? One advantage of having a "starting point" for this type of operation is that we'd also force a `flush()` before out-of-band data gets written. Otherwise, if we don't need/want this, we should probably just add a `flush()` to the encoder we should call before adding any chunks out of band, given that Max already tried to sneak in a BufRead/Writers into the pxar crate for optimization purposes, IIRC ;-)