[PATCH 6 of 6] bundle2: don't attempt I/O after an I/O error occurs (issue4784)
Yuya Nishihara
yuya at tcha.org
Sat Apr 15 23:41:00 EDT 2017
On Fri, 14 Apr 2017 00:44:10 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1492155656 25200
> # Fri Apr 14 00:40:56 2017 -0700
> # Node ID d1b31c378dc8e29d9827f9ded9fd023d5a00b0c9
> # Parent 8525abda8397f2a5a94edc5db2279549ef53b8e8
> bundle2: don't attempt I/O after an I/O error occurs (issue4784)
> A subtle bug with this change is that *all* I/O errors will influence
> behavior, not just I/O errors on the bundle2 stream. e.g. an I/O error
> from the local filesystem could result in not seeking the bundle. This
> is the source of the test-acl.t test change, for example. I question
> whether we care. Considering pretty much all bundle2 stream I/O errors
> resulted in this code not running before (due to a fast I/O error
> re-raise), we seem to be getting along just fine without the code. And
> since this error recovery code is only tested in test-acl.t, I'm
> guessing it isn't important. Or, at least it isn't as important as
> accurately reporting the original I/O error. Considering these "stream
> ended unexpectedly" errors have been masking several problems, I
> think the new code is acceptable. We can always improve the stream
> error detection code later if the new model isn't good enough.
> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
> --- a/mercurial/bundle2.py
> +++ b/mercurial/bundle2.py
> @@ -354,9 +354,13 @@ def processbundle(repo, unbundler, trans
> for nbpart, part in iterparts:
> _processpart(op, part)
> except Exception as exc:
> - for nbpart, part in iterparts:
> - # consume the bundle content
> - part.seek(0, 2)
> + # Don't attempt further I/O after we've encountered an I/O failure.
> + # This will almost certainly just raise another I/O failure and will
> + # only mask the underlying failure.
> + if not isinstance(exc, (IOError, error.RichIOError)):
> + for nbpart, part in iterparts:
> + # consume the bundle content
> + part.seek(0, 2)
I think this is okay, but if we have to care about a local IOError, maybe we
could try consuming bundle and suppress errors?
except Exception as exc:
try:
# consume the bundle content
except (IOError, error.RichIOError):
pass
More information about the Mercurial-devel
mailing list