[GSoC,v4,0/3] reftable: return proper error codes from block_writer_add

Message

Meet Soni March 19, 2025, 7:59 a.m. UTC

This patch attempts to avoid making an assumption regarding error codes
returned by block_writer_add().

Changes since v3:
    - split commit based on the functions it alters
    - add comment back that was earlier removed.

Meet Soni (3):
  reftable: propagate specific error codes in block_writer_add()
  reftable: adapt writer_add_record() to propagate block_writer_add()
    errors
  reftable: adapt write_object_record() to propagate block_writer_add()
    errors

 reftable/block.c  | 13 ++++++------
 reftable/block.h  |  2 +-
 reftable/record.c | 53 +++++++++++++++++++++--------------------------
 reftable/writer.c | 27 +++++++++++++++---------
 4 files changed, 49 insertions(+), 46 deletions(-)

Range-diff against v3:
1:  6ab35d569c = 1:  6ab35d569c reftable: propagate specific error codes in block_writer_add()
2:  a54d440dd3 ! 2:  7f0bdc27e1 reftable: adapt writer code to propagate block_writer_add() errors
    @@ Metadata
     Author: Meet Soni <[e-mail]>
     
      ## Commit message ##
    -    reftable: adapt writer code to propagate block_writer_add() errors
    +    reftable: adapt writer_add_record() to propagate block_writer_add() errors
     
    -    Previously, writer_add_record() and write_object_record() would flush the
    -    current block and retry appending the record whenever block_writer_add()
    -    returned any nonzero error. This forced an assumption that every failure
    -    meant the block was full, even when errors such as memory allocation or
    -    I/O failures occurred.
    +        Previously, writer_add_record() would flush the current block and
    +        retry appending the record whenever block_writer_add() returned any
    +        nonzero error. This forced an assumption that every failure meant
    +        the block was full, even when errors such as memory allocation or I/O
    +        failures occurred.
     
    -    Update the writer code to inspect the error code returned by
    -    block_writer_add() and only flush and reinitialize the writer when the
    -    error is REFTABLE_ENTRY_TOO_BIG_ERROR. For any other error, immediately
    -    propagate it.
    -
    -    All call sites now handle various error codes returned by
    -    block_writer_add().
    +        Update the writer_add_record() to inspect the error code returned by
    +        block_writer_add() and only flush and reinitialize the writer when the
    +        error is REFTABLE_ENTRY_TOO_BIG_ERROR. For any other error, immediately
    +        propagate it.
     
         Signed-off-by: Meet Soni <[e-mail]>
     
    @@ reftable/writer.c: static int writer_add_record(struct reftable_writer *w,
      	 * The current block is full, so we need to flush and reinitialize the
      	 * writer to start writing the next block.
     @@ reftable/writer.c: static int writer_add_record(struct reftable_writer *w,
    - 		goto done;
    - 
      	/*
    --	 * Try to add the record to the writer again. If this still fails then
    --	 * the record does not fit into the block size.
    + 	 * Try to add the record to the writer again. If this still fails then
    + 	 * the record does not fit into the block size.
     -	 *
     -	 * TODO: it would be great to have `block_writer_add()` return proper
     -	 *       error codes so that we don't have to second-guess the failure
     -	 *       mode here.
    -+	 * Try to add the record to the writer again.
      	 */
      	err = block_writer_add(w->block_writer, rec);
     -	if (err) {
    @@ reftable/writer.c: static int writer_add_record(struct reftable_writer *w,
      
      done:
      	return err;
    -@@ reftable/writer.c: static void write_object_record(void *void_arg, void *key)
    - 	if (arg->err < 0)
    - 		goto done;
    - 
    -+	/*
    -+	 * Try to add the record to the writer. If this succeeds then we're
    -+	 * done. Otherwise the block writer may have hit the block size limit
    -+	 * and needs to be flushed.
    -+	 */
    - 	arg->err = block_writer_add(arg->w->block_writer, &rec);
    - 	if (arg->err == 0)
    - 		goto done;
    - 
    -+	if (arg->err != REFTABLE_ENTRY_TOO_BIG_ERROR)
    -+		goto done;
    -+
    -+	/*
    -+	 * 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;
-:  ---------- > 3:  480ac27797 reftable: adapt write_object_record() to propagate block_writer_add() errors