Skip to content

Commit

Permalink
adding caching on look_up repository
Browse files Browse the repository at this point in the history
  • Loading branch information
Sunandadadi committed Jan 15, 2024
1 parent 2410c7a commit fa3b385
Show file tree
Hide file tree
Showing 6 changed files with 138 additions and 11 deletions.
16 changes: 16 additions & 0 deletions data/cache/cache_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,19 @@ def for_security_report(digest, cache_config):
# Security reports don't change often so a longer TTL can be justified.
cache_ttl = cache_config.get("security_report_cache_ttl", "300s")
return CacheKey(f"security_report_{digest}", cache_ttl)


def for_repository_lookup(namespace_name, repo_name, manifest_ref, kind_filter, cache_config):
"""
Returns a cache key for repository lookup.
"""

cache_ttl = cache_config.get("repository_lookup_cache_ttl", "120s")
cache_key = f"repository_lookup_{namespace_name}_{repo_name}"

if manifest_ref is not None:
cache_key = f"{cache_key}_{manifest_ref}"
if kind_filter is not None:
cache_key = f"{cache_key}_{kind_filter}"

return CacheKey(cache_key, cache_ttl)
8 changes: 7 additions & 1 deletion data/registry_model/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,13 @@ def get_most_recent_tag(self, repository_ref):

@abstractmethod
def lookup_repository(
self, namespace_name, repo_name, kind_filter=None, raise_on_error=False, manifest_ref=None
self,
namespace_name,
repo_name,
kind_filter=None,
raise_on_error=False,
manifest_ref=None,
model_cache=None,
):
"""
Looks up and returns a reference to the repository with the given namespace and name, or
Expand Down
63 changes: 60 additions & 3 deletions data/registry_model/registry_oci_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,15 @@

from data import database, model
from data.cache import cache_key
from data.database import db_disallow_replica_use, db_transaction
from data.database import (
Repository,
RepositoryKind,
RepositoryState,
User,
Visibility,
db_disallow_replica_use,
db_transaction,
)
from data.model import DataModelException, QuotaExceededException, namespacequota, oci
from data.model.oci.retriever import RepositoryContentRetriever
from data.readreplica import ReadOnlyModeException
Expand Down Expand Up @@ -732,19 +740,68 @@ def find_repository_with_garbage(self, limit_to_gc_policy_s):

return RepositoryReference.for_repo_obj(repo)

@staticmethod
def get_repository_response_as_json(val):
return {
"id": val.id,
"visibility": {
"id": val.visibility.id,
"name": val.visibility.name,
},
"kind": {
"id": val.kind.id,
"name": val.kind.name,
},
"state": val.state,
"namespace_user": {
"stripe_id": val.namespace_user.stripe_id,
},
}

@staticmethod
def get_repository_response_to_object(val):
return Repository(
id=val["id"],
state=RepositoryState(val["state"]),
kind=RepositoryKind(id=val["kind"]["id"], name=val["kind"]["name"]),
visibility=Visibility(id=val["visibility"]["id"], name=val["visibility"]["name"]),
namespace_user=User(stripe_id=val["namespace_user"]["stripe_id"]),
)

def lookup_repository(
self, namespace_name, repo_name, kind_filter=None, raise_on_error=False, manifest_ref=None
self,
namespace_name,
repo_name,
kind_filter=None,
raise_on_error=False,
manifest_ref=None,
model_cache=None,
):
"""
Looks up and returns a reference to the repository with the given namespace and name, or
None if none.
"""
repo = model.repository.get_repository(namespace_name, repo_name, kind_filter=kind_filter)

def get_repository_loader():
result = model.repository.get_repository(
namespace_name, repo_name, kind_filter=kind_filter
)
return OCIModel.get_repository_response_as_json(result) if result else None

if model_cache is not None:
repository_lookup_key = cache_key.for_repository_lookup(
namespace_name, repo_name, manifest_ref, kind_filter, model_cache.cache_config
)
repo = model_cache.retrieve(repository_lookup_key, get_repository_loader)
else:
repo = get_repository_loader()

if repo is None:
if raise_on_error:
raise model.RepositoryDoesNotExist()
return None

repo = OCIModel.get_repository_response_to_object(repo)
state = repo.state
return RepositoryReference.for_repo_obj(
repo,
Expand Down
8 changes: 7 additions & 1 deletion data/registry_model/registry_proxy_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,13 @@ def __init__(self, namespace_name, repo_name, user):
self._proxy = Proxy(self._config, repo_name)

def lookup_repository(
self, namespace_name, repo_name, kind_filter=None, raise_on_error=False, manifest_ref=None
self,
namespace_name,
repo_name,
kind_filter=None,
raise_on_error=False,
manifest_ref=None,
model_cache=None,
):
"""
Looks up and returns a reference to the repository with the given namespace and name, or
Expand Down
28 changes: 28 additions & 0 deletions data/registry_model/test/test_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

from app import docker_v2_signing_key, storage
from data import model
from data.cache import cache_key
from data.cache.impl import InMemoryDataModelCache
from data.cache.test.test_cache import TEST_CACHE_CONFIG
from data.database import (
Expand Down Expand Up @@ -114,6 +115,33 @@ def test_lookup_repository(repo_namespace, repo_name, expected, registry_model):
assert repo_ref is None


@pytest.mark.parametrize(
"repo_namespace, repo_name, expected",
[
("devtable", "simple", True),
("buynlarge", "orgrepo", True),
("buynlarge", "unknownrepo", False),
],
)
def test_lookup_repository_with_cache(repo_namespace, repo_name, expected, registry_model):
model_cache = InMemoryDataModelCache(TEST_CACHE_CONFIG)
model_cache.empty_for_testing()

cache_key_for_repository_lookup = cache_key.for_repository_lookup(
repo_namespace, repo_name, None, None, {}
)
repo_ref = registry_model.lookup_repository(repo_namespace, repo_name, model_cache=model_cache)
cached_result = model_cache.cache.get(cache_key_for_repository_lookup.key)

if expected:
res_obj = OCIModel.get_repository_response_to_object(json.loads(cached_result))
assert repo_ref._db_id is res_obj.id
assert repo_ref.state == res_obj.state
assert repo_ref.namespace_name == repo_namespace
else:
assert repo_ref is None


@pytest.mark.parametrize(
"repo_namespace, repo_name",
[
Expand Down
26 changes: 20 additions & 6 deletions endpoints/v2/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from flask import Response, request, url_for

import features
from app import app, storage
from app import app, model_cache, storage
from auth.registry_jwt_auth import process_registry_jwt_auth
from data.database import db_disallow_replica_use
from data.model import (
Expand Down Expand Up @@ -68,7 +68,11 @@
def fetch_manifest_by_tagname(namespace_name, repo_name, manifest_ref, registry_model):
try:
repository_ref = registry_model.lookup_repository(
namespace_name, repo_name, raise_on_error=True, manifest_ref=manifest_ref
namespace_name,
repo_name,
raise_on_error=True,
manifest_ref=manifest_ref,
model_cache=model_cache,
)
except RepositoryDoesNotExist as e:
image_pulls.labels("v2", "tag", 404).inc()
Expand Down Expand Up @@ -137,7 +141,11 @@ def fetch_manifest_by_tagname(namespace_name, repo_name, manifest_ref, registry_
def fetch_manifest_by_digest(namespace_name, repo_name, manifest_ref, registry_model):
try:
repository_ref = registry_model.lookup_repository(
namespace_name, repo_name, raise_on_error=True, manifest_ref=manifest_ref
namespace_name,
repo_name,
raise_on_error=True,
manifest_ref=manifest_ref,
model_cache=model_cache,
)
except RepositoryDoesNotExist as e:
image_pulls.labels("v2", "manifest", 404).inc()
Expand Down Expand Up @@ -293,7 +301,9 @@ def write_manifest_by_digest(namespace_name, repo_name, manifest_ref):
# manifest does not contain the tag and this call was not given a tag name.
# Instead, we write the manifest with a temporary tag, as it is being pushed
# as part of a call for a manifest list.
repository_ref = registry_model.lookup_repository(namespace_name, repo_name)
repository_ref = registry_model.lookup_repository(
namespace_name, repo_name, model_cache=model_cache
)
if repository_ref is None:
image_pushes.labels("v2", 404, "").inc()
raise NameUnknown("repository not found")
Expand Down Expand Up @@ -362,7 +372,9 @@ def delete_manifest_by_digest(namespace_name, repo_name, manifest_ref):
forbidden by the spec.
"""
with db_disallow_replica_use():
repository_ref = registry_model.lookup_repository(namespace_name, repo_name)
repository_ref = registry_model.lookup_repository(
namespace_name, repo_name, model_cache=model_cache
)
if repository_ref is None:
raise NameUnknown("repository not found")

Expand Down Expand Up @@ -413,7 +425,9 @@ def _write_manifest(
namespace_name, repo_name, tag_name, manifest_impl, registry_model=registry_model
):
# Ensure that the repository exists.
repository_ref = registry_model.lookup_repository(namespace_name, repo_name)
repository_ref = registry_model.lookup_repository(
namespace_name, repo_name, model_cache=model_cache
)
if repository_ref is None:
raise NameUnknown("repository not found")

Expand Down

0 comments on commit fa3b385

Please sign in to comment.