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

fix: Speed up identity overrides #4840

Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
49142b0
Query only one page of identity overrides for features
zachaysan Nov 18, 2024
9e8d767
Pass feature ids to get_edge_identity_overrides
zachaysan Nov 18, 2024
8d864f2
Paginate the view to get the feature ids for the context
zachaysan Nov 18, 2024
5a5e7e4
Add a test that get_overrides_data gets passed the feature ids
zachaysan Nov 18, 2024
80f6588
simplify caching logic
matthewelwell Nov 18, 2024
678ddd4
Update test to pass
zachaysan Nov 18, 2024
79d46d5
Merge branch 'fix/speed_up_identity_overrides' of github.com:Flagsmit…
zachaysan Nov 18, 2024
33814e5
Add warning docstring
zachaysan Nov 19, 2024
6d0c046
Update mock assertion
zachaysan Nov 19, 2024
cdf378d
Limit based on whether feature ids have been passed in
zachaysan Nov 19, 2024
1a8393b
Remove obsolete argument
zachaysan Nov 19, 2024
415cb51
Add more identity overrides boolean
zachaysan Nov 19, 2024
9044edb
Remove obsolete arg and set more identity overrides
zachaysan Nov 19, 2024
a470918
Set more identity overrides boolean
zachaysan Nov 19, 2024
4fc6b50
Add get_identity_overrides_key_condition_expression and remove argument
zachaysan Nov 19, 2024
d9b52b5
Add more identity overrides
zachaysan Nov 19, 2024
9854471
Remove obsolete argument
zachaysan Nov 19, 2024
3e2c959
Add test for searching based on feature ids
zachaysan Nov 19, 2024
81b402e
Merge branch 'main' into fix/speed_up_identity_overrides
zachaysan Dec 3, 2024
26cd0c9
Remove as_completed
zachaysan Dec 3, 2024
a8c3719
Create get_edge_identity_overrides_for_feature_ids
zachaysan Dec 6, 2024
5a9c5e8
Remove more_identities_overrides and create IdentityOverridesV2List
zachaysan Dec 6, 2024
1c15e36
Use IdentityOverridesQueryResponse
zachaysan Dec 6, 2024
0d84428
Switch to is_num_identity_overrides_complete
zachaysan Dec 6, 2024
15a7138
Update caller to new identities overrides response from client
zachaysan Dec 6, 2024
47b2f1c
Update help text and switch to is_num_identity_overrides_complete
zachaysan Dec 6, 2024
001ca64
Update call assertion
zachaysan Dec 6, 2024
82614bb
Switch to is_num_identity_overrides_complete and test page of identit…
zachaysan Dec 6, 2024
6b74a68
Run test with feature_ids
zachaysan Dec 6, 2024
f1a91b1
Switch from NOTE to TODO with issue linked
zachaysan Dec 6, 2024
0c620a8
Page non-deterministic results
zachaysan Dec 9, 2024
b151941
Merge branch 'main' into fix/speed_up_identity_overrides
zachaysan Dec 9, 2024
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
42 changes: 37 additions & 5 deletions api/edge_api/identities/edge_identity_service.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
import typing

from environments.dynamodb import DynamoEnvironmentV2Wrapper
from environments.dynamodb.types import IdentityOverrideV2
from environments.dynamodb.types import (
IdentityOverridesV2List,
IdentityOverrideV2,
)

ddb_environment_v2_wrapper = DynamoEnvironmentV2Wrapper()


def get_edge_identity_overrides(
environment_id: int,
feature_id: int | None = None,
) -> typing.List[IdentityOverrideV2]:
) -> list[IdentityOverrideV2]:
override_items = (
ddb_environment_v2_wrapper.get_identity_overrides_by_environment_id(
environment_id=environment_id, feature_id=feature_id
environment_id=environment_id,
feature_id=feature_id,
)
)
return [
Expand All @@ -21,3 +23,33 @@ def get_edge_identity_overrides(
)
for item in override_items
]


def get_edge_identity_overrides_for_feature_ids(
environment_id: int,
feature_ids: None | list[int] = None,
) -> list[IdentityOverridesV2List]:
query_responses = (
ddb_environment_v2_wrapper.get_identity_overrides_by_environment_id(
environment_id=environment_id,
feature_ids=feature_ids,
)
)

results = []
for identity_overrides_query_response in query_responses:
identity_overrides = [
IdentityOverrideV2.model_validate(
{**item, "environment_id": str(item["environment_id"])}
)
for item in identity_overrides_query_response.items
]
complete = identity_overrides_query_response.is_num_identity_overrides_complete
results.append(
IdentityOverridesV2List(
identity_overrides=identity_overrides,
is_num_identity_overrides_complete=complete,
)
)

return results
6 changes: 6 additions & 0 deletions api/environments/dynamodb/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ class IdentityOverridesV2Changeset:
to_put: list[IdentityOverrideV2]


@dataclass
class IdentityOverridesV2List:
identity_overrides: list[IdentityOverrideV2]
is_num_identity_overrides_complete: bool


@dataclass
class EdgeV2MigrationResult:
identity_overrides_changeset: IdentityOverridesV2Changeset
Expand Down
71 changes: 61 additions & 10 deletions api/environments/dynamodb/wrappers/environment_wrapper.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import typing
from concurrent.futures import ThreadPoolExecutor
from dataclasses import dataclass
from typing import Any, Iterable

from boto3.dynamodb.conditions import Key
Expand Down Expand Up @@ -29,6 +31,12 @@
from environments.models import Environment


@dataclass
class IdentityOverridesQueryResponse:
items: list[dict[str, Any]]
is_num_identity_overrides_complete: bool


class BaseDynamoEnvironmentWrapper(BaseDynamoWrapper):
def write_environment(self, environment: "Environment") -> None:
self.write_environments([environment])
Expand Down Expand Up @@ -66,23 +74,66 @@ def get_identity_overrides_by_environment_id(
self,
environment_id: int,
feature_id: int | None = None,
) -> typing.List[dict[str, Any]]:
feature_ids: None | list[int] = None,
) -> list[dict[str, Any]] | list[IdentityOverridesQueryResponse]:

try:
return list(
self.query_get_all_items(
KeyConditionExpression=Key(ENVIRONMENTS_V2_PARTITION_KEY).eq(
str(environment_id),
)
& Key(ENVIRONMENTS_V2_SORT_KEY).begins_with(
get_environments_v2_identity_override_document_key(
if feature_ids is None:
return list(
self.query_get_all_items(
KeyConditionExpression=self.get_identity_overrides_key_condition_expression(
environment_id=environment_id,
feature_id=feature_id,
),
)
)
)
)

else:
futures = []
with ThreadPoolExecutor() as executor:
for feature_id in feature_ids:
futures.append(
executor.submit(
self.get_identity_overrides_page,
environment_id,
feature_id,
)
)

results = [future.result() for future in futures]
return results

except KeyError as e:
raise ObjectDoesNotExist() from e

def get_identity_overrides_page(
self, environment_id: int, feature_id: int
) -> IdentityOverridesQueryResponse:
query_response = self.table.query(
KeyConditionExpression=self.get_identity_overrides_key_condition_expression(
environment_id=environment_id,
feature_id=feature_id,
)
)
last_evaluated_key = query_response.get("LastEvaluatedKey")
return IdentityOverridesQueryResponse(
items=query_response["Items"],
is_num_identity_overrides_complete=last_evaluated_key is None,
)

def get_identity_overrides_key_condition_expression(
self,
environment_id: int,
feature_id: None | int,
) -> Key:
return Key(ENVIRONMENTS_V2_PARTITION_KEY).eq(
str(environment_id),
) & Key(ENVIRONMENTS_V2_SORT_KEY).begins_with(
get_environments_v2_identity_override_document_key(
feature_id=feature_id,
),
)

def update_identity_overrides(
self,
changeset: IdentityOverridesV2Changeset,
Expand Down
1 change: 1 addition & 0 deletions api/features/dataclasses.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class EnvironmentFeatureOverridesData:

num_segment_overrides: int = 0
num_identity_overrides: typing.Optional[int] = None
is_num_identity_overrides_complete: bool = True

def add_identity_override(self):
"""
Expand Down
34 changes: 23 additions & 11 deletions api/features/features_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from concurrent.futures import ThreadPoolExecutor

from edge_api.identities.edge_identity_service import (
get_edge_identity_overrides,
get_edge_identity_overrides_for_feature_ids,
)
from features.dataclasses import EnvironmentFeatureOverridesData
from features.versioning.versioning_service import get_environment_flags_list
Expand All @@ -16,20 +16,21 @@

def get_overrides_data(
environment: "Environment",
feature_ids: None | list[int] = None,
) -> OverridesData:
"""
Get correct overrides counts for a given environment.

:param project: project to get overrides data for
:return: overrides data getter
:return: overrides data getter dictionary of {feature_id: EnvironmentFeatureOverridesData}
"""
project = environment.project

if project.enable_dynamo_db:
if project.edge_v2_identity_overrides_migrated:
# If v2 migration is complete, count segment overrides from Core
# and identity overrides from DynamoDB.
return get_edge_overrides_data(environment)
return get_edge_overrides_data(environment, feature_ids)
# If v2 migration is not started, in progress, or incomplete,
# only count segment overrides from Core.
# v1 Edge identity overrides are uncountable.
Expand Down Expand Up @@ -71,7 +72,7 @@ def get_core_overrides_data(


def get_edge_overrides_data(
environment: "Environment",
environment: "Environment", feature_ids: None | list[int] = None
) -> OverridesData:
"""
Get the number of identity / segment overrides in a given environment for each feature in the
Expand All @@ -81,14 +82,18 @@ def get_edge_overrides_data(
:param environment: the environment to get the overrides data for
:return OverridesData: dictionary of {feature_id: EnvironmentFeatureOverridesData}
"""

assert feature_ids is not None

with ThreadPoolExecutor() as executor:
get_environment_flags_list_future = executor.submit(
get_environment_flags_list,
environment,
)
get_overrides_data_future = executor.submit(
get_edge_identity_overrides,
get_edge_identity_overrides_for_feature_ids,
environment_id=environment.id,
feature_ids=feature_ids,
)
all_overrides_data: OverridesData = {}

Expand All @@ -98,11 +103,18 @@ def get_edge_overrides_data(
)
if feature_state.feature_segment_id:
env_feature_overrides_data.num_segment_overrides += 1
for identity_override in get_overrides_data_future.result():
# Only override features that exists in core
if identity_override.feature_state.feature.id in all_overrides_data:
all_overrides_data[
identity_override.feature_state.feature.id
].add_identity_override()
for identity_overrides_v2_list in get_overrides_data_future.result():

for identity_override in identity_overrides_v2_list.identity_overrides:
# Only override features that exists in core
if identity_override.feature_state.feature.id in all_overrides_data:
all_overrides_data[
identity_override.feature_state.feature.id
].add_identity_override()
all_overrides_data[
identity_override.feature_state.feature.id
].is_num_identity_overrides_complete = (
identity_overrides_v2_list.is_num_identity_overrides_complete
)

return all_overrides_data
15 changes: 15 additions & 0 deletions api/features/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,12 @@ class CreateFeatureSerializer(DeleteBeforeUpdateWritableNestedModelSerializer):
"in the environment provided by the `environment` query parameter. "
"Note: will return null for Edge enabled projects."
)
is_num_identity_overrides_complete = serializers.SerializerMethodField(
help_text="A boolean that indicates whether there are more"
" identity overrides than are being listed, if `False`. This field is "
"`True` when querying overrides data for a features list page and "
"exact data has been returned."
)

last_modified_in_any_environment = serializers.SerializerMethodField(
help_text="Datetime representing the last time that the feature was modified "
Expand Down Expand Up @@ -181,6 +187,7 @@ class Meta:
"environment_feature_state",
"num_segment_overrides",
"num_identity_overrides",
"is_num_identity_overrides_complete",
"is_server_key_only",
"last_modified_in_any_environment",
"last_modified_in_current_environment",
Expand Down Expand Up @@ -298,6 +305,14 @@ def get_num_identity_overrides(self, instance) -> typing.Optional[int]:
except (KeyError, AttributeError):
return None

def get_is_num_identity_overrides_complete(self, instance) -> typing.Optional[int]:
try:
return self.context["overrides_data"][
instance.id
].is_num_identity_overrides_complete
except (KeyError, AttributeError):
return None

def get_last_modified_in_any_environment(
self, instance: Feature
) -> datetime | None:
Expand Down
9 changes: 5 additions & 4 deletions api/features/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,10 @@ def get_queryset(self):

if environment_id:
page = self.paginate_queryset(queryset)

self.environment = Environment.objects.get(id=environment_id)
self.feature_ids = [feature.id for feature in page]
q = Q(
feature_id__in=[feature.id for feature in page],
feature_id__in=self.feature_ids,
identity__isnull=True,
feature_segment__isnull=True,
)
Expand Down Expand Up @@ -205,7 +205,6 @@ def perform_destroy(self, instance):

def get_serializer_context(self):
context = super().get_serializer_context()

feature_states = getattr(self, "_feature_states", {})
project = get_object_or_404(Project.objects.all(), pk=self.kwargs["project_pk"])
context.update(
Expand All @@ -216,7 +215,9 @@ def get_serializer_context(self):
environment = get_object_or_404(
Environment, id=self.request.query_params["environment"]
)
context["overrides_data"] = get_overrides_data(environment)
context["overrides_data"] = get_overrides_data(
environment, self.feature_ids
)

return context

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,8 @@ def test_get_edge_identity_overrides_for_a_feature(
}

mock_dynamodb_wrapper.get_identity_overrides_by_environment_id.assert_called_once_with(
environment_id=environment.id, feature_id=feature.id
environment_id=environment.id,
feature_id=feature.id,
)


Expand Down
Loading
Loading