[GitHub] trafficserver issue #1557: brotli support in gzip plugin

2017-03-23 Thread myraid
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 ...

2017-03-23 Thread maskit
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

2017-03-23 Thread atsci
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

2017-03-23 Thread atsci
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

2017-03-23 Thread atsci
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

2017-03-23 Thread atsci
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

2017-03-23 Thread atsci
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

2017-03-23 Thread atsci
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

2017-03-23 Thread atsci
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 ...

2017-03-23 Thread mingzym
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

2017-03-23 Thread SolidWallOfCode
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

2017-03-23 Thread SolidWallOfCode
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 ...

2017-03-23 Thread oknet
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

2017-03-23 Thread atsci
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 ...

2017-03-23 Thread SolidWallOfCode
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

2017-03-23 Thread atsci
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

2017-03-23 Thread atsci
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

2017-03-23 Thread atsci
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

2017-03-23 Thread atsci
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

2017-03-23 Thread atsci
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

2017-03-23 Thread atsci
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

2017-03-23 Thread shukitchan
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

2017-03-23 Thread shukitchan
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

2017-03-23 Thread shukitchan
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...

2017-03-23 Thread oknet
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...

2017-03-23 Thread atsci
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...

2017-03-23 Thread atsci
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...

2017-03-23 Thread atsci
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...

2017-03-23 Thread atsci
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...

2017-03-23 Thread atsci
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...

2017-03-23 Thread atsci
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...

2017-03-23 Thread atsci
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...

2017-03-23 Thread bluestn
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.
---