Skip to content

Commit

Permalink
Fixes azure blob binary caching by not sending put headers for get re…
Browse files Browse the repository at this point in the history 
…quest and by sending headers only once (#592)
  • Loading branch information
@autoantwort
autoantwort authored Jun 17, 2022
1 parent b3b6577 commit 9268e36
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 30 deletions.
3 changes: 2 additions & 1 deletion include/vcpkg/base/downloads.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ namespace vcpkg
View<std::string> azure_blob_headers();

std::vector<int> download_files(Filesystem& fs,
View<std::tuple<std::string, View<std::string>, Path>> url_header_path_tuples);
View<std::pair<std::string, Path>> url_pairs,
View<std::string> headers);
ExpectedS<int> put_file(const Filesystem&, StringView url, View<std::string> headers, const Path& file);
std::vector<int> url_heads(View<std::string> urls, View<std::string> headers, View<std::string> secrets);
std::string replace_secrets(std::string input, View<std::string> secrets);
Expand Down
3 changes: 2 additions & 1 deletion include/vcpkg/binarycaching.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ namespace vcpkg
struct UrlTemplate
{
std::string url_template;
std::vector<std::string> headers;
std::vector<std::string> headers_for_put;
std::vector<std::string> headers_for_get;

LocalizedString valid();
std::string instantiate_variables(const Dependencies::InstallPlanAction& action) const;
Expand Down
35 changes: 18 additions & 17 deletions src/vcpkg/base/downloads.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,8 @@ namespace vcpkg
}

static void download_files_inner(Filesystem&,
View<std::tuple<std::string, View<std::string>, Path>> url_header_path_tuples,
View<std::pair<std::string, Path>> url_pairs,
View<std::string> headers,
std::vector<int>* out)
{
for (auto i : {100, 1000, 10000, 0})
Expand All @@ -368,13 +369,13 @@ namespace vcpkg
.string_arg("--location")
.string_arg("-w")
.string_arg(Strings::concat(guid_marker, " %{http_code}\\n"));
for (auto&& url : url_header_path_tuples)
for (StringView header : headers)
{
cmd.string_arg(std::get<0>(url)).string_arg("-o").string_arg(std::get<2>(url));
for (StringView header : std::get<1>(url))
{
cmd.string_arg("-H").string_arg(header);
}
cmd.string_arg("-H").string_arg(header);
}
for (auto&& url : url_pairs)
{
cmd.string_arg(url.first).string_arg("-o").string_arg(url.second);
}
auto res = cmd_execute_and_stream_lines(cmd, [out](StringView line) {
if (Strings::starts_with(line, guid_marker))
Expand All @@ -384,13 +385,13 @@ namespace vcpkg
}).value_or_exit(VCPKG_LINE_INFO);
Checks::check_exit(VCPKG_LINE_INFO, res == 0, "Error: curl failed to execute with exit code: %d", res);

if (start_size + url_header_path_tuples.size() > out->size())
if (start_size + url_pairs.size() > out->size())
{
// curl stopped before finishing all downloads; retry after some time
msg::println_warning(msgUnexpectedErrorDuringBulkDownload);
std::this_thread::sleep_for(std::chrono::milliseconds(i));
url_header_path_tuples = View<std::tuple<std::string, View<std::string>, Path>>{
url_header_path_tuples.begin() + out->size() - start_size, url_header_path_tuples.end()};
url_pairs =
View<std::pair<std::string, Path>>{url_pairs.begin() + out->size() - start_size, url_pairs.end()};
}
else
{
Expand All @@ -399,27 +400,27 @@ namespace vcpkg
}
}
std::vector<int> download_files(Filesystem& fs,
View<std::tuple<std::string, View<std::string>, Path>> url_header_path_tuples)
View<std::pair<std::string, Path>> url_pairs,
View<std::string> headers)
{
static constexpr size_t batch_size = 50;

std::vector<int> ret;

size_t i = 0;
for (; i + batch_size <= url_header_path_tuples.size(); i += batch_size)
for (; i + batch_size <= url_pairs.size(); i += batch_size)
{
download_files_inner(fs, {url_header_path_tuples.data() + i, batch_size}, &ret);
download_files_inner(fs, {url_pairs.data() + i, batch_size}, headers, &ret);
}
if (i != url_header_path_tuples.size())
download_files_inner(fs, {url_header_path_tuples.begin() + i, url_header_path_tuples.end()}, &ret);
if (i != url_pairs.size()) download_files_inner(fs, {url_pairs.begin() + i, url_pairs.end()}, headers, &ret);

Checks::check_exit(
VCPKG_LINE_INFO,
ret.size() == url_header_path_tuples.size(),
ret.size() == url_pairs.size(),
"Error: curl returned a different number of response codes than were expected for the request (",
ret.size(),
" vs expected ",
url_header_path_tuples.size(),
url_pairs.size(),
")");
return ret;
}
Expand Down
21 changes: 10 additions & 11 deletions src/vcpkg/binarycaching.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ namespace
for (auto&& put_url_template : m_put_url_templates)
{
auto url = put_url_template.instantiate_variables(action);
auto maybe_success = put_file(fs, url, put_url_template.headers, tmp_archive_path);
auto maybe_success = put_file(fs, url, put_url_template.headers_for_put, tmp_archive_path);
if (maybe_success)
{
http_remotes_pushed++;
Expand Down Expand Up @@ -522,7 +522,7 @@ namespace
const auto timer = ElapsedTimer::create_started();
auto& fs = paths.get_filesystem();
size_t this_restore_count = 0;
std::vector<std::tuple<std::string, View<std::string>, Path>> url_paths;
std::vector<std::pair<std::string, Path>> url_paths;
std::vector<size_t> url_indices;
for (auto&& url_template : m_url_templates)
{
Expand All @@ -539,16 +539,15 @@ namespace
auto&& action = actions[idx];
clean_prepare_dir(fs, paths.package_dir(action.spec));
auto uri = url_template.instantiate_variables(action);
url_paths.emplace_back(
std::move(uri), url_template.headers, make_temp_archive_path(paths.buildtrees(), action.spec));
url_paths.emplace_back(std::move(uri), make_temp_archive_path(paths.buildtrees(), action.spec));
url_indices.push_back(idx);
}

if (url_paths.empty()) break;

print2("Attempting to fetch ", url_paths.size(), " packages from HTTP servers.\n");

auto codes = download_files(fs, url_paths);
auto codes = download_files(fs, url_paths, url_template.headers_for_get);
std::vector<size_t> action_idxs;
std::vector<Command> jobs;
for (size_t i = 0; i < codes.size(); ++i)
Expand All @@ -559,7 +558,7 @@ namespace
jobs.push_back(decompress_zip_archive_cmd(paths.get_tool_cache(),
stdout_sink,
paths.package_dir(actions[url_indices[i]].spec),
std::get<2>(url_paths[i])));
url_paths[i].second));
}
}
auto job_results = decompress_in_parallel(jobs);
Expand All @@ -569,12 +568,12 @@ namespace
if (job_results[j])
{
++this_restore_count;
fs.remove(std::get<2>(url_paths[i]), VCPKG_LINE_INFO);
fs.remove(url_paths[i].second, VCPKG_LINE_INFO);
cache_status[url_indices[i]]->mark_restored();
}
else
{
Debug::print("Failed to decompress ", std::get<2>(url_paths[i]), '\n');
Debug::print("Failed to decompress ", url_paths[i].second, '\n');
}
}
}
Expand Down Expand Up @@ -1353,7 +1352,6 @@ namespace

namespace vcpkg
{

LocalizedString UrlTemplate::valid()
{
std::vector<std::string> invalid_keys;
Expand Down Expand Up @@ -1916,7 +1914,7 @@ namespace
state->secrets.push_back(segments[2].second);
UrlTemplate url_template = {p};
auto headers = azure_blob_headers();
url_template.headers.assign(headers.begin(), headers.end());
url_template.headers_for_put.assign(headers.begin(), headers.end());
handle_readwrite(
state->url_templates_to_get, state->url_templates_to_put, std::move(url_template), segments, 3);
}
Expand Down Expand Up @@ -2068,7 +2066,8 @@ namespace
}
if (segments.size() == 4)
{
url_template.headers.push_back(segments[3].second);
url_template.headers_for_get.push_back(segments[3].second);
url_template.headers_for_put.push_back(segments[3].second);
}
handle_readwrite(
state->url_templates_to_get, state->url_templates_to_put, std::move(url_template), segments, 2);
Expand Down

0 comments on commit 9268e36

Please sign in to comment.