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 2024/01/04 20:14:24 UTC
[PR] multiplexer: debug logging and variable name updates [trafficserver]
bneradt opened a new pull request, #10964:
URL: https://github.com/apache/trafficserver/pull/10964
This is a multiplexer cleanup PR that adds debug logging to post.cc and updates variable names for the transform in order to make their purpose explicit (for the origin, for a copy, etc.). In post.cc, for instance, both the incoming and outgoing VIO were named `vio`, with the difference being whether it was called off of the state `s` variable or as a local variable. This updates the names to clarify the purpose of each.
Other than logging changes, this patch makes no functional change to multiplexer.
--
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] multiplexer: debug logging and variable name updates [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #10964:
URL: https://github.com/apache/trafficserver/pull/10964#discussion_r1442254692
##########
plugins/multiplexer/dispatch.cc:
##########
@@ -229,6 +231,7 @@ generateRequests(const Origins &o, const TSMBuffer buffer, const TSMLoc location
const std::string &host = *iterator;
assert(!host.empty());
request.hostHeader(host);
+ Dbg(dbg_ctl, "Prepaing request for \"%s\"", host.c_str());
Review Comment:
Good catch. Thanks! 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] multiplexer: debug logging and variable name updates [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #10964:
URL: https://github.com/apache/trafficserver/pull/10964#issuecomment-1877705572
[approve ci]
--
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] multiplexer: debug logging and variable name updates [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt merged PR #10964:
URL: https://github.com/apache/trafficserver/pull/10964
--
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] multiplexer: debug logging and variable name updates [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #10964:
URL: https://github.com/apache/trafficserver/pull/10964#discussion_r1442213828
##########
plugins/multiplexer/post.cc:
##########
@@ -48,55 +52,65 @@ postTransform(const TSCont c, PostState &s)
{
assert(c != nullptr);
- const TSVConn vconnection = TSTransformOutputVConnGet(c);
- assert(vconnection != nullptr);
+ // As we collect data from the client, we need to write it to the origin. This
+ // is for the original origin. The copies are handled via HttpTransaction
+ // logic in fetcher.h.
+ const TSVConn output_vconn = TSTransformOutputVConnGet(c);
+ assert(output_vconn != nullptr);
- const TSVIO vio = TSVConnWriteVIOGet(c);
- assert(vio != nullptr);
+ // The VIO from which we pull out the client's request.
+ const TSVIO input_vio = TSVConnWriteVIOGet(c);
+ assert(input_vio != nullptr);
- if (!s.buffer) {
- s.buffer = TSIOBufferCreate();
- assert(s.buffer != nullptr);
+ if (!s.origin_buffer) {
+ s.origin_buffer = TSIOBufferCreate();
+ assert(s.origin_buffer != nullptr);
- const TSIOBufferReader reader = TSIOBufferReaderAlloc(s.buffer);
- assert(reader != nullptr);
+ TSIOBufferReader origin_reader = TSIOBufferReaderAlloc(s.origin_buffer);
+ assert(origin_reader != nullptr);
- s.reader = TSIOBufferReaderClone(reader);
- assert(s.reader != nullptr);
+ s.clone_reader = TSIOBufferReaderClone(origin_reader);
+ assert(s.clone_reader != nullptr);
- s.vio = TSVConnWrite(vconnection, c, reader, std::numeric_limits<int64_t>::max());
- assert(s.vio != nullptr);
+ s.output_vio = TSVConnWrite(output_vconn, c, origin_reader, std::numeric_limits<int64_t>::max());
+ assert(s.output_vio != nullptr);
}
- if (!TSVIOBufferGet(vio)) {
- TSVIONBytesSet(s.vio, TSVIONDoneGet(vio));
- TSVIOReenable(s.vio);
+ if (!TSVIOBufferGet(input_vio)) {
+ if (s.output_vio) {
Review Comment:
This is the only "interesting" change in this PR in which I add a nullptr check just in 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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] multiplexer: debug logging and variable name updates [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #10964:
URL: https://github.com/apache/trafficserver/pull/10964#issuecomment-1877790726
[approve ci autest]
--
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] multiplexer: debug logging and variable name updates [trafficserver]
Posted by "randall (via GitHub)" <gi...@apache.org>.
randall commented on code in PR #10964:
URL: https://github.com/apache/trafficserver/pull/10964#discussion_r1442249608
##########
plugins/multiplexer/dispatch.cc:
##########
@@ -229,6 +231,7 @@ generateRequests(const Origins &o, const TSMBuffer buffer, const TSMLoc location
const std::string &host = *iterator;
assert(!host.empty());
request.hostHeader(host);
+ Dbg(dbg_ctl, "Prepaing request for \"%s\"", host.c_str());
Review Comment:
Spelling (Preparing)
--
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] multiplexer: debug logging and variable name updates [trafficserver]
Posted by "oknet (via GitHub)" <gi...@apache.org>.
oknet commented on code in PR #10964:
URL: https://github.com/apache/trafficserver/pull/10964#discussion_r1443645447
##########
plugins/multiplexer/post.cc:
##########
@@ -48,55 +52,65 @@ postTransform(const TSCont c, PostState &s)
{
assert(c != nullptr);
- const TSVConn vconnection = TSTransformOutputVConnGet(c);
- assert(vconnection != nullptr);
+ // As we collect data from the client, we need to write it to the origin. This
+ // is for the original origin. The copies are handled via HttpTransaction
+ // logic in fetcher.h.
+ const TSVConn output_vconn = TSTransformOutputVConnGet(c);
+ assert(output_vconn != nullptr);
- const TSVIO vio = TSVConnWriteVIOGet(c);
- assert(vio != nullptr);
+ // The VIO from which we pull out the client's request.
+ const TSVIO input_vio = TSVConnWriteVIOGet(c);
+ assert(input_vio != nullptr);
- if (!s.buffer) {
- s.buffer = TSIOBufferCreate();
- assert(s.buffer != nullptr);
+ if (!s.origin_buffer) {
+ s.origin_buffer = TSIOBufferCreate();
+ assert(s.origin_buffer != nullptr);
- const TSIOBufferReader reader = TSIOBufferReaderAlloc(s.buffer);
- assert(reader != nullptr);
+ TSIOBufferReader origin_reader = TSIOBufferReaderAlloc(s.origin_buffer);
+ assert(origin_reader != nullptr);
- s.reader = TSIOBufferReaderClone(reader);
- assert(s.reader != nullptr);
+ s.clone_reader = TSIOBufferReaderClone(origin_reader);
+ assert(s.clone_reader != nullptr);
- s.vio = TSVConnWrite(vconnection, c, reader, std::numeric_limits<int64_t>::max());
- assert(s.vio != nullptr);
+ s.output_vio = TSVConnWrite(output_vconn, c, origin_reader, std::numeric_limits<int64_t>::max());
+ assert(s.output_vio != nullptr);
}
- if (!TSVIOBufferGet(vio)) {
- TSVIONBytesSet(s.vio, TSVIONDoneGet(vio));
- TSVIOReenable(s.vio);
+ if (!TSVIOBufferGet(input_vio)) {
+ if (s.output_vio) {
Review Comment:
The value of `s.output_vio` is always `non-null` because it returned by `INKVConnInternal::do_io_write()`.
It's good job to prefix the variable names with `input` and `output`.
--
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