Skip to content

Commit

Permalink
PLUTON-19231 | Fewer database calls in VcLTagBuilder.prepare_redirects
Browse files Browse the repository at this point in the history
Previously, related domains and domain mappings for a given cluster were fetched in a loop, performing many small selects
In this change, two types of of data fetching procedures (acquiring all domain mappings, and static domain mappings for each cluster)
have been moved to VclRendererInput, and the smaller chunks needed for rendering steps are obtained locally from simple data structures.

In local testing, this resulted in roughly halving the time of the prepare_redirects rendering phase (from around 2 to around 0.9 seconds)
  • Loading branch information
mgolski committed Oct 17, 2024
1 parent 6090dab commit e3aec29
Showing 1 changed file with 54 additions and 13 deletions.
67 changes: 54 additions & 13 deletions vaas/vaas/vcl/renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import time
import functools
from typing import Dict, List
from urllib.parse import urlsplit

from django.conf import settings
from django.db.models import Prefetch
Expand All @@ -15,6 +16,7 @@
from vaas.router.models import Route, Redirect
from vaas.cluster.models import VclTemplateBlock, Dc, VclVariable, LogicalCluster, DomainMapping
from vaas.cluster.mapping import MappingProvider
import logging

VCL_TAGS = {
'4.0': [
Expand Down Expand Up @@ -186,23 +188,50 @@ def __init__(self, varnish, input_data):
'mesh_routing': varnish.cluster.service_mesh_routing
}

def filter_cluster_domain_mappings(
self, cluster: LogicalCluster
) -> dict[str, list[str]]:
"""Returns a dictionary where keys are domains, and values are mapped to by the key"""
# we can PROBABLY assume that each domain has only one mapping
# in an experiment trying to add a new mapping for a source domain removed the old one
destination_dict = {}
for domain_mapping in self.input.domain_mappings:
domain = domain_mapping.domain
destination_dict[domain] = domain_mapping.mapped_domains(cluster)
return destination_dict

def provide_related_domains(self, cluster: LogicalCluster) -> list[str]:
"""Returns a list of all domains related to a cluster - for static mappings,
the list is specified within the mapping, for dynamic - clusters are associated using
labels (mapping targets can contain labels, and each cluster can have multiple labels, but
each label belongs to exactly one cluster)"""
result = {m.domain for m in self.input.static_domain_mappings_by_cluster[cluster.name]}
for m in self.input.mapping_provider.mappings["dynamic"]:
if m.is_cluster_related_by_labels(cluster):
result.add(m.domain)
return sorted(list(result))

@collect_processing
def prepare_redirects(self) -> Dict[str, List[VclRedirect]]:
redirects = {}
related_domains = MappingProvider(DomainMapping.objects.all()).provide_related_domains(self.varnish.cluster) # tutaj juz mamy dwa zapytania do bazy
related_domains = self.provide_related_domains(self.varnish.cluster)
destinations_dict = self.filter_cluster_domain_mappings(self.varnish.cluster)
for redirect in self.input.redirects:
destination_domain, destination_mappings = redirect.fetch_all_destinations_mappings(self.varnish.cluster) # tutaj znów sięgamy do bazy po redirecty i liste mappingow
if str(redirect.src_domain) in related_domains:
for mapped_domain in redirect.src_domain.mapped_domains(self.varnish.cluster):
destination = str(redirect.destination)
if destination_domain == redirect.src_domain.domain:
destination = destination.replace(destination_domain, mapped_domain)
elif all((destination_domain, len(destination_mappings) == 1)):
destination = destination.replace(destination_domain, destination_mappings[0])
if entries := redirects.get(mapped_domain, []):
entries.append(VclRedirect(redirect, mapped_domain, destination))
else:
redirects[mapped_domain] = [VclRedirect(redirect, mapped_domain, destination)]
if str(redirect.src_domain) not in related_domains:
continue
destination_domain = urlsplit(redirect.destination).netloc
destination_mappings = destinations_dict.get(destination_domain)
for mapped_domain in redirect.src_domain.mapped_domains(self.varnish.cluster):
destination = str(redirect.destination)
if destination_domain == redirect.src_domain.domain:
destination = destination.replace(destination_domain, mapped_domain)
elif destination_domain and len(destination_mappings) == 1:
destination = destination.replace(destination_domain, destination_mappings[0])

if entries := redirects.get(mapped_domain, []):
entries.append(VclRedirect(redirect, mapped_domain, destination))
else:
redirects[mapped_domain] = [VclRedirect(redirect, mapped_domain, destination)]
return redirects

@collect_processing
Expand Down Expand Up @@ -392,6 +421,18 @@ def fetch_render_data(self):
)
self.distributed_backends = self.distribute_backends(backends)
self.distributed_canary_backends = self.prepare_canary_backends(canary_backend_ids, backends)
self.domain_mappings = DomainMapping.objects.all()
self.mapping_provider = MappingProvider(self.domain_mappings)
self.static_domain_mappings_by_cluster = self.cluster_static_domain_mappings()

def cluster_static_domain_mappings(self) -> dict[str, list[DomainMapping]]:
"""Returns a dict cluster.name: [associated domain mappings]"""
# NTH: get the same output with only one database query, perhaps with prefetch_related()
result_dict = dict()
all_clusters = LogicalCluster.objects.all()
for cluster in all_clusters:
result_dict[cluster.name] = cluster.domainmapping_set.filter(type="static")
return result_dict

@collect_processing
def distribute_backends(self, backends):
Expand Down

0 comments on commit e3aec29

Please sign in to comment.