Skip to content

Refactor ActionCache inheritance structure to be similar to CAS

Adam Coldrick requested to merge sotk/tech-debt/refactor-action-caches into master

Description

Currently, we have 4 different types of ActionCache instance, 3 of which are configured using a tag in the services config section. This is different to the way CAS handles its storage, or the Execution service handles the scheduler, which is confusing. It also complicates type validation due to the various instances not having a shared superclass. Its also not very clear how to create a new type of ActionCache in the current state of things, since there is no explicit API for an ActionCache instance defined anywhere.

This MR refactors the ActionCache instances to use the same pattern as the rest of BuildGrid. Now the !action-cache YAML tag simply takes a cache, which refers to an object labelled with one of the other Action Cache tags (eg. !s3action-cache or the new !lru-action-cache). The old in-memory Action Cache is replaced by a generic instance implementation that simply passes the calls through to the actual implementation being used. The in-memory functionality is now handled by an LruActionCache class, which can be passed to the new generic instance.

The changes are all backwards compatible for now, so existing configs won't actually break. The cost of supporting this is that the base class for the cache backends has extra code to handle being used directly in the services config section.

Summary of changes

  • Add an ABC for the ActionCache cache implementations
  • Move the specialized instances into a caches subdirectory
  • Update the instances to inherit from the ABC
  • Add a generic ActionCache instance that takes a cache backend to pass commands through to
  • Update parser.py and the config schemas to handle the new ActionCache instance

Validation

Run a BuildGrid with an ActionCache, and run the same build twice with a client that has caching enabled.

The docker-compose setup is such a BuildGrid, you can run it with

docker-compose up -d

This patch enables caching in the test client

diff --git a/buildgrid/_app/commands/cmd_execute.py b/buildgrid/_app/commands/cmd_execute.py
index a06ad3d..dd37066 100644
--- a/buildgrid/_app/commands/cmd_execute.py
+++ b/buildgrid/_app/commands/cmd_execute.py
@@ -173,7 +173,7 @@ def run_command(context, input_root, commands, output_file, output_directory,
 
     request = remote_execution_pb2.ExecuteRequest(instance_name=context.instance_name,
                                                   action_digest=action_digest,
-                                                  skip_cache_lookup=True)
+                                                  skip_cache_lookup=False)
 
     response = stub.Execute(request)
 
@@ -254,7 +254,7 @@ def upload_action(commands, context, input_root, output_executables, output_file
 
         action = remote_execution_pb2.Action(command_digest=command_digest,
                                              input_root_digest=input_root_digest,
-                                             do_not_cache=True)
+                                             do_not_cache=False)
 
         action_digest = uploader.put_message(action, queue=True)

After applying that,

tox -e venv -- bgd execute --remote-cas https://localhost:50052 command buildgrid ls
tox -e venv -- bgd execute --remote-cas https://localhost:50052 command buildgrid ls

The second run should be a cache hit, which can be verified in the server logs.

Issues addressed

Closes #290 (closed)

Merge request reports