From b7140e661e6e8ccf7b0ae5e5a2b302c266d8a84d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20S=C5=82owik?= Date: Sun, 23 May 2021 14:18:13 +0200 Subject: [PATCH 1/3] Fix Chia-Network/chia-blockchain/issues/5863 --- src/plotter_disk.hpp | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/src/plotter_disk.hpp b/src/plotter_disk.hpp index d2038b7be..6075c6dc5 100644 --- a/src/plotter_disk.hpp +++ b/src/plotter_disk.hpp @@ -169,15 +169,15 @@ class DiskPlotter { std::vector tmp_1_filenames = std::vector(); // The table0 file will be used for sort on disk spare. tables 1-7 are stored in their own - // file. + // file. Ensure all paths are canonical. tmp_1_filenames.push_back(fs::path(tmp_dirname) / fs::path(filename + ".sort.tmp")); for (size_t i = 1; i <= 7; i++) { tmp_1_filenames.push_back( - fs::path(tmp_dirname) / fs::path(filename + ".table" + std::to_string(i) + ".tmp")); + fs::weakly_canonical(fs::path(tmp_dirname) / fs::path(filename + ".table" + std::to_string(i) + ".tmp"))); } - fs::path tmp_2_filename = fs::path(tmp2_dirname) / fs::path(filename + ".2.tmp"); - fs::path final_2_filename = fs::path(final_dirname) / fs::path(filename + ".2.tmp"); - fs::path final_filename = fs::path(final_dirname) / fs::path(filename); + fs::path tmp_2_filename = fs::weakly_canonical(fs::path(tmp2_dirname) / fs::path(filename + ".2.tmp")); + fs::path final_2_filename = fs::weakly_canonical(fs::path(final_dirname) / fs::path(filename + ".2.tmp")); + fs::path final_filename = fs::weakly_canonical(fs::path(final_dirname) / fs::path(filename)); // Check if the paths exist if (!fs::exists(tmp_dirname)) { @@ -364,6 +364,7 @@ class DiskPlotter { fs::remove(p); } + bool bHardlinked = false; bool bCopied = false; bool bRenamed = false; Timer copy; @@ -381,7 +382,25 @@ class DiskPlotter { << final_filename << std::endl; } } else { - if (!bCopied) { + if (!bHardlinked) { + fs::create_hard_link( + tmp_2_filename, final_2_filename, ec); + if (ec.value() != 0) { + std::cout << "Could not hardlink " << final_2_filename << " to " + << tmp_2_filename << ". Error " << ec.message() + << ". Trying to copy instead." << std::endl; + } else { + std::cout << "Hardlinked final file from " << final_2_filename << " to " + << tmp_2_filename << std::endl; + copy.PrintElapsed("Link time ="); + bHardlinked = true; + + bool removed_2 = fs::remove(tmp_2_filename); + std::cout << "Removed temp2 file " << tmp_2_filename << "? " << removed_2 + << std::endl; + } + } + if (!bHardlinked && !bCopied) { fs::copy( tmp_2_filename, final_2_filename, fs::copy_options::overwrite_existing, ec); if (ec.value() != 0) { @@ -399,7 +418,7 @@ class DiskPlotter { << std::endl; } } - if (bCopied && (!bRenamed)) { + if ((bCopied || bHardlinked) && (!bRenamed)) { fs::rename(final_2_filename, final_filename, ec); if (ec.value() != 0) { std::cout << "Could not rename " << tmp_2_filename << " to " From fae536bc157062bb1f1bb943667fb542e4a8fbeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20S=C5=82owik?= Date: Sun, 23 May 2021 14:33:30 +0200 Subject: [PATCH 2/3] Cleanup, reduce number of canonical requests. --- src/plotter_disk.hpp | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/plotter_disk.hpp b/src/plotter_disk.hpp index 6075c6dc5..17f0df5e1 100644 --- a/src/plotter_disk.hpp +++ b/src/plotter_disk.hpp @@ -168,17 +168,6 @@ class DiskPlotter { // Cross platform way to concatenate paths, gulrak library. std::vector tmp_1_filenames = std::vector(); - // The table0 file will be used for sort on disk spare. tables 1-7 are stored in their own - // file. Ensure all paths are canonical. - tmp_1_filenames.push_back(fs::path(tmp_dirname) / fs::path(filename + ".sort.tmp")); - for (size_t i = 1; i <= 7; i++) { - tmp_1_filenames.push_back( - fs::weakly_canonical(fs::path(tmp_dirname) / fs::path(filename + ".table" + std::to_string(i) + ".tmp"))); - } - fs::path tmp_2_filename = fs::weakly_canonical(fs::path(tmp2_dirname) / fs::path(filename + ".2.tmp")); - fs::path final_2_filename = fs::weakly_canonical(fs::path(final_dirname) / fs::path(filename + ".2.tmp")); - fs::path final_filename = fs::weakly_canonical(fs::path(final_dirname) / fs::path(filename)); - // Check if the paths exist if (!fs::exists(tmp_dirname)) { throw InvalidValueException("Temp directory " + tmp_dirname + " does not exist"); @@ -191,6 +180,23 @@ class DiskPlotter { if (!fs::exists(final_dirname)) { throw InvalidValueException("Final directory " + final_dirname + " does not exist"); } + + // Ensure all paths are canonical. + fs::path tmp_dirname_canonical = fs::canonical(fs::path(tmp_dirname)); + fs::path tmp2_dirname_canonical = fs::canonical(fs::path(tmp2_dirname)); + fs::path final_dirname_canonical = fs::canonical(fs::path(final_dirname)); + + // The table0 file will be used for sort on disk spare. tables 1-7 are stored in their own + // file. + tmp_1_filenames.push_back(tmp_dirname_canonical / fs::path(filename + ".sort.tmp")); + for (size_t i = 1; i <= 7; i++) { + tmp_1_filenames.push_back( + tmp_dirname_canonical / fs::path(filename + ".table" + std::to_string(i) + ".tmp")); + } + fs::path tmp_2_filename = tmp2_dirname_canonical / fs::path(filename + ".2.tmp"); + fs::path final_2_filename = final_dirname_canonical / fs::path(filename + ".2.tmp"); + fs::path final_filename = final_dirname_canonical / fs::path(filename); + for (fs::path& p : tmp_1_filenames) { fs::remove(p); } From 268213f6f1cd5f81d020589464f524f7b7bbe78a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20S=C5=82owik?= Date: Thu, 1 Jul 2021 13:58:06 +0200 Subject: [PATCH 3/3] Change move strategy to "try rename" then "copy" --- src/plotter_disk.hpp | 80 +++++++++++++------------------------------- 1 file changed, 23 insertions(+), 57 deletions(-) diff --git a/src/plotter_disk.hpp b/src/plotter_disk.hpp index e98968798..6d5d0c213 100644 --- a/src/plotter_disk.hpp +++ b/src/plotter_disk.hpp @@ -196,7 +196,6 @@ class DiskPlotter { tmp_dirname_canonical / fs::path(filename + ".table" + std::to_string(i) + ".tmp")); } fs::path tmp_2_filename = tmp2_dirname_canonical / fs::path(filename + ".2.tmp"); - fs::path final_2_filename = final_dirname_canonical / fs::path(filename + ".2.tmp"); fs::path final_filename = final_dirname_canonical / fs::path(filename); for (fs::path& p : tmp_1_filenames) { @@ -371,82 +370,49 @@ class DiskPlotter { fs::remove(p); } - bool bHardlinked = false; - bool bCopied = false; - bool bRenamed = false; + bool bMoved = false; Timer copy; do { std::error_code ec; - if (tmp_2_filename.parent_path() == final_filename.parent_path()) { + if (!bMoved) { fs::rename(tmp_2_filename, final_filename, ec); if (ec.value() != 0) { std::cout << "Could not rename " << tmp_2_filename << " to " << final_filename - << ". Error " << ec.message() << ". Retrying in five minutes." + << ". Error " << ec.message() << ". Trying to copy instead." << std::endl; } else { - bRenamed = true; + bMoved = true; std::cout << "Renamed final file from " << tmp_2_filename << " to " << final_filename << std::endl; } - } else { - if (!bHardlinked) { - fs::create_hard_link( - tmp_2_filename, final_2_filename, ec); - if (ec.value() != 0) { - std::cout << "Could not hardlink " << final_2_filename << " to " - << tmp_2_filename << ". Error " << ec.message() - << ". Trying to copy instead." << std::endl; - } else { - std::cout << "Hardlinked final file from " << final_2_filename << " to " - << tmp_2_filename << std::endl; - copy.PrintElapsed("Link time ="); - bHardlinked = true; - - bool removed_2 = fs::remove(tmp_2_filename); - std::cout << "Removed temp2 file " << tmp_2_filename << "? " << removed_2 - << std::endl; - } - } - if (!bHardlinked && !bCopied) { - fs::copy( - tmp_2_filename, final_2_filename, fs::copy_options::overwrite_existing, ec); - if (ec.value() != 0) { - std::cout << "Could not copy " << tmp_2_filename << " to " - << final_2_filename << ". Error " << ec.message() - << ". Retrying in five minutes." << std::endl; - } else { - std::cout << "Copied final file from " << tmp_2_filename << " to " - << final_2_filename << std::endl; - copy.PrintElapsed("Copy time ="); - bCopied = true; - - bool removed_2 = fs::remove(tmp_2_filename); - std::cout << "Removed temp2 file " << tmp_2_filename << "? " << removed_2 - << std::endl; - } - } - if ((bCopied || bHardlinked) && (!bRenamed)) { - fs::rename(final_2_filename, final_filename, ec); - if (ec.value() != 0) { - std::cout << "Could not rename " << tmp_2_filename << " to " - << final_filename << ". Error " << ec.message() - << ". Retrying in five minutes." << std::endl; - } else { - std::cout << "Renamed final file from " << final_2_filename << " to " - << final_filename << std::endl; - bRenamed = true; - } + } + if (!bMoved) { + fs::copy( + tmp_2_filename, final_filename, fs::copy_options::overwrite_existing, ec); + if (ec.value() != 0) { + std::cout << "Could not copy " << tmp_2_filename << " to " + << final_filename << ". Error " << ec.message() + << ". Retrying in five minutes." << std::endl; + } else { + std::cout << "Copied final file from " << tmp_2_filename << " to " + << final_filename << std::endl; + copy.PrintElapsed("Copy time ="); + bMoved = true; + + bool removed_2 = fs::remove(tmp_2_filename); + std::cout << "Removed temp2 file " << tmp_2_filename << "? " << removed_2 + << std::endl; } } - if (!bRenamed) { + if (!bMoved) { #ifdef _WIN32 Sleep(5 * 60000); #else sleep(5 * 60); #endif } - } while (!bRenamed); + } while (!bMoved); } private: