move the BinaryCacheIndex location into the user home dir by cosmicexplorer · Pull Request #22500 · spack/spack

cosmicexplorer · 2021-03-24T00:49:06Z

Problem

BinaryCacheIndex will pull down index.json files from mirrors, then create a Database from it in a temporary directory to list remote specs. We would like this location to be shared among Spack instances which may share mirrors. Additionally, we would like to ensure that the default location for these Database instances is definitely a user-writable directory.

This is step 1 of addressing #19085, for which we plan to eventually create a separate Database with its own TTL for "all specs Spack has ever seen", including the results of spack spec and spack solve.

Solution

For our fetch cache, misc cache, and binary index cache, separate public *_cache_location() methods (which return the cache's filesystem path) from private _*_cache() methods (which return the python object representing the cache).
- This allows creating caches in subdirectories of other cache directories.
If config:binary_index_root is unset, use a subdirectory of config:misc_cache (which defaults to ~/.spack/cache).
- This ensures that the location is definitely writable by the current user.

Result

After running spack buildcache list, we now find a new directory ~/.spack/cache/indices has been created, with the following contents:

# mcclanahan7@turingtarpit: ~/tools/spack 18:14:30
; ls ~/.spack/cache/indices
total 48M
-rwxr-xr-x 1 mcclanahan7 34309   0 Mar 23 18:13 .714ed82990_c1df2b37cf.json.lock*
-rwxr-xr-x 1 mcclanahan7 34309   0 Mar 23 18:13 .contents.json.lock*
-rw-r--r-- 1 mcclanahan7 34309 48M Mar 23 18:13 714ed82990_c1df2b37cf.json
-rw-r--r-- 1 mcclanahan7 34309 152 Mar 23 18:13 contents.json
# mcclanahan7@turingtarpit: ~/tools/spack 18:14:51
; jq <~/.spack/cache/indices/714ed82990_c1df2b37cf.json | head -n10
{
  "database": {
    "installs": {
      "fkoyyzocpyqsx26aswd63cg3leadscoy": {
        "spec": {
          "libpciaccess": {
            "version": "0.16",
            "arch": {
              "platform": "linux",
              "platform_os": "rhel8",

tgamblin

LGTM modulo one indices

lib/spack/spack/binary_distribution.py

vvolkl · 2021-03-24T10:29:12Z

I think this will then supersede #20137, right?

cosmicexplorer · 2021-03-24T17:14:31Z

Oh yes!! This actually makes use of a config:binary_index_root as well in case we want to keep that configurable. I don't think we have a good method for deprecating options yet, so I'm thinking we would keep that in there.

spack/lib/spack/spack/binary_distribution.py

Line 449 in dc5b244

    
           cache_root = spack.config.get('config:binary_index_root', binary_index_location())

tgamblin · 2021-03-24T19:31:03Z

@vvolkl: do you need this to be configurable? I'm trying to figure out why we'd need to move it/share it. If we can get away wtih sticking it in the misc_cache we can avoid having another config option.

tgamblin · 2021-03-24T19:32:25Z

I guess I should also ask @mslacken whether putting this in misc_cache is sufficient or whether it also needs to be configurable. Sorry I missed #20137.

vvolkl · 2021-03-24T19:39:42Z

misc_cache is completely fine I think, it should just be configurable/not in the spack root dir as we need to distribute spack on a read-only filesystem. Same for #20158

tgamblin · 2021-03-24T20:01:06Z

ok given that, @cosmicexplorer can you just remove the separate config option for this, and assume it lives under misc_cache?

cosmicexplorer · 2021-03-24T20:01:07Z

@tgamblin:

we can avoid having another config option

Just to be clear, config:binary_index_root was added in #19209. Are we sure all possible users of this option since last Halloween will be satisfied with a misc_cache subdirectory?

vvolkl · 2021-03-25T10:44:09Z

@cosmicexplorer True that it is usually a good idea to be very careful with deprecations. But in this case, this option was not mentioned in the default config or documented, so I would say if @mslacken agrees I'd say it's fine to remove

mslacken · 2021-03-25T13:23:14Z

I am fine with it. I just added the option as before that it could point to a non writable directory for the user.

scottwittenburg

I didn't pull this and try it, but in general, I have no objection to the cache living under misc cache in the absence of configuring a specific config:binary_index_root.

And because we used write transaction locking to write the binary index cache files, it should be fine that this is now in a location that could be shared among several spack instances. Thanks for explicitly pointing that out in the description, I would not have picked up on that detail otherwise 👍.

cosmicexplorer · 2021-03-25T17:51:03Z

@mslacken -- thanks so much for putting that PR in! I am glad this would address the issue :D. @vvolkl thank you for the deprecation context -- that seems quite reasonable.

@scottwittenburg -- thanks so much for the feedback and the point about write transaction locking!! I hadn't considered that and since I'm planning to make some other changes it was helpful to understand what makes this safe.

I'm waiting on @tgamblin (who is very very busy today) to confirm that some others he said he'd contact aren't specifically relying on config:binary_index_root being in a particular location, then I think he said this would be good to merge.

tgamblin · 2021-03-25T19:08:01Z

@cosmicexplorer: we're good -- go ahead with it

Remote buildcache indices need to be stored in a place that does not require writing to the Spack prefix. Move them from the install_tree to the misc_cache.

cosmicexplorer · 2021-03-29T23:18:30Z

@tgamblin looks like this is green -- can we merge it?

cosmicexplorer requested review from opadron, scottwittenburg and tgamblin March 24, 2021 00:49

tgamblin requested changes Mar 24, 2021

View reviewed changes

lib/spack/spack/binary_distribution.py Outdated Show resolved Hide resolved

lib/spack/spack/binary_distribution.py Show resolved Hide resolved

cosmicexplorer mentioned this pull request Mar 24, 2021

check remote mirrors for hashes provided by the user #22503

Closed

scottwittenburg previously approved these changes Mar 25, 2021

View reviewed changes

move binary indices are stored into the misc_cache

f03cebd

Remote buildcache indices need to be stored in a place that does not require writing to the Spack prefix. Move them from the install_tree to the misc_cache.

tgamblin dismissed scottwittenburg’s stale review via f03cebd March 28, 2021 05:42

tgamblin force-pushed the binary-indices-to-home-dir branch from dc5b244 to f03cebd Compare March 28, 2021 05:42

tgamblin approved these changes Mar 30, 2021

View reviewed changes

tgamblin merged commit f67d477 into spack:develop Mar 30, 2021

This was referenced Mar 30, 2021

added 'binary_index_root' entry in config #20137

Closed

[WIP -- do not review] reimplement the BinaryCacheIndex to wrap a list of Databases #22634

Closed

elsagermann mentioned this pull request Oct 21, 2021

Update spack v0.xx C2SM/spack-c2sm#164

Closed

alalazo mentioned this pull request Apr 5, 2023

binary_index_root not in standard config #20136

Closed

Conversation

cosmicexplorer commented Mar 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Result

Uh oh!

tgamblin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vvolkl commented Mar 24, 2021

Uh oh!

cosmicexplorer commented Mar 24, 2021

Uh oh!

tgamblin commented Mar 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tgamblin commented Mar 24, 2021

Uh oh!

vvolkl commented Mar 24, 2021

Uh oh!

tgamblin commented Mar 24, 2021

Uh oh!

cosmicexplorer commented Mar 24, 2021

Uh oh!

vvolkl commented Mar 25, 2021

Uh oh!

mslacken commented Mar 25, 2021

Uh oh!

scottwittenburg left a comment

Choose a reason for hiding this comment

Uh oh!

cosmicexplorer commented Mar 25, 2021

Uh oh!

tgamblin commented Mar 25, 2021

Uh oh!

cosmicexplorer commented Mar 29, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cosmicexplorer commented Mar 24, 2021 •

edited

Loading

tgamblin commented Mar 24, 2021 •

edited

Loading