importccl: Resume interrupted http download during import. #43374
Conversation
|
This change is |
miretskiy
force-pushed
the
resume_interrupted
branch
from
19bf2d1
to
2cb6353
Compare
miretskiy
force-pushed
the
resume_interrupted
branch
from
2cb6353
to
b4c2e7a
Compare
There was a problem hiding this comment.
I like the change! However I'm confused by the failing test in CI.
Reviewed 2 of 2 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @dt and @miretskiy)
pkg/storage/cloud/external_storage.go, line 563 at r1 (raw file):
// there is really nothing we can do: http standard says that // the stream ends when the server terminates connection. if err == io.ErrUnexpectedEOF && r.h.canResume {
the new idiom is if errors.Is(err, io.ErrUnexpectedEOF) && ...
miretskiy
force-pushed
the
resume_interrupted
branch
from
b4c2e7a
to
1980c8f
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/storage/cloud/external_storage.go, line 563 at r1 (raw file):
Previously, knz (kena) wrote…
the new idiom is
if errors.Is(err, io.ErrUnexpectedEOF) && ...
Done.
There was a problem hiding this comment.
I took a look at the failures -- those were legit. I had to make some changes in the code to make sure I store retry state in the bodyReader and not on the httpstorage object.
PTAL.
Reviewable status:
miretskiy
force-pushed
the
resume_interrupted
branch
2 times, most recently
from
c63192b
to
cc8a611
Compare
| @@ -474,13 +474,120 @@ func (h *httpStorage) Conf() roachpb.ExternalStorage { | |||
| } | |||
| } | |||
|
|
|||
| type reqSender = func(reqHeaders map[string]string) (io.ReadCloser, http.Header, error) | |||
|
|
|||
| type bodyReader struct { | |||
There was a problem hiding this comment.
2¢: I might call this httpReader or resumingHTTPReader since there are other things in this package that might have bodies that can be read.
| // checkContentRangeHeader parses Content-Range header and | ||
| // ensures that range start offset is the same as 'pos' | ||
| // See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range | ||
| func checkContentRangeHeader(h string, pos int64) error { |
There was a problem hiding this comment.
Ditto here: I’d add “HTTP” to the name
Make importing data into cockroach from external http servers resilient to connection interruption. If the download is interrupted, and if the external server supports "Accept-Ranges: bytes" (and most servers do support that), we attempt to request the next "Content-Range" from the server. Release note (enterprise change): more resilient http import
miretskiy
force-pushed
the
resume_interrupted
branch
from
cc8a611
to
b4f52dd
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/storage/cloud/external_storage.go, line 479 at r2 (raw file):
Previously, dt (David Taylor) wrote…
2¢: I might call this httpReader or resumingHTTPReader since there are other things in this package that might have bodies that can be read.
Done.
pkg/storage/cloud/external_storage.go, line 497 at r2 (raw file):
Previously, dt (David Taylor) wrote…
Ditto here: I’d add “HTTP” to the name
Done.
|
bors r+ |
43374: importccl: Resume interrupted http download during import. r=miretskiy a=miretskiy Make importing data into cockroach from external http servers resilient to connection interruption. If the download is interrupted, and if the external server supports "Accept-Ranges: bytes" (and most servers do support that), we attempt to request the next "Content-Range" from the server. Release note (enterprise change): more resilient http import Co-authored-by: Yevgeniy Miretskiy <yevgeniy@cockroachlabs.com>
Build succeeded |
43687: release-19.2: importccl: resilient import over http r=miretskiy a=miretskiy Backport: * 1/1 commits from "importccl: Resume interrupted http download during import." (#43374) * 1/1 commits from "importccl: improve handling of transient errors when importing over http" (#43558) Please see individual PRs for details. /cc @cockroachdb/release Co-authored-by: Yevgeniy Miretskiy <yevgeniy@cockroachlabs.com>
Make importing data into cockroach from external http servers
resilient to connection interruption.
If the download is interrupted, and if the external server
supports "Accept-Ranges: bytes" (and most servers do support that),
we attempt to request the next "Content-Range" from the server.
Release note (enterprise change): more resilient http import