Improving caching of S3 endpoints by omitting unused parameters

Author: ashovlin

.changes/next-release/enhancement-s3-29532.json

@@ -0,0 +1,5 @@
+{
+  "type": "enhancement",
+  "category": "``s3``",
+  "description": "Improve caching of S3 endpoints, which should improve performance of sync and recursive copy commands that interact with multiple keys in the same bucket"
+}

awscli/botocore/endpoint_provider.py

@@ -47,6 +47,10 @@
     r"^(?!-)[a-zA-Z\d-]{1,63}(?<!-)$",
 )
 CACHE_SIZE = 100
+# S3 endpoint ruleset parameters that are defined but not currently referenced.
+# They are excluded from the cache key to avoid cache thrashing when
+# accessing multiple objects in the same bucket.
+S3_UNREFERENCED_PARAMS = {'Key', 'Prefix', 'CopySource'}
 ARN_PARSER = ArnParser()
 STRING_FORMATTER = Formatter()
 
@@ -700,16 +704,22 @@ def evaluate(self, input_parameters):
 class EndpointProvider:
     """Derives endpoints from a RuleSet for given input parameters."""
 
-    def __init__(self, ruleset_data, partition_data):
+    def __init__(self, ruleset_data, partition_data, excluded_params=None):
         self.ruleset = RuleSet(**ruleset_data, partitions=partition_data)
+        self._excluded_params = excluded_params or frozenset()
 
-    @lru_cache_weakref(maxsize=CACHE_SIZE)
     def resolve_endpoint(self, **input_parameters):
         """Match input parameters to a rule.
 
         :type input_parameters: dict
         :rtype: RuleSetEndpoint
         """
+        for param in self._excluded_params:
+            input_parameters.pop(param, None)
+        return self._resolve_endpoint(**input_parameters)
+
+    @lru_cache_weakref(maxsize=CACHE_SIZE)
+    def _resolve_endpoint(self, **input_parameters):
         params_for_error = input_parameters.copy()
         endpoint = self.ruleset.evaluate(input_parameters)
         if endpoint is None:

awscli/botocore/regions.py

@@ -25,7 +25,7 @@
 import jmespath
 from botocore import UNSIGNED, xform_name
 from botocore.auth import AUTH_TYPE_MAPS
-from botocore.endpoint_provider import EndpointProvider
+from botocore.endpoint_provider import S3_UNREFERENCED_PARAMS, EndpointProvider
 from botocore.exceptions import (
     EndpointProviderError,
     EndpointVariantError,
@@ -470,6 +470,11 @@ def __init__(
         self._provider = EndpointProvider(
             ruleset_data=endpoint_ruleset_data,
             partition_data=partition_data,
+            excluded_params=(
+                S3_UNREFERENCED_PARAMS
+                if service_model.service_name == 's3'
+                else None
+            ),
         )
         self._param_definitions = self._provider.ruleset.parameters
         self._service_model = service_model
@@ -515,9 +520,9 @@ def construct_endpoint(
 
         # The endpoint provider does not support non-secure transport.
         if (
-                not self._use_ssl
-                and provider_result.url.startswith('https://')
-                and 'Endpoint' not in provider_params
+            not self._use_ssl
+            and provider_result.url.startswith('https://')
+            and 'Endpoint' not in provider_params
         ):
             provider_result = provider_result._replace(
                 url=f'http://{provider_result.url[8:]}'

tests/functional/botocore/test_endpoint_rulesets.py

@@ -18,7 +18,10 @@
 import pytest
 from botocore import xform_name
 from botocore.config import Config
-from botocore.endpoint_provider import EndpointProvider
+from botocore.endpoint_provider import (
+    S3_UNREFERENCED_PARAMS,
+    EndpointProvider,
+)
 from botocore.exceptions import (
     BotoCoreError,
     ClientError,
@@ -295,3 +298,34 @@ def builtin_overwriter_handler(builtins, **kwargs):
             except (ClientError, ResponseParserError):
                 pass
         assert len(http_stubber.requests) == 0
+
+
+def _collect_refs_from_rules(rules):
+    """Recursively collect all {\"ref\": \"...\"} values from rules."""
+    refs = set()
+    if isinstance(rules, dict):
+        if 'ref' in rules:
+            refs.add(rules['ref'])
+        for value in rules.values():
+            refs.update(_collect_refs_from_rules(value))
+    elif isinstance(rules, list):
+        for item in rules:
+            refs.update(_collect_refs_from_rules(item))
+    return refs
+
+
+@pytest.mark.validates_model
+def test_s3_unreferenced_endpoint_ruleset_params():
+    """Assert that the set of known S3 endpoint ruleset parameters that are
+    not used in the rules hasn't changed. These are set in the
+    S3_UNREFERENCED_PARAMS constant to prevent them from influencing the cache.
+    """
+    ruleset = LOADER.load_service_model('s3', 'endpoint-rule-set-1')
+    all_params = set(ruleset['parameters'].keys())
+    referenced = _collect_refs_from_rules(ruleset['rules'])
+    unreferenced = all_params - referenced
+    assert unreferenced == S3_UNREFERENCED_PARAMS, (
+        "The set of unused S3 endpoints parameters have changed, examine the "
+        "implication of this on endpoint caching behavior before updating"
+        "S3_UNREFERENCED_PARAMS."
+    )