On Thu, Aug 22, 2019 at 09:25:34AM -0700, Jonathan Tan wrote:
> > The batch fetch is purely an optimization, because we'd eventually fetch
> > the individual objects on demand. If the batch one fails, should we
> > continue with the operation? That leaves any error-handling for the
> > overall operation to the "real" code. And it would also mean that this
> > bug became an annoying error message,
>
> On the one hand, if the batch fetch fails, then the individual
> prefetching would likely fail as well. But on the other hand, as you
> said below, we sometimes extraneously fetch objects, so making the batch
> fetch non-fatal might be a good idea too.
Yeah, I'd expect it to fail again on the individual fetch[1]. But then
we are leaving that code to handle the error as it sees fit, which might
not be to die(). Though TBH, the diff code is pretty eager to die() on
missing objects anyway.
Of course the die() we see in this case is not really in your new code
at all, but somewhere deep in the fetch stack. So there's probably a
fair bit of work in making a failed fetch a recoverable error in the
first place.
[1] You'd incur two attempts to fetch, which are expensive, but if
we care about that it would be easy enough to keep an in-memory list
of "I tried to fetch this once already and could not", and just
quickly return failure on the second attempt.
-Peff
