Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to cache package payloads synchronously #1679

Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/rez/cli/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ def setup_parser(parser, completions=False):
parser.add_argument(
"--no-pkg-cache", action="store_true",
help="Disable package caching")
parser.add_argument(
"--pkg-cache-sync", action="store_true",
help="Disable asynchronous package caching. "
"Process will block until packages are cached.")
isohedronpipeline marked this conversation as resolved.
Show resolved Hide resolved
parser.add_argument(
"--pre-command", type=str, help=SUPPRESS)
PKG_action = parser.add_argument(
Expand Down Expand Up @@ -212,7 +216,8 @@ def command(opts, parser, extra_arg_groups=None):
caching=(not opts.no_cache),
suppress_passive=opts.no_passive,
print_stats=opts.stats,
package_caching=(not opts.no_pkg_cache)
package_caching=(not opts.no_pkg_cache),
package_cache_async=(not opts.pkg_cache_sync),
)

success = (context.status == ResolverStatus.solved)
Expand Down
1 change: 1 addition & 0 deletions src/rez/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ def _parse_env_var(self, value):
"package_cache_during_build": Bool,
"package_cache_local": Bool,
"package_cache_same_device": Bool,
"package_cache_async": Bool,
"color_enabled": ForceOrBool,
"resolve_caching": Bool,
"cache_package_files": Bool,
Expand Down
12 changes: 7 additions & 5 deletions src/rez/package_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -366,21 +366,21 @@ def remove_variant(self, variant):

return self.VARIANT_REMOVED

def add_variants_async(self, variants):
def add_variants(self, variants, package_cache_async=True):
isohedronpipeline marked this conversation as resolved.
Show resolved Hide resolved
"""Update the package cache by adding some or all of the given variants.

This method is called when a context is created or sourced. Variants
are then added to the cache in a separate process.
"""

# A prod install is necessary because add_variants_async works by
# A prod install is necessary because add_variants works by
# starting a rez-pkg-cache proc, and this can only be done reliably in
# a prod install. On non-windows we could fork instead, but there would
# remain no good solution on windows.
#
if not system.is_production_rez_install:
raise PackageCacheError(
"PackageCache.add_variants_async is only supported in a "
"PackageCache.add_variants is only supported in a "
"production rez installation."
)

Expand Down Expand Up @@ -460,12 +460,14 @@ def add_variants_async(self, variants):
else:
out_target = devnull

subprocess.Popen(
[exe, "--daemon", self.path],
isohedronpipeline marked this conversation as resolved.
Show resolved Hide resolved
process = subprocess.Popen(
args,
stdout=out_target,
stderr=out_target,
**kwargs
)
if not package_cache_async:
process.wait()
isohedronpipeline marked this conversation as resolved.
Show resolved Hide resolved

except Exception as e:
print_warning(
Expand Down
15 changes: 12 additions & 3 deletions src/rez/resolved_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ def __init__(self, package_requests, verbosity=0, timestamp=None,
package_filter=None, package_orderers=None, max_fails=-1,
add_implicit_packages=True, time_limit=-1, callback=None,
package_load_callback=None, buf=None, suppress_passive=False,
print_stats=False, package_caching=None):
print_stats=False, package_caching=None, package_cache_async=None):
"""Perform a package resolve, and store the result.

Args:
Expand Down Expand Up @@ -205,6 +205,8 @@ def __init__(self, package_requests, verbosity=0, timestamp=None,
package_caching (bool|None): If True, apply package caching settings
as per the config. If None, enable as determined by config
setting :data:`package_cache_during_build`.
package_cache_async (Optional[bool]): If True, cache packages asynchronously.
isohedronpipeline marked this conversation as resolved.
Show resolved Hide resolved
If None, use the config setting :data:`package_cache_async`
"""
self.load_path = None

Expand Down Expand Up @@ -249,6 +251,10 @@ def __init__(self, package_requests, verbosity=0, timestamp=None,

self.package_caching = package_caching

if package_cache_async is None:
package_cache_async = config.package_cache_async
self.package_cache_async = package_cache_async
JeanChristopheMorinPerso marked this conversation as resolved.
Show resolved Hide resolved

# patch settings
self.default_patch_lock = PatchLock.no_lock
self.patch_locks = {}
Expand Down Expand Up @@ -1838,13 +1844,16 @@ def _update_package_cache(self):
not self.success:
return

# see PackageCache.add_variants_async
# see PackageCache.add_variants
if not system.is_production_rez_install:
return

pkgcache = self._get_package_cache()
if pkgcache:
pkgcache.add_variants_async(self.resolved_packages)
pkgcache.add_variants(
self.resolved_packages,
self.package_cache_async,
)

@classmethod
def _init_context_tracking_payload_base(cls):
Expand Down
8 changes: 6 additions & 2 deletions src/rez/rezconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,10 @@
# Enable package caching during a package build.
package_cache_during_build = False

# Enable package caching to run asynchronously during a resolve.
# If this is false, a resolve will block until all packages are cached.
isohedronpipeline marked this conversation as resolved.
Show resolved Hide resolved
package_cache_async = True

# Allow caching of local packages. You would only want to set this True for
# testing purposes.
package_cache_local = False
Expand Down Expand Up @@ -313,7 +317,7 @@
# This is useful as Platform.os might show different
# values depending on the availability of ``lsb-release`` on the system.
# The map supports regular expression, e.g. to keep versions.
#
#
# .. note::
# The following examples are not necessarily recommendations.
#
Expand Down Expand Up @@ -1119,7 +1123,7 @@

# Enables/disables colorization globally.
#
# .. warning::
# .. warning::
# Turned off for Windows currently as there seems to be a problem with the colorama module.
#
# May also set to the string ``force``, which will make rez output color styling
Expand Down
56 changes: 52 additions & 4 deletions src/rez/tests/test_package_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,24 +134,72 @@ def test_caching_on_resolve(self):

# Creating the context will asynchronously add variants to the cache
# in a separate proc.
#
# NOTE: pyfoo will not cache, because its repo is set to non-caching (see above)
c = ResolvedContext([
"timestamped-1.2.0",
"pyfoo-3.1.0" # won't cache, see earlier test
"pyfoo-3.1.0"
])
JeanChristopheMorinPerso marked this conversation as resolved.
Show resolved Hide resolved

variant = c.get_resolved_package("timestamped")

# Retry 50 times with 0.1 sec interval, 5 secs is more than enough for
# the very small variant to be copied to cache.
#
cached_root = None
resolve_not_always_cached = False
for _ in range(50):
time.sleep(0.1)
cached_root = pkgcache.get_cached_root(variant)
if cached_root:
break

resolve_not_always_cached = True
time.sleep(0.1)

self.assertNotEqual(cached_root, None)
isohedronpipeline marked this conversation as resolved.
Show resolved Hide resolved

# Test that the package is not immediately cached, since it is asynchronous
# WARNING: This is dangerous since it does open the test to a race condition and
# will fail if the cache happens faster than the resolve.
self.assertNotEqual(resolve_not_always_cached, False)

expected_payload_file = os.path.join(cached_root, "stuff.txt")
self.assertTrue(os.path.exists(expected_payload_file))

# check that refs to root point to cache location in rex code
for ref in ("resolve.timestamped.root", "'{resolve.timestamped.root}'"):
proc = c.execute_rex_code(
code="info(%s)" % ref,
stdout=subprocess.PIPE,
universal_newlines=True
)

out, _ = proc.communicate()
root = out.strip()

self.assertEqual(
root, cached_root,
"Reference %r should resolve to %s, but resolves to %s"
% (ref, cached_root, root)
)

@install_dependent()
def test_caching_on_resolve_synchronous(self):
"""Test that cache is updated as expected on
resolved env using syncrhonous package caching."""
pkgcache = self._pkgcache()

with restore_os_environ():
# set config settings into env so rez-pkg-cache proc sees them
os.environ.update(self.get_settings_env())

# Creating the context will synchronously add variants to the cache
c = ResolvedContext(
["timestamped-1.2.0", "pyfoo-3.1.0"],
package_cache_async=False,
)

variant = c.get_resolved_package("timestamped")
# The first time we try to access it will be cached, because the cache is blocking
cached_root = pkgcache.get_cached_root(variant)
self.assertNotEqual(cached_root, None)

expected_payload_file = os.path.join(cached_root, "stuff.txt")
Expand Down