From c507c19eeaec88abfe875e82a971c31364e93c0f Mon Sep 17 00:00:00 2001
From: Philipp Spilger <philipp.spilger@kip.uni-heidelberg.de>
Date: Thu, 22 Feb 2024 11:20:09 +0100
Subject: [PATCH] feat: Use atomic move for updating build cache

* removes need to lock build cache when updating

Change-Id: I0374ba4df9709f97a1eda6f27f0215de445d4e03
---
 lib/yashchiki/commons.sh                      | 63 -------------------
 .../update_build_cache_in_container.sh        | 26 ++++----
 2 files changed, 15 insertions(+), 74 deletions(-)

diff --git a/lib/yashchiki/commons.sh b/lib/yashchiki/commons.sh
index 6529a50a..532c3d72 100755
--- a/lib/yashchiki/commons.sh
+++ b/lib/yashchiki/commons.sh
@@ -25,8 +25,6 @@ export MY_SPACK_BIN=/opt/spack/bin/spack
 export MY_SPACK_CMD="${MY_SPACK_BIN} --config-scope ${YASHCHIKI_SPACK_CONFIG}"
 export MY_SPACK_VIEW_PREFIX="/opt/spack_views"
 
-export LOCK_FILENAME=lock
-
 # NOTE: build caches contain relavite symlinks to preserved_packages, so the
 # relation that build_caches and preserved_packages are in the same folder
 # should be maintained inside the container!
@@ -41,10 +39,8 @@ export BUILD_CACHE_OUTSIDE
 
 BASE_BUILD_CACHE_INSIDE="/opt/build_cache"
 BUILD_CACHE_INSIDE="${BASE_BUILD_CACHE_INSIDE}/${YASHCHIKI_BUILD_CACHE_NAME}"
-BUILD_CACHE_LOCK="${BUILD_CACHE_INSIDE}/${LOCK_FILENAME}"
 export BASE_BUILD_CACHE_INSIDE
 export BUILD_CACHE_INSIDE
-export BUILD_CACHE_LOCK
 
 SOURCE_CACHE_DIR="${YASHCHIKI_CACHES_ROOT}/download_cache"
 export SOURCE_CACHE_DIR
@@ -204,63 +200,6 @@ populate_views() {
 
 
 
-#################################
-# HELPER FUNCTIONS NEEDED BELOW #
-#################################
-
-# Usage:
-#       lock_file [-e] [-w <sec>] <file>
-#
-# Lock the given file, the file will be unlocked once the process exits. Make
-# sure to only use it in subshells to automatically unlock afterwards.
-#
-# Args:
-#   -e          Lock exculsively (otherwise shared)
-#   -w <secs>   How long to wait for lock until retrying. [default: 10]
-#
-lock_file() {
-    local OPTIND
-    local opt
-    local args_flock=()
-    local exclusive=0
-    local info_exclusive=""
-    local wait_secs=10
-    local opts OPTIND OPTARG
-
-    while getopts ":ew:" opt
-    do
-        case "${opt}" in
-            e) exclusive=1 ;;
-            w) wait_secs="${OPTARG}" ;;
-            *) echo -e "Invalid option to lock_file(): $OPTARG\n" >&2; exit 1 ;;
-        esac
-    done
-    shift $(( OPTIND - 1 ))
-
-    local fd_lock
-    local filename_lock="$1"
-    # ensure that we can always access lockfile 
-    if [ ! -f "${filename_lock}" ]; then
-        touch "${filename_lock}"
-        chmod 777 "${filename_lock}"
-    fi
-    exec {fd_lock}>"${filename_lock}"
-
-    if (( exclusive == 1 )); then
-        args_flock+=( "-e" )
-        info_exclusive="(exclusively) "
-    else
-        args_flock+=( "-s" )
-        info_exclusive="(shared) "
-    fi
-
-    while /bin/true; do
-        echo "Obtaining build_cache lock ${info_exclusive}from ${filename_lock}." 1>&2
-        flock ${args_flock[*]} -w "${wait_secs}" ${fd_lock} && break \
-            || echo "Could not lock ${filename_lock}, retrying in ${wait_secs} seconds.." 1>&2
-    done
-}
-
 ###################################
 # HELPER FUNCTIONS FOR BUILDCACHE #
 ###################################
@@ -382,9 +321,7 @@ get_specfiles() {
 # Install the given set of packages from yashchiki's buildcache.
 install_from_buildcache() {
     local install_failed=0
-    # don't forget to unlock builcache in case of error, but then propagate
     (
-        lock_file "${BUILD_CACHE_LOCK}"
         _install_from_buildcache "${@}"
     ) || install_failed=1
 
diff --git a/lib/yashchiki/update_build_cache_in_container.sh b/lib/yashchiki/update_build_cache_in_container.sh
index c16ee96f..9615bdbf 100755
--- a/lib/yashchiki/update_build_cache_in_container.sh
+++ b/lib/yashchiki/update_build_cache_in_container.sh
@@ -3,7 +3,6 @@
 # General plan of action:
 #
 # Create a hard-linked copy, update that copy and only copy new additions back.
-# This ensure minimum locking time.
 #
 
 set -euo pipefail
@@ -69,14 +68,6 @@ get_hashes_to_store() {
     comm -13 <(get_hashes_in_buildcache "${base_buildcache}") <(get_hashes_in_spack)
 }
 
-if [ "${destination_folder}" = "${base_buildcache}" ]; then
-    # if the destination folder is the same as the base buildcache, we need to
-    # lock exclusively since we put things into the buildcache
-    lock_file -e "${base_buildcache}/${LOCK_FILENAME}"
-else
-    lock_file "${base_buildcache}/${LOCK_FILENAME}"
-fi
-
 args_progress="--eta"
 if (( quiet == 1 )); then
     args_progress=""
@@ -85,10 +76,23 @@ fi
 # find requires current working directory to be readable by spack user
 cd ${MY_SPACK_FOLDER}
 
+# Create tmpdir in destination folder to compress into,
+# atomically move compressed files into the destination folder afterwards
+tmpdir_in_destination_folder="$(mktemp -d --tmpdir=${destination_folder})"
+
+rm_tmpdir() {
+    rm -r $tmpdir_in_destination_folder
+}
+trap rm_tmpdir EXIT
+
 get_hashes_to_store \
     | parallel -r ${args_progress} -j${YASHCHIKI_JOBS} \
-        tar Pcfz "${destination_folder}/{}.tar.gz" \"\$\(spack location -i /{}\)\"
+        tar Pcfz "${tmpdir_in_destination_folder}/{}.tar.gz" \"\$\(spack location -i /{}\)\"
 
 # verify integrity (of actual files, not possible symlinks)
-find "${destination_folder}" -type f -name "*.tar.gz" -print0 \
+find "${tmpdir_in_destination_folder}" -type f -name "*.tar.gz" -print0 \
     | parallel -r -0 -j${YASHCHIKI_JOBS} "tar Ptf '{}' 1>/dev/null"
+
+# atomically move files into destination folder
+find "${tmpdir_in_destination_folder}" -type f -name "*.tar.gz" -print0 \
+    | parallel -r -0 -j${YASHCHIKI_JOBS} "mv '{}' ${destination_folder} 1>/dev/null"
-- 
GitLab