You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "bneradt (via GitHub)" <gi...@apache.org> on 2025/10/03 20:01:32 UTC

[PR] remap_acl speedup via config reload [trafficserver]

bneradt opened a new pull request, #12540:
URL: https://github.com/apache/trafficserver/pull/12540

   This changes the remap_acl.test.py test to use config reloads instead of spinning up new ATS insteads for every TestRun. The 1/3 speedup is relatively modest, but straightforward enough to implement that it seems worth it to save about 40 seconds on the test run.
   
   Before:
   
   ```
   ./autest.sh --sandbox /tmp/sb --clean=none -f remap_acl  169.64s user 59.69s system 52% cpu 7:17.19 total
   ```
   
   After:
   ```
   ./autest.sh --sandbox /tmp/sb --clean=none -f remap_acl  129.60s user 8.27s system 109% cpu 2:06.28 total
   ```


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] remap_acl speedup via config reload [trafficserver]

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #12540:
URL: https://github.com/apache/trafficserver/pull/12540#issuecomment-3369535229

   /cc @mattyw 


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] remap_acl speedup via config reload [trafficserver]

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt merged PR #12540:
URL: https://github.com/apache/trafficserver/pull/12540


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] remap_acl speedup via config reload [trafficserver]

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #12540:
URL: https://github.com/apache/trafficserver/pull/12540#issuecomment-3367462901

   
   > Adding @brbzull0 for the config update logic. I ran into bugs when I updated the `proxy.config.config_update_interval_ms` - the wait_for_cache and some other configs didn't get set appropriately. This fixes the update logic so that both are set now. I kept this as a separate commit.
   
   This is getting more complicated because some tests were broken due to the longer reload interval. I will hold off on this remap_reload update until I fix the other issues in a separate PR:
   #12541
   
   In the meantime, I'll make this as a draft and rebase once that lands.


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] remap_acl speedup via config reload [trafficserver]

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #12540:
URL: https://github.com/apache/trafficserver/pull/12540#discussion_r2408646102


##########
tests/gold_tests/remap/remap_acl.test.py:
##########
@@ -104,9 +124,64 @@ def _configure_traffic_server(self, tr: 'TestRun', proxy_protocol: bool) -> None
         for name, _ in self._named_acls:
             remap_config_lines.append(f'.activatefilter {name}')
 
-        remap_config_lines.append(f'map / http://127.0.0.1:{self._server.Variables.http_port} {self._acl_configuration}')
-        ts.Disk.remap_config.AddLines(remap_config_lines)
-        ts.Disk.ip_allow_yaml.AddLines(self._ip_allow_content.split("\n"))
+        remap_config_lines.append(f'map / http://127.0.0.1:{server_port} {self._acl_configuration}')
+
+        if call_reload:
+            #
+            # Update the ATS configuration.
+            #
+            tr = Test.AddTestRun("Change the ATS configuration")
+            p = tr.Processes.Default
+            p.Command = (
+                f'traffic_ctl config set proxy.config.http.connect_ports {server_port} && '
+                f'traffic_ctl config set proxy.config.url_remap.acl_behavior_policy {self._acl_behavior_policy}')
+
+            p.Env = ts.Env
+            tr.StillRunningAfter = ts
+
+            remap_cfg_path = os.path.join(ts.Variables.CONFIGDIR, 'remap.config')
+            ip_allow_path = os.path.join(ts.Variables.CONFIGDIR, 'ip_allow.yaml')
+            p.Setup.Lambda(
+                lambda: update_config_file(
+                    remap_cfg_path, '\n'.join(remap_config_lines), ip_allow_path, '\n'.join(self._ip_allow_lines)))
+

Review Comment:
   This is the heart of this patch:
   * For the first test run, start ATS via MakeATSProcess (see the `else:` below this).
   * For subsequent test runs, update the config and do a config reload instead of starting a new test process. Doing a config reload is faster than starting a brand new ATS process for every test run.



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] remap_acl speedup via config reload [trafficserver]

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #12540:
URL: https://github.com/apache/trafficserver/pull/12540#issuecomment-3367375481

   Adding @brbzull0 for the config update logic. I ran into bugs when I updated the `proxy.config.config_update_interval_ms` - the wait_for_cache and some other configs didn't get set appropriately. This fixes the update logic so that both are set now. I kept this as a separate commit.


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org