All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Han-Wen Nienhuys <hanwenn@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v2 06/11] reftable/writer: refactorings for `writer_add_record()`
Date: Thu, 4 Apr 2024 09:32:21 +0200	[thread overview]
Message-ID: <Zg5XhYq802lG4AgU@tanuki> (raw)
In-Reply-To: <CAOw_e7YeqEK4O=KWowMYGtRVMLwL3y6bWw2LRfC9TqJz06Esyg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]

On Thu, Apr 04, 2024 at 08:58:08AM +0200, Han-Wen Nienhuys wrote:
> On Thu, Apr 4, 2024 at 7:48 AM Patrick Steinhardt <ps@pks.im> wrote:
> > +       /*
> > +        * 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.
> > +        */
> 
> The Go code returns a (size, boolean) tuple for the write routines
> here, but that does not really work in the Git C style.
> 
> If you make the routines return error codes it suggests that the
> in-memory write can fail for other reasons beyond "does not fit". Not
> sure if that is really an improvement.

In reality, `block_writer_add()` already can fail because of different
reasons: it returns `REFTABLE_API_ERROR` if the passed-in record has an
empty key. This shouldn't ever happen, but it demonstrates that this is
certainly an area which needs some further cleanups.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2024-04-04  7:32 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-02 17:29 [PATCH 0/9] reftable: optimize write performance Patrick Steinhardt
2024-04-02 17:29 ` [PATCH 1/9] refs/reftable: fix D/F conflict error message on ref copy Patrick Steinhardt
2024-04-03 18:28   ` Junio C Hamano
2024-04-02 17:29 ` [PATCH 2/9] refs/reftable: perform explicit D/F check when writing symrefs Patrick Steinhardt
2024-04-02 17:30 ` [PATCH 3/9] refs/reftable: skip duplicate name checks Patrick Steinhardt
2024-04-02 17:30 ` [PATCH 4/9] refs/reftable: don't recompute committer ident Patrick Steinhardt
2024-04-03 18:58   ` Junio C Hamano
2024-04-04  5:36     ` Patrick Steinhardt
2024-04-02 17:30 ` [PATCH 5/9] reftable/writer: refactorings for `writer_add_record()` Patrick Steinhardt
2024-04-02 17:30 ` [PATCH 6/9] reftable/writer: refactorings for `writer_flush_nonempty_block()` Patrick Steinhardt
2024-04-02 17:30 ` [PATCH 7/9] reftable/block: reuse zstream when writing log blocks Patrick Steinhardt
2024-04-03 19:35   ` Junio C Hamano
2024-04-04  5:36     ` Patrick Steinhardt
2024-04-02 17:30 ` [PATCH 8/9] reftable/block: reuse compressed array Patrick Steinhardt
2024-04-02 17:30 ` [PATCH 9/9] reftable/writer: reset `last_key` instead of releasing it Patrick Steinhardt
2024-04-04  5:48 ` [PATCH v2 00/11] reftable: optimize write performance Patrick Steinhardt
2024-04-04  5:48   ` [PATCH v2 01/11] refs/reftable: fix D/F conflict error message on ref copy Patrick Steinhardt
2024-04-04  5:48   ` [PATCH v2 02/11] refs/reftable: perform explicit D/F check when writing symrefs Patrick Steinhardt
2024-04-04  5:48   ` [PATCH v2 03/11] refs/reftable: skip duplicate name checks Patrick Steinhardt
2024-04-04  5:48   ` [PATCH v2 04/11] reftable: remove " Patrick Steinhardt
2024-04-04  5:48   ` [PATCH v2 05/11] refs/reftable: don't recompute committer ident Patrick Steinhardt
2024-04-04  5:48   ` [PATCH v2 06/11] reftable/writer: refactorings for `writer_add_record()` Patrick Steinhardt
2024-04-04  6:58     ` Han-Wen Nienhuys
2024-04-04  7:32       ` Patrick Steinhardt [this message]
2024-04-04  5:48   ` [PATCH v2 07/11] reftable/writer: refactorings for `writer_flush_nonempty_block()` Patrick Steinhardt
2024-04-04  5:48   ` [PATCH v2 08/11] reftable/writer: unify releasing memory Patrick Steinhardt
2024-04-04  7:08     ` Han-Wen Nienhuys
2024-04-04  7:32       ` Patrick Steinhardt
2024-04-04  9:00         ` Han-Wen Nienhuys
2024-04-04 11:43           ` Patrick Steinhardt
2024-04-04  5:48   ` [PATCH v2 09/11] reftable/writer: reset `last_key` instead of releasing it Patrick Steinhardt
2024-04-04  5:48   ` [PATCH v2 10/11] reftable/block: reuse zstream when writing log blocks Patrick Steinhardt
2024-04-04  5:48   ` [PATCH v2 11/11] reftable/block: reuse compressed array Patrick Steinhardt
2024-04-04  7:09   ` [PATCH v2 00/11] reftable: optimize write performance Han-Wen Nienhuys
2024-04-04  7:32     ` Patrick Steinhardt
2024-04-08 12:23 ` [PATCH v3 " Patrick Steinhardt
2024-04-08 12:23   ` [PATCH v3 01/11] refs/reftable: fix D/F conflict error message on ref copy Patrick Steinhardt
2024-04-08 12:23   ` [PATCH v3 02/11] refs/reftable: perform explicit D/F check when writing symrefs Patrick Steinhardt
2024-04-08 12:24   ` [PATCH v3 03/11] refs/reftable: skip duplicate name checks Patrick Steinhardt
2024-04-08 12:24   ` [PATCH v3 04/11] reftable: remove " Patrick Steinhardt
2024-04-08 12:24   ` [PATCH v3 05/11] refs/reftable: don't recompute committer ident Patrick Steinhardt
2024-04-08 12:24   ` [PATCH v3 06/11] reftable/writer: refactorings for `writer_add_record()` Patrick Steinhardt
2024-04-08 12:24   ` [PATCH v3 07/11] reftable/writer: refactorings for `writer_flush_nonempty_block()` Patrick Steinhardt
2024-04-08 12:24   ` [PATCH v3 08/11] reftable/writer: unify releasing memory Patrick Steinhardt
2024-04-08 12:24   ` [PATCH v3 09/11] reftable/writer: reset `last_key` instead of releasing it Patrick Steinhardt
2024-04-08 12:24   ` [PATCH v3 10/11] reftable/block: reuse zstream when writing log blocks Patrick Steinhardt
2024-04-08 12:24   ` [PATCH v3 11/11] reftable/block: reuse compressed array Patrick Steinhardt
2024-04-09  0:09   ` [PATCH v3 00/11] reftable: optimize write performance Junio C Hamano
2024-04-09  3:16     ` Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zg5XhYq802lG4AgU@tanuki \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hanwenn@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.