lix-project/lix
20
57
Fork 31
Code Issues 347 Code Review (Gerrit) Projects 3 Releases Wiki Activity

libstore: make baseRetryTimeMs a FileTransfer property

Browse source 
we don't even need this outside of tests. maybe we should not do
automatic retries at this level at all and use retrying wrappers
instead? at some point we may have to do this, but not just yet.

Change-Id: If0088aa55215be81f1770c25b3bb1b5268c65cf8
This commit is contained in:
eldritch horrors 2024-10-26 17:33:14 +02:00
parent 1e3b45546c
commit af27d1ecd8
3 changed files with 13 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
src/libstore/filetransfer.cc
View file

@ -39,6 +39,7 @@ struct curlFileTransfer : public FileTransfer
std::random_device rd; std::random_device rd;
std::mt19937 mt19937; std::mt19937 mt19937;
const unsigned int baseRetryTimeMs;
struct TransferItem : public std::enable_shared_from_this<TransferItem> struct TransferItem : public std::enable_shared_from_this<TransferItem>
{ {
@ -442,7 +443,7 @@ struct curlFileTransfer : public FileTransfer
|| writtenToSink == 0 || writtenToSink == 0
|| (acceptRanges && encoding.empty()))) || (acceptRanges && encoding.empty())))
{ {
int ms = request.baseRetryTimeMs * std::pow(2.0f, attempt - 1 + std::uniform_real_distribution<>(0.0, 0.5)(fileTransfer.mt19937)); int ms = fileTransfer.baseRetryTimeMs * std::pow(2.0f, attempt - 1 + std::uniform_real_distribution<>(0.0, 0.5)(fileTransfer.mt19937));
if (writtenToSink) if (writtenToSink)
warn("%s; retrying from offset %d in %d ms", exc.what(), writtenToSink, ms); warn("%s; retrying from offset %d in %d ms", exc.what(), writtenToSink, ms);
else else
@ -471,8 +472,9 @@ struct curlFileTransfer : public FileTransfer
std::thread workerThread; std::thread workerThread;
curlFileTransfer() curlFileTransfer(unsigned int baseRetryTimeMs)
: mt19937(rd()) : mt19937(rd())
, baseRetryTimeMs(baseRetryTimeMs)
{ {
static std::once_flag globalInit; static std::once_flag globalInit;
std::call_once(globalInit, curl_global_init, CURL_GLOBAL_ALL); std::call_once(globalInit, curl_global_init, CURL_GLOBAL_ALL);
@ -874,24 +876,24 @@ struct curlFileTransfer : public FileTransfer
} }
}; };
ref<curlFileTransfer> makeCurlFileTransfer() ref<curlFileTransfer> makeCurlFileTransfer(std::optional<unsigned int> baseRetryTimeMs)
{ {
return make_ref<curlFileTransfer>(); return make_ref<curlFileTransfer>(baseRetryTimeMs.value_or(250));
} }
ref<FileTransfer> getFileTransfer() ref<FileTransfer> getFileTransfer()
{ {
static ref<curlFileTransfer> fileTransfer = makeCurlFileTransfer(); static ref<curlFileTransfer> fileTransfer = makeCurlFileTransfer({});
if (fileTransfer->state_.lock()->quit) if (fileTransfer->state_.lock()->quit)
fileTransfer = makeCurlFileTransfer(); fileTransfer = makeCurlFileTransfer({});
return fileTransfer; return fileTransfer;
} }
ref<FileTransfer> makeFileTransfer() ref<FileTransfer> makeFileTransfer(std::optional<unsigned int> baseRetryTimeMs)
{ {
return makeCurlFileTransfer(); return makeCurlFileTransfer(baseRetryTimeMs);
} }
template<typename... Args> template<typename... Args>

3
src/libstore/filetransfer.hh
View file

@ -60,7 +60,6 @@ struct FileTransferRequest
bool verifyTLS = true; bool verifyTLS = true;
bool head = false; bool head = false;
size_t tries = fileTransferSettings.tries; size_t tries = fileTransferSettings.tries;
unsigned int baseRetryTimeMs = 250;
FileTransferRequest(std::string_view uri) FileTransferRequest(std::string_view uri)
: uri(uri) { } : uri(uri) { }
@ -123,7 +122,7 @@ ref<FileTransfer> getFileTransfer();
* *
* Prefer getFileTransfer() to this; see its docs for why. * Prefer getFileTransfer() to this; see its docs for why.
*/ */
ref<FileTransfer> makeFileTransfer(); ref<FileTransfer> makeFileTransfer(std::optional<unsigned int> baseRetryTimeMs = {});
class FileTransferError : public Error class FileTransferError : public Error
{ {

6
tests/unit/libstore/filetransfer.cc
View file

@ -178,9 +178,8 @@ TEST(FileTransfer, NOT_ON_DARWIN(defersFailures))
// might only do so once its internal buffer has already been filled.) // might only do so once its internal buffer has already been filled.)
return std::string(1024 * 1024, ' '); return std::string(1024 * 1024, ' ');
}); });
auto ft = makeFileTransfer(); auto ft = makeFileTransfer(0);
FileTransferRequest req(fmt("http://[::1]:%d/index", port)); FileTransferRequest req(fmt("http://[::1]:%d/index", port));
req.baseRetryTimeMs = 0;
auto src = ft->download(std::move(req)); auto src = ft->download(std::move(req));
ASSERT_THROW(src->drain(), FileTransferError); ASSERT_THROW(src->drain(), FileTransferError);
} }
@ -216,9 +215,8 @@ TEST(FileTransfer, usesIntermediateLinkHeaders)
[] { return ""; }}, [] { return ""; }},
{"200 ok", "content-length: 1\r\n", [] { return "a"; }}, {"200 ok", "content-length: 1\r\n", [] { return "a"; }},
}); });
auto ft = makeFileTransfer(); auto ft = makeFileTransfer(0);
FileTransferRequest req(fmt("http://[::1]:%d/first", port)); FileTransferRequest req(fmt("http://[::1]:%d/first", port));
req.baseRetryTimeMs = 0;
auto result = ft->enqueueDownload(req).get(); auto result = ft->enqueueDownload(req).get();
ASSERT_EQ(result.immutableUrl, "http://foo"); ASSERT_EQ(result.immutableUrl, "http://foo");
} }