JakeChampion commented on code in PR #13971:
URL: https://github.com/apache/trafficserver/pull/13971#discussion_r4457808685


##########
tests/uranium_tests/pluginTest/compress/test_compress_transform_cache_write.py:
##########
@@ -0,0 +1,94 @@
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+from concurrent.futures import ThreadPoolExecutor
+from pathlib import Path
+import sys
+
+import pytest
+
+from tools.uranium.services import ATSFactory, Curl, ServiceFactory
+
+PATHS = 10
+CONCURRENCY = 6
+CLIENT_MAX_TIME = 10
+
+
+def test_compress_transform_cache_write(ats_factory: ATSFactory, services: 
ServiceFactory, curl: Curl) -> None:
+    """Concurrent transforms of a cached identity object must all complete.
+
+    Each request that accepts br hits the stored uncompressed object, runs it
+    through compress.so, and opens a cache write for the compressed copy. With
+    several of these requests at the same time the open write retries on the
+    write lock. If the cache-to-transform tunnel completes during the retry,
+    HTTP_TUNNEL_EVENT_DONE reaches HttpSM::state_cache_open_write. That
+    handler must not cancel the pending open write, or the transaction never
+    sends a response.
+
+    :param ats_factory: Factory for isolated proxy instances.
+    :param services: Factory owning the origin.
+    :param curl: Transport-aware client.
+    """
+    directory = Path(__file__).parent
+    port = services.allocate_port()
+    origin = services.process(
+        "origin",
+        (sys.executable, directory / "compress_large_origin.py", "--port", 
str(port)),
+        ready_port=port,
+    )
+    ats = ats_factory.create("ts", enable_cache=True)
+    if not ats.plugin_exists("compress.so"):
+        pytest.skip("compress.so is required")
+    ats.records.update(
+        {
+            "proxy.config.diags.debug.enabled": 1,
+            "proxy.config.diags.debug.tags": "compress",
+            # Make the open write for the transformed copy wait long enough
+            # for the transform tunnel to complete first.
+            "proxy.config.http.cache.max_open_read_retries": 5,

Review Comment:
   Fixed in 070c3ee41: the test sets 
`proxy.config.http.cache.ignore_accept_encoding_mismatch: 1`, so the `br` 
requests use the stored identity object and the test uses the 
cache-to-transform path that it describes. (From the code, I expect that the 
stall can also occur on a miss, because the origin-to-transform tunnel can 
complete during the write retry too. I have not tested that case.)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to