[GitHub] trafficserver issue #1557: brotli support in gzip plugin
Github user myraid commented on the issue: https://github.com/apache/trafficserver/pull/1557 fixed the build failures with preprocessor derivatives. @shukitchan @bryancall @zwoop is this approach OK? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1583: Set HttpSM::tunnel_handler_post to handle ...
Github user maskit commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1583#discussion_r107818082 --- Diff: proxy/http/HttpSM.cc --- @@ -2771,7 +2771,8 @@ HttpSM::tunnel_handler_post(int event, void *data) } if (event != HTTP_TUNNEL_EVENT_DONE) { -if ((event == VC_EVENT_WRITE_COMPLETE) || (event == VC_EVENT_EOS)) { +if ((event == VC_EVENT_WRITE_COMPLETE) || (event == VC_EVENT_EOS) || (event == VC_EVENT_ERROR) || --- End diff -- I think we should add the inline function to make sure that the same condition is used under the same context, and to clarify the condition we expect. The switch case style is more readable than current style though, it doesn't tell us what the case is. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1601: TS-4976: Regularize plugins - protocol
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1601 clang-analyzer build *successful*! https://ci.trafficserver.apache.org/job/clang-analyzer-github/353/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1601: TS-4976: Regularize plugins - protocol
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1601 Intel CC build *successful*! https://ci.trafficserver.apache.org/job/icc-github/221/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1601: TS-4976: Regularize plugins - protocol
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1601 Linux build *successful*! https://ci.trafficserver.apache.org/job/linux-github/1683/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1601: TS-4976: Regularize plugins - protocol
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1601 FreeBSD11 build *successful*! https://ci.trafficserver.apache.org/job/freebsd-github/1790/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1601: TS-4976: Regularize plugins - protocol
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1601 AU check *successful*! https://ci.trafficserver.apache.org/job/autest-github/92/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1601: TS-4976: Regularize plugins - protocol
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1601 clang format *successful*! https://ci.trafficserver.apache.org/job/clang-format-github/95/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1601: TS-4976: Regularize plugins - protocol
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1601 RAT check *successful*! https://ci.trafficserver.apache.org/job/RAT-github/108/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1583: Set HttpSM::tunnel_handler_post to handle ...
Github user mingzym commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1583#discussion_r107705366 --- Diff: proxy/http/HttpSM.cc --- @@ -2771,7 +2771,8 @@ HttpSM::tunnel_handler_post(int event, void *data) } if (event != HTTP_TUNNEL_EVENT_DONE) { -if ((event == VC_EVENT_WRITE_COMPLETE) || (event == VC_EVENT_EOS)) { +if ((event == VC_EVENT_WRITE_COMPLETE) || (event == VC_EVENT_EOS) || (event == VC_EVENT_ERROR) || --- End diff -- switch case is much ATS style than any others, more clear for Human reading. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1601: TS-4976: Regularize plugins - protocol
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1601 @maskit 's comment reminds me that I also remove a number of `printf` statements which seemed to duplicate the debug output. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1601: TS-4976: Regularize plugins - protocol
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1601#discussion_r107704029 --- Diff: example/protocol/Protocol.c --- @@ -122,33 +122,29 @@ TSPluginInit(int argc, const char *argv[]) server_port = 4666; if (argc < 3) { -TSDebug("protocol", "Usage: protocol.so accept_port server_port"); +TSDebug(PLUGIN_NAME, "Usage: protocol.so accept_port server_port"); printf("[protocol_plugin] Usage: protocol.so accept_port server_port\n"); --- End diff -- Probably not, I must have missed that one. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1583: Set HttpSM::tunnel_handler_post to handle ...
Github user oknet commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1583#discussion_r107694127 --- Diff: proxy/http/HttpSM.cc --- @@ -2771,7 +2771,8 @@ HttpSM::tunnel_handler_post(int event, void *data) } if (event != HTTP_TUNNEL_EVENT_DONE) { -if ((event == VC_EVENT_WRITE_COMPLETE) || (event == VC_EVENT_EOS)) { +if ((event == VC_EVENT_WRITE_COMPLETE) || (event == VC_EVENT_EOS) || (event == VC_EVENT_ERROR) || --- End diff -- I can rewrite it to switch-case : ``` switch (event) { case VC_EVENT_WRITE_COMPLETE: case VC_EVENT_EOS: case VC_EVENT_ERROR: case VC_EVENT_INACTIVITY_TIMEOUT: case VC_EVENT_ACTIVE_TIMEOUT: // do something break; default: break; } ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1557: brotli support in gzip plugin
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1557 clang-analyzer build *successful*! https://ci.trafficserver.apache.org/job/clang-analyzer-github/352/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1583: Set HttpSM::tunnel_handler_post to handle ...
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1583#discussion_r107681516 --- Diff: proxy/http/HttpSM.cc --- @@ -2771,7 +2771,8 @@ HttpSM::tunnel_handler_post(int event, void *data) } if (event != HTTP_TUNNEL_EVENT_DONE) { -if ((event == VC_EVENT_WRITE_COMPLETE) || (event == VC_EVENT_EOS)) { +if ((event == VC_EVENT_WRITE_COMPLETE) || (event == VC_EVENT_EOS) || (event == VC_EVENT_ERROR) || --- End diff -- @shinrich and I discussed this a bit. It's a big ugly to have these two long conditionals that are identical but not obviously so. What I would recommend is adding an inline function like `isTxnTerminalEvent` that covers `EOS`, `ERROR`, `INACTIVITY_TIMEOUT` and `INACTIVITY_TIMEOUT` and have ``` if (event == VC_EVENT_WRITE_COMPLETE || isEventTxTerminal(event)) { ``` which would be easier to understand and easier to see the set of events are the same here and on line 2784. Our opinion is this would be useful in no small number of other places in the code as well and help prevent problems like this in the future. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1557: brotli support in gzip plugin
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1557 FreeBSD11 build *failed*! https://ci.trafficserver.apache.org/job/freebsd-github/1789/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1557: brotli support in gzip plugin
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1557 Linux build *failed*! https://ci.trafficserver.apache.org/job/linux-github/1682/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1557: brotli support in gzip plugin
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1557 Intel CC build *failed*! https://ci.trafficserver.apache.org/job/icc-github/220/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1557: brotli support in gzip plugin
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1557 RAT check *successful*! https://ci.trafficserver.apache.org/job/RAT-github/107/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1557: brotli support in gzip plugin
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1557 clang format *successful*! https://ci.trafficserver.apache.org/job/clang-format-github/94/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1557: brotli support in gzip plugin
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1557 AU check *successful*! https://ci.trafficserver.apache.org/job/autest-github/91/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1557: brotli support in gzip plugin
Github user shukitchan commented on the issue: https://github.com/apache/trafficserver/pull/1557 it looks good to me. @bryancall @zwoop can either of you guys take a look, too? thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1557: brotli support in gzip plugin
Github user shukitchan commented on the issue: https://github.com/apache/trafficserver/pull/1557 [approve ci] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1557: brotli support in gzip plugin
Github user shukitchan commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1557#discussion_r107666043 --- Diff: plugins/gzip/gzip.cc --- @@ -115,20 +142,32 @@ gzip_data_destroy(GzipData *data) } static TSReturnCode -gzip_content_encoding_header(TSMBuffer bufp, TSMLoc hdr_loc, const int compression_type) +content_encoding_header(TSMBuffer bufp, TSMLoc hdr_loc, const int compression_type, int algorithm) { TSReturnCode ret; TSMLoc ce_loc; - + const char *value = nullptr; + int value_len = 0; // Delete Content-Encoding if present??? + if (compression_type & COMPRESSION_TYPE_BROTLI && (algorithm & ALGORITHM_BROTLI)) { --- End diff -- so does that mean i must have "br" in supported-algorithm for the content header to be set accordingly? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1540: Metalink Plugin: Must not destroy the Transform C...
Github user oknet commented on the issue: https://github.com/apache/trafficserver/pull/1540 The transform contp will be closed and destroied after TransformVC is closed. The transformation plugin receives EVENT_IMMEDIATE when transform contp close then the plugin could release 'TransformData' and call TSContDestroy(). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1540: Metalink Plugin: Must not destroy the Transform C...
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1540 clang-analyzer build *successful*! https://ci.trafficserver.apache.org/job/clang-analyzer-github/351/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1540: Metalink Plugin: Must not destroy the Transform C...
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1540 Intel CC build *successful*! https://ci.trafficserver.apache.org/job/icc-github/219/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1540: Metalink Plugin: Must not destroy the Transform C...
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1540 Linux build *successful*! https://ci.trafficserver.apache.org/job/linux-github/1681/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1540: Metalink Plugin: Must not destroy the Transform C...
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1540 FreeBSD11 build *successful*! https://ci.trafficserver.apache.org/job/freebsd-github/1788/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1540: Metalink Plugin: Must not destroy the Transform C...
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1540 AU check *failed*! https://ci.trafficserver.apache.org/job/autest-github/90/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1540: Metalink Plugin: Must not destroy the Transform C...
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1540 RAT check *successful*! https://ci.trafficserver.apache.org/job/RAT-github/106/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1540: Metalink Plugin: Must not destroy the Transform C...
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/1540 clang format *successful*! https://ci.trafficserver.apache.org/job/clang-format-github/93/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1603: if we had a server with big diskspace, function b...
GitHub user bluestn opened an issue: https://github.com/apache/trafficserver/issues/1603 if we had a server with big diskspace, function build_vol_hash_table cost to much to build the vol_hash_table for CacheVol **if the Volume space is 50TB, the rtable_size will be 50* 1000 * 1000 / 8=625,it cost too much memory and cpu usage( see the code in build_vol_hash_table listed below), maybe we need to optimize this build_vol_hash_table function.** rtable_pair *rtable = (rtable_pair *)ats_malloc(sizeof(rtable_pair) * rtable_size); int rindex = 0; for (int i = 0; i < num_vols; i++) for (int j = 0; j < (int)rtable_entries[i]; j++) { rtable[rindex].rval = next_rand(&rnd[i]); rtable[rindex].idx = i; rindex++; } --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---