cmcfarlen commented on code in PR #13666:
URL: https://github.com/apache/trafficserver/pull/13666#discussion_r4041443292
##########
tests/gold_tests/origin_connection/per_server_connection_max.test.py:
##########
@@ -580,9 +593,125 @@ def run(self) -> None:
self._test_metrics()
+class AggregateRetractionTest:
+ """Verify that raising metric_aggregate to 2 withdraws already published
per group metrics.
+
+ metric_aggregate 2 (AGGREGATE_MAX) publishes the per hostname max and
nothing else, so this
+ also covers that the hostname sums are not published at that level.
+
+ metric_aggregate is dynamic, but the publication decision is made in the
ConnectionTracker
+ Group constructor, and a published metric name is never removed from the
metric store. Before
+ the store grew a tombstone, a name published while the setting was 0 kept
reporting for the
+ life of the process no matter what the setting was changed to, which is
exactly what was seen
+ in production: per group and per hostname metrics side by side at
metric_aggregate 2.
+
+ Origin keep alive is disabled so each request opens and closes its own
upstream connection.
+ That returns the group count to zero, which erases the group, so the next
request constructs a
+ fresh one and re-evaluates the setting. A group that never goes idle would
keep whatever was in
+ effect when it was created.
+ """
+
+ def __init__(self) -> None:
+ """Configure the processes for the test."""
+ self._dns = _dns
+ self._server = Test.MakeHttpBinServer("retract_server")
+ self._configure_trafficserver()
+
+ def _configure_trafficserver(self) -> None:
+ """Configure an ATS that starts out publishing the per group
metrics."""
+ self._ts = Test.MakeATSProcess("retract_ts")
+ self._ts.Disk.records_config.update(
+ {
+ **_STAT_SYNC_RECORDS,
+ 'proxy.config.dns.nameservers':
f"127.0.0.1:{self._dns.Variables.Port}",
+ 'proxy.config.dns.resolv_conf': 'NULL',
+ 'proxy.config.http.per_server.connection.metric_enabled': 1,
+ # Start with the per group metrics published, then raise it at
runtime below.
+ 'proxy.config.http.per_server.connection.metric_aggregate': 0,
+ 'proxy.config.http.per_server.connection.match': 'both',
+ # Force the upstream connection closed after each transaction
so the group is
+ # erased and the next request rebuilds it.
+ 'proxy.config.http.keep_alive_enabled_out': 0,
+ })
+ self._ts.Disk.remap_config.AddLine(
+ f"map http://retract.origin.com/
http://retract.origin.com:{self._server.Variables.Port}/";)
+
+ def _curl(self, tr) -> None:
+ """Drive one request through the remap rule."""
+ tr.MakeCurlCommand(f"-v --fail -s -x
127.0.0.1:{self._ts.Variables.port} 'http://retract.origin.com/get'",
ts=self._ts)
+ tr.Processes.Default.ReturnCode = 0
+ tr.StillRunningAfter = self._ts
Review Comment:
`StillRunningAfter` is an assertion, not a lifetime directive, so omitting
the origin cannot cause it to be stopped. From `autest/core/testrun.py:60`:
```python
self._Register(
"Test.Process.StillRunningAfter",
TesterSet(testers.Equal, True, self.FinishedEvent,
converter=lambda val: LamdaEq(val._isRunningAfter)),
"StillRunningAfter")
```
It registers a tester, evaluated on `FinishedEvent`, that the process *is*
still running. Nothing in the framework tears down processes at run boundaries
based on it.
Three confirmations that the second curl is not at risk:
- 45 gold tests already use this pattern -- origin started once with
`StartBefore`, never named in `StillRunningAfter`, traffic driven through it in
later runs. `headers/good_request_after_bad.test.py` does it across 20 runs,
`headers/normalize_ae.test.py` 16, `pluginTest/regex_remap/regex_remap.test.py`
10.
- The test passing requires the origin to be alive: the second curl runs
with `--fail -s` and `ReturnCode = 0`, and the following run asserts the group
metric is present, which needs a real outbound connection. A stopped origin
fails both.
- All four AuTest shards pass on this file in the current run.
No change here.
--
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]
