mirror of
https://github.com/ggml-org/llama.cpp.git
synced 2026-09-17 20:31:47 +02:00
download: keep concurrent downloads of one blob apart
Every process writes the same path + .downloadInProgress, so a second download of the same blob finds that file, takes it for its own partial transfer and asks for the bytes after it, which produces a corrupt result. The in-progress file now carries the pid of the process writing it. std::rename also replaces an existing destination on POSIX but fails on Windows, so a download whose blob appeared in the meantime is dropped after every retry and an etag rewrite silently keeps the old value. std::filesystem::rename has the POSIX behaviour everywhere, and the error now carries the reason reported by the system.
This commit is contained in:
+27
-6
@@ -38,6 +38,7 @@
|
||||
// isatty
|
||||
#if defined(_WIN32)
|
||||
#include <io.h>
|
||||
#include <process.h>
|
||||
#else
|
||||
#include <unistd.h>
|
||||
#endif
|
||||
@@ -46,6 +47,17 @@
|
||||
// downloader
|
||||
//
|
||||
|
||||
// the in-progress file of a download is private to the process writing it, so
|
||||
// two downloads of the same blob never share a partial file
|
||||
static std::string download_temporary_path(const std::string & path) {
|
||||
#ifdef _WIN32
|
||||
const int pid = _getpid();
|
||||
#else
|
||||
const int pid = getpid();
|
||||
#endif
|
||||
return path + "." + std::to_string(pid) + ".downloadInProgress";
|
||||
}
|
||||
|
||||
// validate repo name format: owner/repo
|
||||
static void write_file(const std::string & fname, const std::string & content) {
|
||||
const std::string fname_tmp = fname + ".tmp";
|
||||
@@ -58,9 +70,13 @@ static void write_file(const std::string & fname, const std::string & content) {
|
||||
file << content;
|
||||
file.close();
|
||||
|
||||
// Makes write atomic
|
||||
if (rename(fname_tmp.c_str(), fname.c_str()) != 0) {
|
||||
LOG_ERR("%s: unable to rename file: %s to %s\n", __func__, fname_tmp.c_str(), fname.c_str());
|
||||
// makes the write atomic, and replaces an existing destination on
|
||||
// every platform, which std::rename does not do on Windows
|
||||
std::error_code ec;
|
||||
std::filesystem::rename(fname_tmp, fname, ec);
|
||||
if (ec) {
|
||||
LOG_ERR("%s: unable to rename file: %s to %s (%s)\n",
|
||||
__func__, fname_tmp.c_str(), fname.c_str(), ec.message().c_str());
|
||||
// If rename fails, try to delete the temporary file
|
||||
if (remove(fname_tmp.c_str()) != 0) {
|
||||
LOG_ERR("%s: unable to delete temporary file: %s\n", __func__, fname_tmp.c_str());
|
||||
@@ -366,7 +382,7 @@ static int common_download_file_single_online(const std::string & url,
|
||||
}
|
||||
|
||||
bool success = false;
|
||||
const std::string path_temporary = path + ".downloadInProgress";
|
||||
const std::string path_temporary = download_temporary_path(path);
|
||||
int delay = retry_delay_seconds;
|
||||
|
||||
if (opts.callback) {
|
||||
@@ -401,8 +417,13 @@ static int common_download_file_single_online(const std::string & url,
|
||||
path_temporary.c_str(), etag.c_str());
|
||||
|
||||
if (common_pull_file(cli, parts.path, path_temporary, supports_ranges, p, opts.callback)) {
|
||||
if (std::rename(path_temporary.c_str(), path.c_str()) != 0) {
|
||||
LOG_ERR("%s: unable to rename file: %s to %s\n", __func__, path_temporary.c_str(), path.c_str());
|
||||
// replaces an existing destination on every platform, which
|
||||
// std::rename does not do on Windows
|
||||
std::error_code ec;
|
||||
std::filesystem::rename(path_temporary, path, ec);
|
||||
if (ec) {
|
||||
LOG_ERR("%s: unable to rename file: %s to %s (%s)\n",
|
||||
__func__, path_temporary.c_str(), path.c_str(), ec.message().c_str());
|
||||
break;
|
||||
}
|
||||
if (!etag.empty() && !skip_etag) {
|
||||
|
||||
Reference in New Issue
Block a user