On Wed, 12 Mar 2025 at 18:19, Patrick Steinhardt <ps@xxxxxx> wrote:
>
> > + /*
> > + * The current block is full, so we need to flush and reinitialize the
> > + * writer to start writing the next block.
> > + */
> > arg->err = writer_flush_block(arg->w);
> > if (arg->err < 0)
> > goto done;
>
> But there is another case further down where we do `block_writer_add()`
> and then re-try in case the write fails. This one is a bit more curious:
> if the write fails, we don't create a new block -- after all we have
> just created one. Instead, we reset the record's offset length to zero
> before retrying.
>
> I _think_ that this is done because we know that when resetting the
> offset we would write less data to the block, as can be seen in
> `reftable_obj_record_encode()`. But I'm honestly not quite sure here as
> I haven't yet done a deep dive into object records -- after all, we
> don't even really use them in Git.
>
> In any case, I think that this callsite also needs adjustment and
> warrants a comment. And if so, all changes to `write_object_record()`
> should probably go into a separate commit, as well.
>
Regarding the callsite in write_object_record() where we reset the
record's offset length to zero before retrying: my changes currently
follow the same principle.
- If block_writer_add() returns an error other than
REFTABLE_ENTRY_TOO_BIG_ERROR, we simply return.
- For REFTABLE_ENTRY_TOO_BIG_ERROR, we flush the block and retry.
- If that fails, we reset the record's offset length to zero and
then retry.
I'm not sure what adjustments or additional comments you are referring to.
Could you please clarify what changes you expect at this callsite?
Thanks!
Meet
