Re: [PATCH 05/16] reftable/table: move reading block into block reader

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 7, 2025, 12:31 UTC
Message-ID: <Z_PFjr9p4u4MA4yD@pks.im>
In-Reply-To: <umaolgemnks4g4cgwgnurl7alic2a3pzrfx2ws3s7uxypiedia@7mwqxvhw2zxg>

On Wed, Apr 02, 2025 at 03:13:03PM -0500, Justin Tobler wrote:

Show 35 quoted lines

> On 25/03/31 10:41AM, Patrick Steinhardt wrote:
> > diff --git a/reftable/block.c b/reftable/block.c
> > index f2567a8f0fd..2517108b8ef 100644
> > --- a/reftable/block.c
> > +++ b/reftable/block.c
> > @@ -209,31 +209,57 @@ int block_writer_finish(struct block_writer *w)
> >  	return w->next;
> >  }
> >  
> > -int block_reader_init(struct block_reader *br, struct reftable_block *block,
> > -		      uint32_t header_off, uint32_t table_block_size,
> > -		      uint32_t hash_size)
> > +static int read_block(struct reftable_block_source *source,
> > +		      struct reftable_block *dest, uint64_t off,
> > +		      uint32_t sz)
> >  {
> > +	size_t size = block_source_size(source);
> > +	block_source_return_block(dest);
> > +	if (off >= size)
> > +		return 0;
> > +	if (off + sz > size)
> > +		sz = size - off;
> > +	return block_source_read_block(source, dest, off, sz);
> > +}
> > +
> > +int block_reader_init(struct block_reader *br,
> > +		      struct reftable_block_source *source,
> > +		      uint32_t offset, uint32_t header_size,
> > +		      uint32_t table_block_size, uint32_t hash_size)
> > +{
> > +	uint32_t guess_block_size = table_block_size ?
> > +		table_block_size : DEFAULT_BLOCK_SIZE;
> 
> Out of curiousity, in what scenarios would the table not know the block
> size and we have to rely on the guess?

By default, reftable blocks are aligned with padding, and if so the block size is tracked as part of the footer and well-known. Ideally, the block size would be picked so that it is the same as your disk sector size. Git picks 4kB by default.

There is another mode though: reftable blocks can be unaligned, so the padding is dropped. Consequently there is no fixed block siz, and this is indicated by having a block size of 0 in the header/trailer.

Patrick