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 2026/07/15 22:59:04 UTC
[PR] Add HTTP alternate quality AuTest [trafficserver]
bneradt opened a new pull request, #13391:
URL: https://github.com/apache/trafficserver/pull/13391
The TSHttpAltInfoQualitySet regression gives every alternate the same
quality and does not inspect the selected response. It therefore cannot
detect whether the API affects cache selection.
This adds an AuTest plugin that scores alternates by matching request
query strings. The test verifies alternate creation and selection
through observable cached responses.
Closes: #7205
--
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] Add HTTP alternate quality AuTest [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #13391:
URL: https://github.com/apache/trafficserver/pull/13391#discussion_r3595974175
##########
tests/tools/plugins/http_alt_info_quality.cc:
##########
@@ -0,0 +1,97 @@
+/** @file
+
+ Select cache alternates by matching request query strings.
+
+ @section license License
+
+ 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.
+ */
+
+#include <ts/ts.h>
+
+#include <string>
+
+namespace
+{
+constexpr char PLUGIN_NAME[] = "http_alt_info_quality";
+
+std::string
+get_query(TSMBuffer buffer, TSMLoc header)
+{
+ TSMLoc url;
+ int length = 0;
+
+ if (TSHttpHdrUrlGet(buffer, header, &url) != TS_SUCCESS) {
+ return {};
+ }
+
+ const char *query = TSUrlHttpQueryGet(buffer, url, &length);
+ std::string result{query == nullptr ? "" : query, static_cast<size_t>(length)};
+
+ TSHandleMLocRelease(buffer, header, url);
+ return result;
+}
+
+int
+select_alternate(TSCont /* contp ATS_UNUSED */, TSEvent event, void *edata)
+{
+ TSAssert(event == TS_EVENT_HTTP_SELECT_ALT);
+
+ auto alt_info = static_cast<TSHttpAltInfo>(edata);
+ TSMBuffer client_buffer;
+ TSMBuffer cached_buffer;
+ TSMLoc client_header;
+ TSMLoc cached_header;
+
+ if (TSHttpAltInfoClientReqGet(alt_info, &client_buffer, &client_header) != TS_SUCCESS) {
+ TSHttpAltInfoQualitySet(alt_info, 0.0F);
+ return 0;
+ }
+ if (TSHttpAltInfoCachedReqGet(alt_info, &cached_buffer, &cached_header) != TS_SUCCESS) {
+ TSHttpAltInfoQualitySet(alt_info, 0.0F);
+ TSHandleMLocRelease(client_buffer, TS_NULL_MLOC, client_header);
+ return 0;
+ }
+
+ std::string client_query = get_query(client_buffer, client_header);
+ std::string cached_query = get_query(cached_buffer, cached_header);
+
+ TSHttpAltInfoQualitySet(alt_info, client_query == cached_query ? 1.0F : 0.0F);
+
+ TSHandleMLocRelease(client_buffer, TS_NULL_MLOC, client_header);
+ TSHandleMLocRelease(cached_buffer, TS_NULL_MLOC, cached_header);
+ return 0;
+}
+} // namespace
+
+void
+TSPluginInit(int /* argc ATS_UNUSED */, const char ** /* argv ATS_UNUSED */)
+{
+ TSPluginRegistrationInfo info;
+
+ info.plugin_name = const_cast<char *>(PLUGIN_NAME);
+ info.vendor_name = const_cast<char *>("Apache Software Foundation");
+ info.support_email = const_cast<char *>("dev@trafficserver.apache.org");
Review Comment:
Agreed. These fields are const char *, so I removed the unnecessary casts.
--
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] Add HTTP alternate quality AuTest [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt merged PR #13391:
URL: https://github.com/apache/trafficserver/pull/13391
--
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] Add HTTP alternate quality AuTest [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #13391:
URL: https://github.com/apache/trafficserver/pull/13391#discussion_r3595972501
##########
tests/tools/plugins/http_alt_info_quality.cc:
##########
@@ -0,0 +1,97 @@
+/** @file
+
+ Select cache alternates by matching request query strings.
+
+ @section license License
+
+ 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.
+ */
+
+#include <ts/ts.h>
+
+#include <string>
+
+namespace
+{
+constexpr char PLUGIN_NAME[] = "http_alt_info_quality";
+
+std::string
+get_query(TSMBuffer buffer, TSMLoc header)
+{
+ TSMLoc url;
+ int length = 0;
+
+ if (TSHttpHdrUrlGet(buffer, header, &url) != TS_SUCCESS) {
+ return {};
+ }
+
+ const char *query = TSUrlHttpQueryGet(buffer, url, &length);
+ std::string result{query == nullptr ? "" : query, static_cast<size_t>(length)};
Review Comment:
Good catch. The current API reports zero length for an absent query, but guarding the null pointer explicitly is safer and clearer. Updated.
--
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] Add HTTP alternate quality AuTest [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #13391:
URL: https://github.com/apache/trafficserver/pull/13391#issuecomment-5144755840
Cherry-picked to the 10.2.x branch as a51cc7bea76a9992138666eef8e63c0bdafa7054 for the 10.2.0 release.
--
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] Add HTTP alternate quality AuTest [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13391:
URL: https://github.com/apache/trafficserver/pull/13391#discussion_r3591427513
##########
tests/tools/plugins/http_alt_info_quality.cc:
##########
@@ -0,0 +1,97 @@
+/** @file
+
+ Select cache alternates by matching request query strings.
+
+ @section license License
+
+ 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.
+ */
+
+#include <ts/ts.h>
+
+#include <string>
+
+namespace
+{
+constexpr char PLUGIN_NAME[] = "http_alt_info_quality";
+
+std::string
+get_query(TSMBuffer buffer, TSMLoc header)
+{
+ TSMLoc url;
+ int length = 0;
+
+ if (TSHttpHdrUrlGet(buffer, header, &url) != TS_SUCCESS) {
+ return {};
+ }
+
+ const char *query = TSUrlHttpQueryGet(buffer, url, &length);
+ std::string result{query == nullptr ? "" : query, static_cast<size_t>(length)};
Review Comment:
TSUrlHttpQueryGet() can return nullptr; constructing std::string with a fallback pointer ("") but a non-zero length would read past the string literal. Guard the nullptr case by forcing length=0 (or returning an empty string) before building the std::string.
##########
tests/tools/plugins/http_alt_info_quality.cc:
##########
@@ -0,0 +1,97 @@
+/** @file
+
+ Select cache alternates by matching request query strings.
+
+ @section license License
+
+ 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.
+ */
+
+#include <ts/ts.h>
+
+#include <string>
+
+namespace
+{
+constexpr char PLUGIN_NAME[] = "http_alt_info_quality";
+
+std::string
+get_query(TSMBuffer buffer, TSMLoc header)
+{
+ TSMLoc url;
+ int length = 0;
+
+ if (TSHttpHdrUrlGet(buffer, header, &url) != TS_SUCCESS) {
+ return {};
+ }
+
+ const char *query = TSUrlHttpQueryGet(buffer, url, &length);
+ std::string result{query == nullptr ? "" : query, static_cast<size_t>(length)};
+
+ TSHandleMLocRelease(buffer, header, url);
+ return result;
+}
+
+int
+select_alternate(TSCont /* contp ATS_UNUSED */, TSEvent event, void *edata)
+{
+ TSAssert(event == TS_EVENT_HTTP_SELECT_ALT);
+
+ auto alt_info = static_cast<TSHttpAltInfo>(edata);
+ TSMBuffer client_buffer;
+ TSMBuffer cached_buffer;
+ TSMLoc client_header;
+ TSMLoc cached_header;
+
+ if (TSHttpAltInfoClientReqGet(alt_info, &client_buffer, &client_header) != TS_SUCCESS) {
+ TSHttpAltInfoQualitySet(alt_info, 0.0F);
+ return 0;
+ }
+ if (TSHttpAltInfoCachedReqGet(alt_info, &cached_buffer, &cached_header) != TS_SUCCESS) {
+ TSHttpAltInfoQualitySet(alt_info, 0.0F);
+ TSHandleMLocRelease(client_buffer, TS_NULL_MLOC, client_header);
+ return 0;
+ }
+
+ std::string client_query = get_query(client_buffer, client_header);
+ std::string cached_query = get_query(cached_buffer, cached_header);
+
+ TSHttpAltInfoQualitySet(alt_info, client_query == cached_query ? 1.0F : 0.0F);
+
+ TSHandleMLocRelease(client_buffer, TS_NULL_MLOC, client_header);
+ TSHandleMLocRelease(cached_buffer, TS_NULL_MLOC, cached_header);
+ return 0;
+}
+} // namespace
+
+void
+TSPluginInit(int /* argc ATS_UNUSED */, const char ** /* argv ATS_UNUSED */)
+{
+ TSPluginRegistrationInfo info;
+
+ info.plugin_name = const_cast<char *>(PLUGIN_NAME);
+ info.vendor_name = const_cast<char *>("Apache Software Foundation");
+ info.support_email = const_cast<char *>("dev@trafficserver.apache.org");
Review Comment:
TSPluginRegistrationInfo fields are const char*, so the const_casts are unnecessary and discard constness without benefit. Assign string literals / PLUGIN_NAME directly.
--
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] Add HTTP alternate quality AuTest [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13391:
URL: https://github.com/apache/trafficserver/pull/13391#discussion_r3591364394
##########
tests/gold_tests/cache/replay/alternate-caching-quality.yaml:
##########
@@ -0,0 +1,142 @@
+# 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.
+
+meta:
+ version: "1.0"
+
+autest:
+ description: 'Verify TSHttpAltInfoQualitySet affects alternate selection'
+
+ dns:
+ name: 'dns_alt_quality'
+
+ server:
+ name: 'server_alt_quality'
+
+ client:
+ name: 'client_alt_quality'
+
+ ats:
+ name: 'ts_alt_quality'
+ process_config:
+ enable_cache: true
+
+ records_config:
+ proxy.config.cache.select_alternate: 1
+ proxy.config.cache.limits.http.max_alts: 4
+ proxy.config.http.cache.ignore_query: 1
+
+ plugin_config:
+ - "xdebug.so --enable=x-cache"
+
+ remap_config:
+ - from: "http://example.com/"
+ to: "http://backend.example.com:{SERVER_HTTP_PORT}/"
+
+sessions:
+- transactions:
+ # Store the English response as the first alternate.
+ - all: { headers: { fields: [[ uuid, 1 ]]}}
+ client-request:
+ method: "GET"
+ version: "1.1"
+ scheme: "http"
+ url: /path/request?variant=English
+ delay: 100ms
+ headers:
+ fields:
+ - [ Host, example.com ]
+ - [ Accept-Language, English ]
+ - [ X-Debug, X-Cache ]
+
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [ Content-Length, 0 ]
+ - [ Cache-Control, max-age=60 ]
+ - [ Content-Language, English ]
+ - [ X-Response-Variant, English ]
+
+ proxy-response:
+ status: 200
+ headers:
+ fields:
+ - [ X-Response-Variant, { value: English, as: equal } ]
+ - [ X-Cache, { value: miss, as: equal } ]
+
+ # The plugin rejects the English alternate because its query differs,
+ # allowing the French response to be stored as another alternate.
Review Comment:
The comment says the plugin rejects the English alternate because its query differs, but TSHttpAltInfoQualitySet only scales the match quality and is clamped to [0..1]; it can’t force a miss by itself. In this replay, the miss/store for the French transaction is driven by the core Accept-Language/Content-Language matching, not the query mismatch, so this comment is misleading for anyone debugging the test.
--
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] Add HTTP alternate quality AuTest [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #13391:
URL: https://github.com/apache/trafficserver/pull/13391#discussion_r3591421291
##########
tests/gold_tests/cache/replay/alternate-caching-quality.yaml:
##########
@@ -0,0 +1,142 @@
+# 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.
+
+meta:
+ version: "1.0"
+
+autest:
+ description: 'Verify TSHttpAltInfoQualitySet affects alternate selection'
+
+ dns:
+ name: 'dns_alt_quality'
+
+ server:
+ name: 'server_alt_quality'
+
+ client:
+ name: 'client_alt_quality'
+
+ ats:
+ name: 'ts_alt_quality'
+ process_config:
+ enable_cache: true
+
+ records_config:
+ proxy.config.cache.select_alternate: 1
+ proxy.config.cache.limits.http.max_alts: 4
+ proxy.config.http.cache.ignore_query: 1
+
+ plugin_config:
+ - "xdebug.so --enable=x-cache"
+
+ remap_config:
+ - from: "http://example.com/"
+ to: "http://backend.example.com:{SERVER_HTTP_PORT}/"
+
+sessions:
+- transactions:
+ # Store the English response as the first alternate.
+ - all: { headers: { fields: [[ uuid, 1 ]]}}
+ client-request:
+ method: "GET"
+ version: "1.1"
+ scheme: "http"
+ url: /path/request?variant=English
+ delay: 100ms
+ headers:
+ fields:
+ - [ Host, example.com ]
+ - [ Accept-Language, English ]
+ - [ X-Debug, X-Cache ]
+
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [ Content-Length, 0 ]
+ - [ Cache-Control, max-age=60 ]
+ - [ Content-Language, English ]
+ - [ X-Response-Variant, English ]
+
+ proxy-response:
+ status: 200
+ headers:
+ fields:
+ - [ X-Response-Variant, { value: English, as: equal } ]
+ - [ X-Cache, { value: miss, as: equal } ]
+
+ # The plugin rejects the English alternate because its query differs,
+ # allowing the French response to be stored as another alternate.
Review Comment:
Good catch for this transaction. Accept-Language excludes the English alternate before the select-alt hook runs, so the language mismatch—not query quality—causes the French miss/store. I updated the comment accordingly.
--
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