bryancall commented on code in PR #14002:
URL: https://github.com/apache/trafficserver/pull/14002#discussion_r4488778563
##########
tests/uranium_tests/pluginTest/stats_over_http/test_stats_over_http_compression.py:
##########
@@ -38,7 +38,11 @@ def test_stats_over_http_compression(ats_factory:
ATSFactory, services: ServiceF
{
"proxy.config.diags.debug.enabled": 1,
"proxy.config.diags.debug.tags": "test_metrics|stats_over_http",
- "proxy.config.dump_mem_info_frequency": 1
+ "proxy.config.dump_mem_info_frequency": 1,
+ # Linux lets a loopback socket buffer a few megabytes, more than
the whole prometheus body, so without a
+ # small send buffer the intercept can finish its write before the
client's reset arrives and never log the
+ # EOS or ERROR that the test looks for. On macOS the buffer stays
well under the body size either way.
+ "proxy.config.net.sock_send_buffer_size_in": 16384,
Review Comment:
I kept the record in each test. Uranium runs pytest with
`--import-mode=importlib`, so a test can't import `fetch_stats.py` from its own
directory. Sharing it would take either a module in `uranium_tests/lib` or a
`conftest.py` fixture here, which would be the first conftest under
`uranium_tests`, for one record used by two tests. I moved the requirement into
the docstring of `reset_after_first_byte` instead, next to the code that
depends on it.
##########
tests/uranium_tests/pluginTest/stats_over_http/test_stats_over_http_remap_intercept.py:
##########
@@ -64,6 +64,10 @@ def test_stats_over_http_remap_intercept(ats_factory:
ATSFactory, services: Serv
"proxy.config.diags.debug.enabled": 1,
"proxy.config.diags.debug.tags": "stats_over_http",
"proxy.config.dump_mem_info_frequency": 1,
+ # Linux lets a loopback socket buffer a few megabytes, more than
the whole prometheus body, so without a
+ # small send buffer the intercept can finish its write before the
client's reset arrives and never log the
+ # EOS or ERROR that the test looks for. On macOS the buffer stays
well under the body size either way.
Review Comment:
Done in
[c0d8732ddf](https://github.com/apache/trafficserver/pull/14002/commits/c0d8732ddf752094dde44ace00305979907d6285).
The comment now says the record sets `SO_SNDBUF`, which fixes the size and
turns the autotuning off, and that the kernel doubles it to 32 KB against the
1.6 MB body, or about 134 KB when the test asks for gzip. I dropped the macOS
sentence.
##########
tests/uranium_tests/pluginTest/stats_over_http/fetch_stats.py:
##########
@@ -125,7 +125,8 @@ def fetch_records(args: argparse.Namespace) -> int:
def reset_after_first_byte(args: argparse.Namespace) -> int:
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
- # A small receive window keeps most of the body in Traffic Server when the
client goes away.
+ # A small receive window, with the small
proxy.config.net.sock_send_buffer_size_in that the --reset callers
+ # set, keeps most of the body in the intercept when the client goes away.
Review Comment:
Done in
[c0d8732ddf](https://github.com/apache/trafficserver/pull/14002/commits/c0d8732ddf752094dde44ace00305979907d6285).
The `--reset` help line was already at the column limit, so the requirement
went into the docstring of `reset_after_first_byte`, and this comment is back
to covering only the receive window.
--
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]
