Skip to content

Commit

Permalink
Add a TTL to cached binary indices
Browse files Browse the repository at this point in the history
To allow for index re-fetches in sufficiently long-running Spack commands
  • Loading branch information
blue42u committed Aug 27, 2022
1 parent d73dd5b commit a719c9d
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 15 deletions.
4 changes: 4 additions & 0 deletions etc/spack/defaults/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -201,3 +201,7 @@ config:
# building and installing packages. This gives information about Spack's
# current progress as well as the current and total number of packages.
terminal_title: false

# Number of seconds a buildcache's index.json is cached locally before probing
# for updates, within a single Spack invocation. Defaults to 10 minutes.
binary_index_ttl: 600
42 changes: 31 additions & 11 deletions lib/spack/spack/binary_distribution.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import sys
import tarfile
import tempfile
import time
import traceback
import warnings
from contextlib import closing
Expand Down Expand Up @@ -105,6 +106,9 @@ def __init__(self, cache_root):
# cache (_mirrors_for_spec)
self._specs_already_associated = set()

# mapping from mirror urls to the time.time() of the last index fetch.
self._last_fetch_times = {}

# _mirrors_for_spec is a dictionary mapping DAG hashes to lists of
# entries indicating mirrors where that concrete spec can be found.
# Each entry is a dictionary consisting of:
Expand Down Expand Up @@ -137,6 +141,7 @@ def clear(self):
self._index_file_cache = None
self._local_index_cache = None
self._specs_already_associated = set()
self._last_fetch_times = {}
self._mirrors_for_spec = {}

def _write_local_index_cache(self):
Expand Down Expand Up @@ -327,24 +332,36 @@ def update(self):

fetch_errors = []
all_methods_failed = True
ttl = spack.config.get("config:binary_index_ttl", 600)
now = time.time()

for cached_mirror_url in self._local_index_cache:
cache_entry = self._local_index_cache[cached_mirror_url]
cached_index_hash = cache_entry["index_hash"]
cached_index_path = cache_entry["index_path"]
if cached_mirror_url in configured_mirror_urls:
# May need to fetch the index and update the local caches
try:
needs_regen = self._fetch_and_cache_index(
cached_mirror_url, expect_hash=cached_index_hash
)
# Only do a fetch if the last fetch was longer than TTL ago
if (
ttl > 0
and cached_mirror_url in self._last_fetch_times
and now - self._last_fetch_times[cached_mirror_url] < ttl
):
# The fetch worked last time, so don't error
all_methods_failed = False
except FetchCacheError as fetch_error:
needs_regen = False
fetch_errors.extend(fetch_error.errors)
# The need to regenerate implies a need to clear as well.
spec_cache_clear_needed |= needs_regen
spec_cache_regenerate_needed |= needs_regen
else:
# May need to fetch the index and update the local caches
try:
needs_regen = self._fetch_and_cache_index(
cached_mirror_url, expect_hash=cached_index_hash
)
self._last_fetch_times[cached_mirror_url] = now
all_methods_failed = False
except FetchCacheError as fetch_error:
needs_regen = False
fetch_errors.extend(fetch_error.errors)
# The need to regenerate implies a need to clear as well.
spec_cache_clear_needed |= needs_regen
spec_cache_regenerate_needed |= needs_regen
else:
# No longer have this mirror, cached index should be removed
items_to_remove.append(
Expand All @@ -353,6 +370,8 @@ def update(self):
"cache_key": os.path.join(self._index_cache_root, cached_index_path),
}
)
if cached_mirror_url in self._last_fetch_times:
del self._last_fetch_times[cached_mirror_url]
spec_cache_clear_needed = True
spec_cache_regenerate_needed = True

Expand All @@ -371,6 +390,7 @@ def update(self):
# Need to fetch the index and update the local caches
try:
needs_regen = self._fetch_and_cache_index(mirror_url)
self._last_fetch_times[mirror_url] = now
all_methods_failed = False
except FetchCacheError as fetch_error:
fetch_errors.extend(fetch_error.errors)
Expand Down
1 change: 1 addition & 0 deletions lib/spack/spack/schema/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"binary_index_root": {"type": "string"},
"url_fetch_method": {"type": "string", "enum": ["urllib", "curl"]},
"additional_external_search_paths": {"type": "array", "items": {"type": "string"}},
"binary_index_ttl": {"type": "integer", "minimum": 0},
},
"deprecatedProperties": {
"properties": ["module_roots"],
Expand Down
9 changes: 5 additions & 4 deletions lib/spack/spack/test/bindist.py
Original file line number Diff line number Diff line change
Expand Up @@ -453,10 +453,11 @@ def test_generate_index_missing(monkeypatch, tmpdir, mutable_config):
# Update index
buildcache_cmd("update-index", "-d", mirror_dir.strpath)

# Check dependency not in buildcache
cache_list = buildcache_cmd("list", "--allarch")
assert "libdwarf" in cache_list
assert "libelf" not in cache_list
with spack.config.override("config:binary_index_ttl", 0):
# Check dependency not in buildcache
cache_list = buildcache_cmd("list", "--allarch")
assert "libdwarf" in cache_list
assert "libelf" not in cache_list


def test_generate_indices_key_error(monkeypatch, capfd):
Expand Down

0 comments on commit a719c9d

Please sign in to comment.