Skip to content

Commit a53a544

Browse files
markstoryc298lee
authored andcommitted
feat(hybridcloud) Add control generational caching table (#68465)
Add a control silo version of the cross-region generational caching model. I'd like to start high-frequency RPC calls coming from control to the regions.
1 parent e20807e commit a53a544

File tree

12 files changed

+171
-23
lines changed

12 files changed

+171
-23
lines changed

fixtures/backup/model_dependencies/detailed.json

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,22 @@
8787
"table_name": "hybridcloud_apitokenreplica",
8888
"uniques": []
8989
},
90+
"hybridcloud.controlcacheversion": {
91+
"dangling": false,
92+
"foreign_keys": {},
93+
"model": "hybridcloud.controlcacheversion",
94+
"relocation_dependencies": [],
95+
"relocation_scope": "Excluded",
96+
"silos": [
97+
"Control"
98+
],
99+
"table_name": "hybridcloud_controlcacheversion",
100+
"uniques": [
101+
[
102+
"key"
103+
]
104+
]
105+
},
90106
"hybridcloud.externalactorreplica": {
91107
"dangling": false,
92108
"foreign_keys": {

fixtures/backup/model_dependencies/flat.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
"sentry.organization",
1515
"sentry.user"
1616
],
17+
"hybridcloud.controlcacheversion": [],
1718
"hybridcloud.externalactorreplica": [
1819
"sentry.externalactor",
1920
"sentry.integration",

fixtures/backup/model_dependencies/sorted.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
[
2+
"hybridcloud.controlcacheversion",
23
"hybridcloud.regioncacheversion",
34
"hybridcloud.webhookpayload",
45
"nodestore.node",

fixtures/backup/model_dependencies/truncate.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
[
2+
"hybridcloud_controlcacheversion",
23
"hybridcloud_regioncacheversion",
34
"hybridcloud_webhookpayload",
45
"nodestore_node",

migrations_lockfile.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ To resolve this, rebase against latest master and regenerate your migration. Thi
66
will then be regenerated, and you should be able to merge without conflicts.
77

88
feedback: 0004_index_together
9-
hybridcloud: 0015_apitokenreplica_hashed_token_index
9+
hybridcloud: 0016_add_control_cacheversion
1010
nodestore: 0002_nodestore_no_dictfield
1111
replays: 0004_index_together
1212
sentry: 0693_add_monitors_ownership_actor_id
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# Generated by Django 5.0.3 on 2024-04-08 20:58
2+
3+
from django.db import migrations, models
4+
5+
import sentry.db.models.fields.bounded
6+
from sentry.new_migrations.migrations import CheckedMigration
7+
8+
9+
class Migration(CheckedMigration):
10+
# This flag is used to mark that a migration shouldn't be automatically run in production.
11+
# This should only be used for operations where it's safe to run the migration after your
12+
# code has deployed. So this should not be used for most operations that alter the schema
13+
# of a table.
14+
# Here are some things that make sense to mark as post deployment:
15+
# - Large data migrations. Typically we want these to be run manually so that they can be
16+
# monitored and not block the deploy for a long period of time while they run.
17+
# - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
18+
# run this outside deployments so that we don't block them. Note that while adding an index
19+
# is a schema change, it's completely safe to run the operation after the code has deployed.
20+
# Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment
21+
22+
is_post_deployment = False
23+
24+
dependencies = [
25+
("hybridcloud", "0015_apitokenreplica_hashed_token_index"),
26+
]
27+
28+
operations = [
29+
migrations.CreateModel(
30+
name="ControlCacheVersion",
31+
fields=[
32+
(
33+
"id",
34+
sentry.db.models.fields.bounded.BoundedBigAutoField(
35+
primary_key=True, serialize=False
36+
),
37+
),
38+
("key", models.CharField(max_length=64, unique=True)),
39+
("version", models.PositiveBigIntegerField(default=0)),
40+
],
41+
options={
42+
"db_table": "hybridcloud_controlcacheversion",
43+
},
44+
),
45+
]

src/sentry/hybridcloud/models/cacheversion.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
from django.db.models import F
33

44
from sentry.backup.scopes import RelocationScope
5-
from sentry.db.models import Model, region_silo_only_model
5+
from sentry.db.models import Model, control_silo_only_model, region_silo_only_model
66
from sentry.db.postgres.transactions import enforce_constraints
77

88

@@ -33,3 +33,12 @@ class RegionCacheVersion(CacheVersionBase):
3333
class Meta:
3434
app_label = "hybridcloud"
3535
db_table = "hybridcloud_regioncacheversion"
36+
37+
38+
@control_silo_only_model
39+
class ControlCacheVersion(CacheVersionBase):
40+
__relocation_scope__ = RelocationScope.Excluded
41+
42+
class Meta:
43+
app_label = "hybridcloud"
44+
db_table = "hybridcloud_controlcacheversion"

src/sentry/hybridcloud/rpc/services/caching/impl.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@
33

44
from django.core.cache import cache
55

6-
from sentry.hybridcloud.models.cacheversion import CacheVersionBase, RegionCacheVersion
6+
from sentry.hybridcloud.models.cacheversion import (
7+
CacheVersionBase,
8+
ControlCacheVersion,
9+
RegionCacheVersion,
10+
)
711
from sentry.hybridcloud.rpc.services.caching import ControlCachingService, RegionCachingService
812
from sentry.silo import SiloMode
913

@@ -21,7 +25,7 @@ def _consume_generator(g: Generator[None, None, _V]) -> _V:
2125
return e.value
2226

2327

24-
def _set_cache(key: str, value: str, version: int) -> Generator[None, None, bool]:
28+
def _set_cache(key: str, value: str | None, version: int) -> Generator[None, None, bool]:
2529
result = cache.add(_versioned_key(key, version), value)
2630
yield
2731
return result
@@ -34,6 +38,8 @@ def _versioned_key(key: str, version: int) -> str:
3438
def _version_model(mode: SiloMode) -> type[CacheVersionBase]:
3539
if mode == SiloMode.REGION:
3640
return RegionCacheVersion
41+
if mode == SiloMode.CONTROL:
42+
return ControlCacheVersion
3743
raise ValueError
3844

3945

src/sentry/hybridcloud/rpc/services/caching/service.py

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
# defined, because we want to reflect on type annotations and avoid forward references.
55
import abc
66
from collections.abc import Callable, Generator, Mapping, Sequence
7-
from typing import TYPE_CHECKING, Generic, TypeVar
7+
from typing import TYPE_CHECKING, Generic, TypeVar, Union
88

99
import pydantic
1010

@@ -35,21 +35,24 @@ def clear_key(self, *, region_name: str, key: str) -> int:
3535

3636

3737
_R = TypeVar("_R", bound=pydantic.BaseModel)
38+
_OptionalR = Union[_R, None]
3839

3940

4041
class SiloCacheBackedCallable(Generic[_R]):
4142
silo_mode: SiloMode
4243
base_key: str
43-
cb: Callable[[int], _R]
44+
cb: Callable[[int], _R | None]
4445
type_: type[_R]
4546

46-
def __init__(self, base_key: str, silo_mode: SiloMode, cb: Callable[[int], _R], t: type[_R]):
47+
def __init__(
48+
self, base_key: str, silo_mode: SiloMode, cb: Callable[[int], _OptionalR], t: type[_R]
49+
):
4750
self.base_key = base_key
4851
self.silo_mode = silo_mode
4952
self.cb = cb
5053
self.type_ = t
5154

52-
def __call__(self, args: int) -> _R:
55+
def __call__(self, args: int) -> _OptionalR:
5356
if (
5457
SiloMode.get_current_mode() != self.silo_mode
5558
and SiloMode.get_current_mode() != SiloMode.MONOLITH
@@ -60,7 +63,9 @@ def __call__(self, args: int) -> _R:
6063
def key_from(self, args: int) -> str:
6164
return f"{self.base_key}:{args}"
6265

63-
def resolve_from(self, i: int, values: Mapping[str, int | str]) -> Generator[None, None, _R]:
66+
def resolve_from(
67+
self, i: int, values: Mapping[str, int | str]
68+
) -> Generator[None, None, _OptionalR]:
6469
from .impl import _consume_generator, _delete_cache, _set_cache
6570

6671
key = self.key_from(i)
@@ -75,10 +80,11 @@ def resolve_from(self, i: int, values: Mapping[str, int | str]) -> Generator[Non
7580
version = value
7681

7782
r = self.cb(i)
78-
_consume_generator(_set_cache(key, r.json(), version))
83+
if r is not None:
84+
_consume_generator(_set_cache(key, r.json(), version))
7985
return r
8086

81-
def get_many(self, ids: Sequence[int]) -> list[_R]:
87+
def get_many(self, ids: Sequence[int]) -> list[_OptionalR]:
8288
from .impl import _consume_generator, _get_cache
8389

8490
keys = [self.key_from(i) for i in ids]
@@ -88,8 +94,8 @@ def get_many(self, ids: Sequence[int]) -> list[_R]:
8894

8995
def back_with_silo_cache(
9096
base_key: str, silo_mode: SiloMode, t: type[_R]
91-
) -> Callable[[Callable[[int], _R]], "SiloCacheBackedCallable[_R]"]:
92-
def wrapper(cb: Callable[[int], _R]) -> "SiloCacheBackedCallable[_R]":
97+
) -> Callable[[Callable[[int], _OptionalR]], "SiloCacheBackedCallable[_R]"]:
98+
def wrapper(cb: Callable[[int], _OptionalR]) -> "SiloCacheBackedCallable[_R]":
9399
return SiloCacheBackedCallable(base_key, silo_mode, cb, t)
94100

95101
return wrapper

src/sentry/services/hybrid_cloud/user/service.py

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,7 @@ def flush_nonce(self, *, user_id: int) -> None:
121121
pass
122122

123123
def get_user(self, user_id: int) -> RpcUser | None:
124-
user = get_user(user_id)
125-
if user.is_anonymous:
126-
return None
127-
return user
124+
return get_user(user_id)
128125

129126
@rpc_method
130127
@abstractmethod
@@ -195,12 +192,11 @@ def get_user_avatar(self, *, user_id: int) -> RpcAvatar | None:
195192

196193

197194
@back_with_silo_cache("user_service.get_user", SiloMode.REGION, RpcUser)
198-
def get_user(user_id: int) -> RpcUser:
195+
def get_user(user_id: int) -> RpcUser | None:
199196
users = user_service.get_many(filter=dict(user_ids=[user_id]))
200197
if len(users) > 0:
201198
return users[0]
202-
else:
203-
return RpcUser(is_anonymous=True)
199+
return None
204200

205201

206202
user_service = UserService.create_delegation()

tests/sentry/backup/snapshots/test_comparators/test_default_comparators.pysnap

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
---
2-
created: '2024-04-10T14:45:05.473571+00:00'
2+
created: '2024-04-10T15:02:31.213310+00:00'
33
creator: sentry
44
source: tests/sentry/backup/test_comparators.py
55
---
@@ -24,6 +24,10 @@ source: tests/sentry/backup/test_comparators.py
2424
- organization
2525
- user_id
2626
model_name: hybridcloud.apitokenreplica
27+
- comparators:
28+
- class: ForeignKeyComparator
29+
fields: []
30+
model_name: hybridcloud.controlcacheversion
2731
- comparators:
2832
- class: ForeignKeyComparator
2933
fields:

tests/sentry/hybridcloud/test_caching.py

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,20 @@
22

33
from django.core.cache import cache
44

5-
from sentry.hybridcloud.rpc.services.caching import back_with_silo_cache, region_caching_service
5+
from sentry.hybridcloud.rpc.services.caching import (
6+
back_with_silo_cache,
7+
control_caching_service,
8+
region_caching_service,
9+
)
610
from sentry.hybridcloud.rpc.services.caching.impl import CacheBackend
11+
from sentry.services.hybrid_cloud.organization.model import RpcOrganizationSummary
12+
from sentry.services.hybrid_cloud.organization.service import organization_service
713
from sentry.services.hybrid_cloud.user import RpcUser
814
from sentry.services.hybrid_cloud.user.service import user_service
915
from sentry.silo import SiloMode
1016
from sentry.testutils.factories import Factories
1117
from sentry.testutils.pytest.fixtures import django_db_all
12-
from sentry.testutils.silo import assume_test_silo_mode, no_silo_test
18+
from sentry.testutils.silo import assume_test_silo_mode, control_silo_test, no_silo_test
1319
from sentry.types.region import get_local_region
1420

1521

@@ -26,6 +32,7 @@ def get_user(user_id: int) -> RpcUser:
2632

2733
for u in users:
2834
next_user = get_user(u.id)
35+
assert next_user
2936
assert next_user == get_user.cb(u.id)
3037
old.append(next_user)
3138

@@ -43,9 +50,65 @@ def get_user(user_id: int) -> RpcUser:
4350
)
4451

4552
for u, cached in zip(users, get_user.get_many([u.id for u in users])):
53+
assert cached
4654
assert cached.username == u.username
4755

4856

57+
@control_silo_test
58+
@django_db_all(transaction=True)
59+
def test_caching_function_control():
60+
cache.clear()
61+
62+
@back_with_silo_cache(
63+
base_key="my-test-key", silo_mode=SiloMode.CONTROL, t=RpcOrganizationSummary
64+
)
65+
def get_org(org_id: int) -> RpcOrganizationSummary | None:
66+
return organization_service.get_org_by_id(id=org_id)
67+
68+
user = Factories.create_user()
69+
orgs = [Factories.create_organization(owner=user) for _ in range(2)]
70+
old: list[RpcOrganizationSummary] = []
71+
72+
for org in orgs:
73+
next_org = get_org(org.id)
74+
assert next_org
75+
assert next_org == get_org.cb(org.id)
76+
old.append(next_org)
77+
78+
for org in orgs:
79+
with assume_test_silo_mode(SiloMode.REGION):
80+
org.update(name=org.name + " updated")
81+
82+
# Does not include updates
83+
for org in old:
84+
next_org = get_org(org.id)
85+
assert next_org == org
86+
87+
control_caching_service.clear_key(key=get_org.key_from(org.id))
88+
89+
for o, cached in zip(orgs, get_org.get_many([o.id for o in orgs])):
90+
assert cached
91+
assert cached.name == o.name
92+
93+
94+
@control_silo_test
95+
@django_db_all(transaction=True)
96+
def test_caching_function_none_value():
97+
cache.clear()
98+
99+
@back_with_silo_cache(
100+
base_key="my-test-key", silo_mode=SiloMode.CONTROL, t=RpcOrganizationSummary
101+
)
102+
def get_org(org_id: int) -> RpcOrganizationSummary | None:
103+
return organization_service.get_org_by_id(id=org_id)
104+
105+
result = get_org(9999)
106+
assert result is None
107+
108+
result = get_org(9999)
109+
assert result is None
110+
111+
49112
@django_db_all(transaction=True)
50113
@no_silo_test
51114
def test_cache_versioning():

0 commit comments

Comments
 (0)