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 10 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
15 changes: 14 additions & 1 deletion src/rez/cli/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ def setup_parser(parser, completions=False):
parser.add_argument(
"--no-pkg-cache", action="store_true",
help="Disable package caching")
parser.add_argument(
"--pkg-cache-mode", choices=["sync", "async"],
help="If provided, override the rezconfig's package_cache_async key. "
"If 'sync', the process will block until packages are cached. "
"If 'async', the process will not block while packages are cached.")
parser.add_argument(
"--pre-command", type=str, help=SUPPRESS)
PKG_action = parser.add_argument(
Expand Down Expand Up @@ -198,6 +203,13 @@ def command(opts, parser, extra_arg_groups=None):
rule = Rule.parse_rule(rule_str)
package_filter.add_inclusion(rule)

if opts.pkg_cache_mode == "async":
package_cache_mode = True
elif opts.pkg_cache_mode == "sync":
package_cache_mode = False
else:
package_cache_mode = None

# perform the resolve
context = ResolvedContext(
package_requests=request,
Expand All @@ -212,7 +224,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=package_cache_mode,
)

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
21 changes: 17 additions & 4 deletions src/rez/package_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,18 +369,29 @@ def remove_variant(self, variant):
def add_variants_async(self, variants):
"""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.

.. deprecated:: 3.1.0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO for maintainers: Update with appropriate version number before merging.

Use :method:`add_variants` instead.
"""
return self.add_variants(variants, package_cache_async=True)

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 +471,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.communicate()

except Exception as e:
print_warning(
Expand Down
16 changes: 12 additions & 4 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 (bool|None): If True, cache packages asynchronously.
If None, use the config setting :data:`package_cache_async`
"""
self.load_path = None

Expand Down Expand Up @@ -246,9 +248,12 @@ def __init__(self, package_requests, verbosity=0, timestamp=None,
package_caching = config.package_cache_during_build
else:
package_caching = True

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 +1843,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
10 changes: 8 additions & 2 deletions src/rez/rezconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,12 @@
# Enable package caching during a package build.
package_cache_during_build = False

# Asynchronously cache packages. If this is false, resolves will block until
# all packages are cached.
#
# .. versionadded:: 3.1.0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO for maintainers: Update with appropriate version number before merging.

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 +319,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 +1125,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
60 changes: 56 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,76 @@ 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

# Prove that the resolved context used async mode.
self.assertTrue(c.package_cache_async)

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,
msg="Packages were expected to be cached, but were not.")

# 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