Displaying #traffic-server/2015-10-23.log:

Fri Oct 23 00:00:02 2015  jpeach:zwoop: it is super ugly :(
Fri Oct 23 00:00:13 2015  zwoop:jpeach because you hate _'s. Everyone else loves them.
Fri Oct 23 00:00:27 2015  zwoop:gancho_ oh, so it's already in there ?
Fri Oct 23 00:00:32 2015  jpeach:underscores make my pinky cry
Fri Oct 23 00:00:35 2015  sudheerv:lol
Fri Oct 23 00:01:02 2015  sudheerv:i'm neutral to _s
Fri Oct 23 00:01:04 2015  sudheerv::)
Fri Oct 23 00:01:15 2015  sudheerv:and i can be convinced (easily) to accept
Fri Oct 23 00:01:17 2015  gancho_:also added --include-params (which is including only certain query params)
Fri Oct 23 00:01:17 2015  sudheerv:_s
Fri Oct 23 00:01:25 2015  sudheerv:gancho_: nice :)
Fri Oct 23 00:01:33 2015  sudheerv:that's very useful
Fri Oct 23 00:02:07 2015  gancho_:jpeach: I don't really have strong opinion about the name :)
Fri Oct 23 00:02:30 2015  gancho_:--include-headers and -include-cookies
Fri Oct 23 00:02:54 2015  sudheerv:aren't all our plugins using _s already :)
Fri Oct 23 00:03:17 2015  jpeach:incorporate the regex stuff from cacheurl and call it 'cachekey'
Fri Oct 23 00:03:21 2015  gancho_:yes, zwoop it is already there
Fri Oct 23 00:03:24 2015  Amaryllis:@pparam=__include_params?
Fri Oct 23 00:03:31 2015  jpeach:rofl
Fri Oct 23 00:03:40 2015  sudheerv:lol
Fri Oct 23 00:03:45 2015  sudheerv:Amaryllis: just the plugin names :)
Fri Oct 23 00:04:00 2015  gancho_:yes, it has similar thing as the url cache but only for --ua-capture ...
Fri Oct 23 00:04:06 2015  Amaryllis:is cachekey_norm/cachekey-norm/cachekey the one that's waiting for legal?
Fri Oct 23 00:04:32 2015  gancho_:but I can easily add the whole url cache functionality in the cachekey_norm
Fri Oct 23 00:04:51 2015  gancho_:in fact I this was my next step
Fri Oct 23 00:05:05 2015  gancho_:(together with adding yaml configuration)
Fri Oct 23 00:06:10 2015  gancho_:legal promised to expedite the request (just got of the phone)
Fri Oct 23 00:06:51 2015  jpeach:yaml?
Fri Oct 23 00:07:58 2015  gancho_:instead of using plugin parameters to read it from yaml ... since we combined enough things into the plugin it felt that we need better way to configure
Fri Oct 23 00:08:23 2015  gancho_:will see, not implemented yet ...
Fri Oct 23 00:10:47 2015  jpeach:nothing against yaml per se, but not sure how I feel about just having 1 config being yaml
Fri Oct 23 00:11:28 2015  jpeach:the reason I usually prefer command-line options is that it plays well with configuration reloading
Fri Oct 23 00:11:41 2015  gancho_:ok, will show you the whole list of parameters, any suggestions are greatly appreciated :)
Fri Oct 23 00:11:55 2015  jpeach:yeh I can imagine there are many :)
Fri Oct 23 00:11:56 2015  gancho_:I see
Fri Oct 23 00:12:06 2015  jpeach:if all the plugins used yaml, that would be neat
Fri Oct 23 00:12:17 2015  Amaryllis:reveller: where is your cache-key-genid PR?
Fri Oct 23 00:13:12 2015  _klk_:Joined the channel
Fri Oct 23 00:14:04 2015  gancho_:just to clarify the comment about including the cache url functionality into cachekey_norm ... after I added the UA capture /pattern/replace/ realized that it is one more step to allow doing the cacheurl stuff from cachekey_norm ... will play with it ... it might be a good idea
Fri Oct 23 00:14:13 2015  Amaryllis:never mind, found it
Fri Oct 23 00:51:47 2015  rokubo:Joined the channel
Fri Oct 23 01:34:21 2015  _klk_:Joined the channel
Fri Oct 23 02:33:02 2015  jrushford:Joined the channel
Fri Oct 23 02:36:33 2015  Becoming:Joined the channel
Fri Oct 23 02:50:05 2015  gasolwu:Joined the channel
Fri Oct 23 02:53:28 2015  es:Joined the channel
Fri Oct 23 03:32:48 2015  biilmann:Joined the channel
Fri Oct 23 04:02:16 2015  gancho:Joined the channel
Fri Oct 23 05:03:23 2015  oag:Joined the channel
Fri Oct 23 05:10:32 2015  psp:Joined the channel
Fri Oct 23 05:26:49 2015  biilmann:Joined the channel
Fri Oct 23 05:43:46 2015  psp1:Joined the channel
Fri Oct 23 06:48:17 2015  masaori_:Joined the channel
Fri Oct 23 08:52:51 2015  Lethalman:Joined the channel
Fri Oct 23 09:16:23 2015  dynek:Joined the channel
Fri Oct 23 09:19:05 2015  gasolwu1:Joined the channel
Fri Oct 23 09:19:41 2015  dustywusty_:Joined the channel
Fri Oct 23 10:01:10 2015  kichan_:Joined the channel
Fri Oct 23 10:01:34 2015  SaveTheRb0tz:Joined the channel
Fri Oct 23 10:01:41 2015  sudheerv-:Joined the channel
Fri Oct 23 10:02:16 2015  zwoop_:Joined the channel
Fri Oct 23 10:03:17 2015  pdm_:Joined the channel
Fri Oct 23 10:04:23 2015  ggherdov_:Joined the channel
Fri Oct 23 10:06:13 2015  swoc:Joined the channel
Fri Oct 23 10:16:20 2015  briang:Joined the channel
Fri Oct 23 10:16:20 2015  briang:Joined the channel
Fri Oct 23 10:22:55 2015  jpeach:Joined the channel
Fri Oct 23 10:37:49 2015  gancho_:Joined the channel
Fri Oct 23 11:10:27 2015  jrushford:Joined the channel
Fri Oct 23 11:31:21 2015  Amaryllis:what are the implications of changing the layout of struct CacheVC?
Fri Oct 23 12:07:25 2015  bahumbug:Joined the channel
Fri Oct 23 13:19:52 2015  esproul:Joined the channel
Fri Oct 23 13:25:53 2015  esproul:Joined the channel
Fri Oct 23 14:11:57 2015  biilmann:Joined the channel
Fri Oct 23 14:28:20 2015  sudheerv:Amaryllis: umm…add new fields? or reorder fields?
Fri Oct 23 14:29:34 2015  sudheerv:order in some structs related to cache are sensitive enough to break cache compatibility
Fri Oct 23 14:29:42 2015  sudheerv:unsure if CacheVC is one of them - would have to double check
Fri Oct 23 14:30:50 2015  niq:Joined the channel
Fri Oct 23 14:34:37 2015  bahumbug:Joined the channel
Fri Oct 23 14:34:37 2015  bahumbug:Joined the channel
Fri Oct 23 14:35:57 2015  Amaryllis:sudheerv: i worked out i didn't need to in the end :)
Fri Oct 23 14:36:24 2015  davet_:Joined the channel
Fri Oct 23 14:39:39 2015  sudheerv:nice!
Fri Oct 23 14:50:45 2015  Amaryllis:sudheerv: do any other reverse proxies refuse to cache empty documents without content-length? it's not something i've noticed before (in varnish, for example)
Fri Oct 23 14:50:59 2015  Amaryllis:the connection could still be broken at any point in the response and result in broken content being cached
Fri Oct 23 14:51:21 2015  Amaryllis:but as a compromise, what if there was another option to only cache empty responses for 3xx, which is probably 80% of use cases?
Fri Oct 23 14:52:21 2015  Amaryllis:(maybe, in addition, it should accept transfer-encoding: chunked, since that can indicate end-of-body)
Fri Oct 23 14:54:41 2015  zwoop_:Amaryllis we should talk to amc (he knows everything, we don't need Google here) about this
Fri Oct 23 14:54:48 2015  zwoop:Joined the channel
Fri Oct 23 14:55:12 2015  zwoop:Amaryllis amc Maybe we could have a plugin API, that basically forces to set f.allow_empty_doc ?
Fri Oct 23 14:55:42 2015  amc:I think you can change the CacheVC structure without problem.
Fri Oct 23 14:55:46 2015  Amaryllis:i think some sort of fix for this should be in core, at the moment TS won't cache any of our redirects
Fri Oct 23 14:56:04 2015  amc:There's not a global config to allow that?
Fri Oct 23 14:56:10 2015  zwoop:Amaryllis does it send a Cache-Control header with the redirects ?
Fri Oct 23 14:56:25 2015  Amaryllis:amc, zwoop: the origin sends transfer-encoding: chunked instead of content-length.
Fri Oct 23 14:56:33 2015  Amaryllis:so even with cache-control it won't be cached, that's what this PR is to change
Fri Oct 23 14:56:40 2015  Amaryllis:TS-3978
Fri Oct 23 14:56:59 2015  zwoop:ah
Fri Oct 23 14:57:04 2015  zwoop:yeah, that's expected
Fri Oct 23 14:57:11 2015  zwoop:and is why we added that option to allow empty docs to be cached
Fri Oct 23 14:57:18 2015  Amaryllis:this is with that option enabled
Fri Oct 23 14:57:23 2015  zwoop:huh
Fri Oct 23 14:57:24 2015  Amaryllis:it still won't cache any document without content-length
Fri Oct 23 14:57:31 2015  zwoop:yeah, need CL: too
Fri Oct 23 14:57:36 2015  zwoop:that's a requirement
Fri Oct 23 14:57:52 2015  Amaryllis:so i'll update the PR to also recognise chunked responses as being cacheable even if empty
Fri Oct 23 14:57:53 2015  zwoop:otherwise, it can't distinguish the cases you were worried about (a broken connection) vs a truly empty doc
Fri Oct 23 14:58:07 2015  Amaryllis:zwoop: right, but the PR makes it optional, only if allow_empty_doc=2
Fri Oct 23 14:58:09 2015  zwoop:heh, your redirect is Chunked ??
Fri Oct 23 14:58:16 2015  Amaryllis:yes, an empty chunked response :)
Fri Oct 23 14:58:28 2015  zwoop:is that even correct?
Fri Oct 23 14:58:37 2015  Amaryllis:it's perfectly valid, if somewhat unusual
Fri Oct 23 14:58:38 2015  zwoop:doesn't the chunking require the final "0" ?
Fri Oct 23 14:58:41 2015  zwoop:which means, the doc isn't empty
Fri Oct 23 14:58:54 2015  Amaryllis:i think ATS is looking at 'empty' after de-chucking, isn't it?
Fri Oct 23 14:59:00 2015  Amaryllis:because it's definitely not cacheing these responses
Fri Oct 23 14:59:12 2015  zwoop:not sure
Fri Oct 23 14:59:47 2015  amc:I think Amaryllis is correct.
Fri Oct 23 14:59:52 2015  zwoop:Amaryllis but, I definitely remember that the CL: header was a requirement that can not be ignored (safely), so not sure I think having an allow_empty_doc=2 is ok
Fri Oct 23 15:00:15 2015  zwoop:unless allow_empty_doc=2 also means allow Chunked content to be empty
Fri Oct 23 15:00:32 2015  amc:I need to check to see what ATS actually caches - the chunked data or the payload.
Fri Oct 23 15:00:35 2015  zwoop:but, you have to have some indicator telling us that the doc really is empty before we can cache it
Fri Oct 23 15:00:47 2015  zwoop:amc is caches the unchunked data
Fri Oct 23 15:00:54 2015  zwoop:it dechunks it, and caches that
Fri Oct 23 15:01:10 2015  zwoop:is dechunk even a proper verb? :)
Fri Oct 23 15:01:16 2015  amc:Ah, but it caches the encoding header.
Fri Oct 23 15:01:30 2015  Amaryllis:zwoop: https://dpaste.de/6kUT
Fri Oct 23 15:01:38 2015  zwoop:amc Hmmm, really ? That doesn't sound right
Fri Oct 23 15:01:40 2015  amc:"zwoop drank too much at the summit and dechunked himself in the hallway". Yep, a proper verb.
Fri Oct 23 15:01:52 2015  Amaryllis:that's the actual origin response causing problems for us
Fri Oct 23 15:01:54 2015  zwoop:amc I'd expect it to change it from chunked to a Content-Length:
Fri Oct 23 15:02:22 2015  amc:I was wondering about that.
Fri Oct 23 15:02:25 2015  zwoop:amc lets dechunk big time at the Bar camp
Fri Oct 23 15:02:58 2015  amc:What happens when the content is served? Is the encoding preserved and obeyed?
Fri Oct 23 15:03:15 2015  zwoop:for the first client (the cache miss) I think it seems the chunked response
Fri Oct 23 15:03:18 2015  Amaryllis:yes, TS returns a correct thunked response
Fri Oct 23 15:03:24 2015  Amaryllis:and never caches it, so it's always the same
Fri Oct 23 15:03:25 2015  zwoop:but subsequent requests (when served out of cache) should have a CL:
Fri Oct 23 15:03:54 2015  amc:I was thinking of what happens for chunked responses from origin that are non-zero lenght.
Fri Oct 23 15:03:57 2015  zwoop:Amaryllis can you not convince your origin that they are crazy, and change it to not send a Chunked empty response, and instead send CL: 0 ?
Fri Oct 23 15:04:00 2015  sudheerv:catching up on the messages…but, my concern is the same as zwoop's
Fri Oct 23 15:04:07 2015  Amaryllis:zwoop: no, it never adds a CL: https://dpaste.de/Goex
Fri Oct 23 15:04:18 2015  zwoop:Amaryllis because youa re not caching it
Fri Oct 23 15:04:26 2015  Amaryllis:yes, exactly, because it's not cacheable :)
Fri Oct 23 15:04:27 2015  zwoop:Amaryllis amc is asking the general question, to which my answer is
Fri Oct 23 15:04:31 2015  Amaryllis:ah
Fri Oct 23 15:04:52 2015  zwoop:amc yes, I'd expect it to send the chunked response to the first client, and subsequent responses (out of cache) with a CL: 0
Fri Oct 23 15:04:54 2015  Amaryllis:perhaps i can make uwsgi do some sort of output buffering and put a CL header in
Fri Oct 23 15:05:03 2015  zwoop:amc that's at least how it's worked for the last ~10 years, we better not ahve changed it
Fri Oct 23 15:05:13 2015  Amaryllis:zwoop: how can that ever happen? a chunked empty response will never be cached
Fri Oct 23 15:05:24 2015  zwoop:not talking empty responses
Fri Oct 23 15:05:26 2015  zwoop:general case
Fri Oct 23 15:05:29 2015  amc:I have many opinions on this. I do think it should be cacheable as an empty doc if it's chunked with a zero length payload.
Fri Oct 23 15:05:31 2015  Amaryllis:okay, but it's not CL: 0 then
Fri Oct 23 15:05:31 2015  zwoop:your case is deranged
Fri Oct 23 15:05:44 2015  zwoop:Amaryllis right, nothing to do with your case
Fri Oct 23 15:05:53 2015  Amaryllis:amc: shall i submit a separate PR to fix that?
Fri Oct 23 15:06:09 2015  amc:Yes.
Fri Oct 23 15:06:13 2015  zwoop:amc yeah, that seems reasonable. But, If it was me, I'd still beat the Origin people over the head, cause they are crazy
Fri Oct 23 15:06:29 2015  sudheerv:amc: yes, but, you have to ensure that you received the final chunk
Fri Oct 23 15:06:44 2015  sudheerv:if it already is the case, by the time you are checking for allow empty doc caching, then that's fine
Fri Oct 23 15:06:47 2015  amc:Right. That is happening in the clips Amaryllis showed us.
Fri Oct 23 15:06:48 2015  Amaryllis:zwoop: i think it's because the origin sends the headers before it's generated the response body. so it doesn't know it's going to be empty
Fri Oct 23 15:06:49 2015  sudheerv:but, i dont know if that's the case already
Fri Oct 23 15:07:02 2015  zwoop:Amaryllis it would know it's a redirect no?
Fri Oct 23 15:07:10 2015  zwoop:Amaryllis and why would a redirect have a body, ever ?
Fri Oct 23 15:07:11 2015  Amaryllis:yes, but redirects can still have bodies
Fri Oct 23 15:07:14 2015  sudheerv:you may get empy responses without content-length when we support outbound spdy/h2 as well
Fri Oct 23 15:07:17 2015  zwoop:true
Fri Oct 23 15:07:22 2015  Amaryllis:most web servers include bodies in 3xx
Fri Oct 23 15:07:32 2015  Amaryllis:i did have the origin fix it in one specific case but we need something more general
Fri Oct 23 15:07:33 2015  zwoop:yeah, I forgot about that, you're right
Fri Oct 23 15:08:25 2015  amc:If it's chunked and ATS recieves the final payload (0 length frame) it can reasonably presume it got valid (non-truncated) content.
Fri Oct 23 15:08:27 2015  sudheerv:Amaryllis: I think the patch should ensure it can cover non-broken empty body cases for chunked-encoding/spdy/h2
Fri Oct 23 15:08:56 2015  Amaryllis:sudheerv: does this even apply to spdy/h2?
Fri Oct 23 15:09:11 2015  Amaryllis:actually, there is no H2 to origins at all...
Fri Oct 23 15:09:16 2015  sudheerv:not right now - we don't support outbound spdy/h2 yet
Fri Oct 23 15:09:25 2015  Amaryllis:right
Fri Oct 23 15:09:26 2015  sudheerv:but, when we do, it will be a similar problem there
Fri Oct 23 15:09:31 2015  Amaryllis:yes
Fri Oct 23 15:09:35 2015  sudheerv:there's no chunked encoding or content-len with spdy/h2
Fri Oct 23 15:09:38 2015  Amaryllis:but i can't really test code that's yet to be written :)
Fri Oct 23 15:09:43 2015  sudheerv:so, yet another case of empty body :)
Fri Oct 23 15:09:57 2015  sudheerv:no, my point is - the code we write now must not break when we have spdy/h2
Fri Oct 23 15:10:16 2015  sudheerv:so, instead of "assuming" that the body is valid, it might make sense
Fri Oct 23 15:10:21 2015  sudheerv:to somehow indicate that it is
Fri Oct 23 15:10:24 2015  Amaryllis:well, i think it will break anyway, in the sense that empty H2 responses won't be cached... since they have no CL?
Fri Oct 23 15:10:36 2015  sudheerv:well, but you are fixing that ;)
Fri Oct 23 15:10:49 2015  Amaryllis:but H2 doesn't do TE either, does it? i thought it replaces that entire thing
Fri Oct 23 15:10:50 2015  sudheerv:if it's consistently broken (or not supported), that's alright
Fri Oct 23 15:10:56 2015  sudheerv:yes, it doesn't
Fri Oct 23 15:10:58 2015  sudheerv:that's my point
Fri Oct 23 15:11:05 2015  Amaryllis:all i'm adding (for now) is to make it accept CL _or_ TE
Fri Oct 23 15:11:07 2015  sudheerv:so, we need to have a generic way of notifying cache layer
Fri Oct 23 15:11:13 2015  Amaryllis:and test that empty TE is properly checked for the empty chunk
Fri Oct 23 15:11:13 2015  sudheerv:that the response is valid
Fri Oct 23 15:12:01 2015  amc:Hmmm. Maybe a virtual on CacheVConnection "setAllowEmptyContent(bool)".
Fri Oct 23 15:12:13 2015  sudheerv:yeah, something like that
Fri Oct 23 15:12:14 2015  amc:Then an implementation in CacheVC.
Fri Oct 23 15:12:32 2015  sudheerv:amc: i also want to make this setting overridable
Fri Oct 23 15:12:34 2015  amc:Then the higher layer can say "I know this was a valid zero length content, cache monkey"
Fri Oct 23 15:12:39 2015  sudheerv:(but, that's for later)
Fri Oct 23 15:12:49 2015  sudheerv:yes, that makes sense
Fri Oct 23 15:13:04 2015  Amaryllis:amc: CacheVC already has allow_empty_doc flag, i think that's enough
Fri Oct 23 15:13:40 2015  sudheerv:Amaryllis: yeah, it does - I poked around that a bit recently, didn't seem very straightfwd
Fri Oct 23 15:13:41 2015  amc:Just bang on that directly?
Fri Oct 23 15:13:52 2015  sudheerv:but, pls go ahead and do it if you find it easy :)
Fri Oct 23 15:13:59 2015  Amaryllis:amc: that was the plan, yes :)
Fri Oct 23 15:14:13 2015  Amaryllis:amc: https://github.com/apache/trafficserver/pull/310/files#diff-7fada9a23fa1d12e90ca6bec876ecf8fL477
Fri Oct 23 15:14:24 2015  Amaryllis:amc: ... that check just needs another check for chunked as well.
Fri Oct 23 15:15:15 2015  sudheerv:Amaryllis: if we did something like what amc suggested above
Fri Oct 23 15:15:19 2015  amc:No, I meant that it would be set from (for instance) the chunked decoder or just above that.
Fri Oct 23 15:15:22 2015  sudheerv:where the higher layer can notify cache that it can be cached
Fri Oct 23 15:15:27 2015  sudheerv:then we don't have to check any headers
Fri Oct 23 15:15:33 2015  sudheerv:that already seems hacky enough
Fri Oct 23 15:15:38 2015  amc:So there wouldn't be an addiitional value for the config, it would just work as epexted.
Fri Oct 23 15:15:42 2015  amc:expected.
Fri Oct 23 15:15:43 2015  sudheerv:yes
Fri Oct 23 15:15:58 2015  jpeach:there's already a function that checks whether the whole document was received
Fri Oct 23 15:16:10 2015  amc:Hmmm. Interesting.
Fri Oct 23 15:16:31 2015  Amaryllis:i think some things are getting mixed up here - i'm not going to add an additional config value
Fri Oct 23 15:16:36 2015  Amaryllis:that's what the rejected PR did
Fri Oct 23 15:16:45 2015  jpeach:it is used when figuring what to do when getting an EOF
Fri Oct 23 15:16:52 2015  amc:What I would want is a way for the chunk decoder or its parent logic to pass on to the CacheVC the validity of the document.
Fri Oct 23 15:17:06 2015  amc:Amaryllis- I thought your PR added the value '2' to the config variable.
Fri Oct 23 15:17:11 2015  jpeach:afaict as long as you can know you have the entire doc, it could be cacheable
Fri Oct 23 15:17:28 2015  sudheerv:jpeach: that makes sense
Fri Oct 23 15:17:34 2015  Amaryllis:amc: yes, but people disliked that, so instead i suggested fixing it to check for chucked encoding
Fri Oct 23 15:17:41 2015  Amaryllis:amc: which doesn't need an additional config setting
Fri Oct 23 15:17:47 2015  sudheerv:but, i guess the complication is that, at what ppint to do you know that you received the whole doc
Fri Oct 23 15:17:50 2015  amc:OK, I must be reading the wrong PR.
Fri Oct 23 15:17:53 2015  sudheerv:is it before checking allow empty doc or after
Fri Oct 23 15:18:01 2015  Amaryllis:amc: there's only one PR, i didn't write the chunked version yet
Fri Oct 23 15:18:34 2015  sudheerv:if it's just a matter of changing the order to check for whole doc, without breaking anything ;), then that seems like a cleaner solution
Fri Oct 23 15:18:43 2015  amc:Ah, OK.
Fri Oct 23 15:19:11 2015  amc:Yes, I agree. I think we're all on the same page - check for getting the whole document and if it's zero length and the config value is 1, cache it.
Fri Oct 23 15:19:18 2015  Amaryllis:(we need something for 6.0 fairly quickly, so we'll be using this patch in production, but i'm happy to put more effort into fixing it properly)
Fri Oct 23 15:19:20 2015  sudheerv:+1
Fri Oct 23 15:19:27 2015  amc:The brokeness is that complete chunked stuff isn't being checked correctly.
Fri Oct 23 15:19:51 2015  amc:Amaryllis - coool.
Fri Oct 23 15:19:52 2015  Amaryllis:yes - that's the only bit that actually needs to be fixed, but it might also make sense to refactor how the check is done
Fri Oct 23 15:20:20 2015  amc:Allright.
Fri Oct 23 15:20:53 2015  Amaryllis:i'm not sure i know enough about the TS internals to actually implement the second bit though
Fri Oct 23 15:20:53 2015  amc:I have to go get my Macbook fixed - update took out my wireless.
Fri Oct 23 15:21:11 2015  amc:I'll see if I have some time this weekend to take a look.
Fri Oct 23 15:21:44 2015  sudheerv:Amaryllis: if it's okay with everyone else, adding the check for TE header just to fix the TE part alone for now is fine by me as well
Fri Oct 23 15:21:55 2015  sudheerv:(we can open a separate jira to improve the code for future)
Fri Oct 23 15:22:30 2015  sudheerv:(was just saying, that instead of adding case by case, it'd be nicer to have a cleaner solution - but, by no means, it's immediately required)
Fri Oct 23 15:23:05 2015  Amaryllis:sudheerv: sure
Fri Oct 23 15:23:10 2015  jpeach:found it ... HttpSM::is_http_server_eos_truncation
Fri Oct 23 15:23:54 2015  sudheerv:jpeach: that might be a bit late?
Fri Oct 23 15:24:10 2015  sudheerv:it seems to be after tunnel completes
Fri Oct 23 15:24:28 2015  jpeach:the place it is called from might be too late, but that's the logic for knowing if we have the whole response
Fri Oct 23 15:24:33 2015  sudheerv:whereas, the tunnel is writing to UA and Cache in parallel
Fri Oct 23 15:24:34 2015  zwoop:sorry, had to reboot.
Fri Oct 23 15:24:45 2015  sudheerv:jpeach: right, but, the point is
Fri Oct 23 15:24:53 2015  zwoop:amc sudheerv I'm thinking, maybe we should just deprecate this setting completely ? And always allow caching empty docs when we safely can ?
Fri Oct 23 15:24:57 2015  sudheerv:you may get an EOS after some body is recieved already
Fri Oct 23 15:25:04 2015  sudheerv:zwoop: +1 to that
Fri Oct 23 15:25:10 2015  zwoop:it was added for compatibility reasons eons ago, but we changed the default to "1" in 5.x I think
Fri Oct 23 15:25:18 2015  sudheerv:we just need to know when is it safe :)
Fri Oct 23 15:25:41 2015  jpeach:sudheerv: I'm just saying that we have logic to know whether we have the whole response; not that the logic is already used in all the right places
Fri Oct 23 15:25:43 2015  zwoop:we can deprecate it now (marking it as "don't change / use this unless you really, really have to" and remove it for 7.0.0
Fri Oct 23 15:27:03 2015  sudheerv:+1
Fri Oct 23 15:27:31 2015  reveller:Left the channel
Fri Oct 23 15:28:50 2015  zwoop:sudheerv file a Jira on it maybe ? Or two. We should change the docs now (marking it deprecated) and a bug for 7.0.0 for removal
Fri Oct 23 15:29:16 2015  sudheerv:sure..
Fri Oct 23 15:30:13 2015  jrickman:Joined the channel
Fri Oct 23 15:30:16 2015  jpeach:also, you don't need to guarantee that you make the correct caching decision before reading the body
Fri Oct 23 15:30:54 2015  jpeach:you can make a good guess and abort
Fri Oct 23 15:32:30 2015  sudheerv:TS-3979
Fri Oct 23 15:34:55 2015  sudheerv:TS-3980
Fri Oct 23 15:35:29 2015  zwoop:jpeach I don't recall the details, but that was difficult with the way the cache works. Even John couldn't figure it out :)
Fri Oct 23 15:35:48 2015  zwoop:jpeach hence, we agreed on the compromise of allowing empty docs only when we know it's supposed to be empty
Fri Oct 23 15:36:18 2015  sudheerv:yeah, i poked around this too and came to the same conclusion :)
Fri Oct 23 15:36:30 2015  sudheerv:only amc can touch cache
Fri Oct 23 15:36:34 2015  zwoop:not dealing with empty Chunked responses was probably just something we didn't even consider
Fri Oct 23 15:36:47 2015  sudheerv:all of us other mere mortals better stay away from it :)
Fri Oct 23 15:37:18 2015  reveller:Joined the channel
Fri Oct 23 15:38:07 2015  niq:Joined the channel
Fri Oct 23 15:40:15 2015  sudheerv:zwoop: jpeach: how do i mark a config deprecated?
Fri Oct 23 15:40:23 2015  sudheerv:is there a doc that needs to be updated :)?
Fri Oct 23 15:40:34 2015  zwoop:I think there's a way to mark it such, looking
Fri Oct 23 15:40:40 2015  sudheerv:ok, thanks
Fri Oct 23 15:40:49 2015  reveller1:Joined the channel
Fri Oct 23 15:40:52 2015  sudheerv:i've seen it for api, but not for configs
Fri Oct 23 15:41:31 2015  zwoop:https://docs.trafficserver.apache.org/en/latest/arch/hacking/config-var-impl.en.html?highlight=deprecated#documentation-and-defaults
Fri Oct 23 15:41:53 2015  bcall:zwoop: yeah, think it would be a good idea to always cache zero length responses
Fri Oct 23 15:42:00 2015  zwoop:cool
Fri Oct 23 15:42:27 2015  zwoop:yeah, we just dropped the ball on it before, I'm pretty sure the intent was to remove the option. We've been doing a better job the last 1-2 years making sure to file Jira's for the upcoming Major release for removals.
Fri Oct 23 15:42:28 2015  bcall:I was surprised it was a config and off by default when I looked at it awhile back
Fri Oct 23 15:42:37 2015  zwoop:I'm pretty sure it's on now, right ?
Fri Oct 23 15:42:49 2015  zwoop:proxy.config.http.cache.allow_empty_doc", RECD_INT, "1"
Fri Oct 23 15:42:56 2015  bcall:I thought it was off by default - I was looking at 5.3.0
Fri Oct 23 15:43:07 2015  zwoop:I'm certain we made it off by default for compatibility issues
Fri Oct 23 15:43:15 2015  bcall:k
Fri Oct 23 15:43:17 2015  zwoop:but changed it at some point, sounds like we changed it for 6.0 ?
Fri Oct 23 15:43:33 2015  bcall:good to see it is on now
Fri Oct 23 15:43:46 2015  zwoop:TS-2542
Fri Oct 23 15:43:49 2015  jpeach:so the consensus is we can fix this by removing code?
Fri Oct 23 15:44:13 2015  zwoop:jpeach I think we have to fix Amaryllis' problem with Chunked encoded zero lenght objects
Fri Oct 23 15:44:25 2015  bcall:https://issues.apache.org/jira/browse/TS-2542
Fri Oct 23 15:44:31 2015  bcall:changed in 5.0.0
Fri Oct 23 15:44:34 2015  zwoop:yeah
Fri Oct 23 15:44:38 2015  jpeach:ok, my view is that there's nothing to do for that (or almost nothing)
Fri Oct 23 15:44:50 2015  zwoop:jpeach yeah?
Fri Oct 23 15:44:59 2015  zwoop:jpeach it doesn't seem to work though ?
Fri Oct 23 15:45:10 2015  jpeach:we deal with cachiing chunked responses, right?
Fri Oct 23 15:45:24 2015  amc:I'll be at the summit hackathon, look me up.
Fri Oct 23 15:45:38 2015  jpeach:we also deal with the case where we tried to cache but the response was incomplete
Fri Oct 23 15:46:07 2015  jpeach:so by definition, we should already be able to deal with empty chunked responses
Fri Oct 23 15:46:22 2015  zwoop:jpeach but the code explicitly requires a CL: header
Fri Oct 23 15:46:27 2015  zwoop:in fact, CL: 0
Fri Oct 23 15:46:39 2015  Amaryllis:so maybe that check should be removed from set_http_info()?
Fri Oct 23 15:46:46 2015  Amaryllis:and put back somewhere else, if it's not already
Fri Oct 23 15:46:51 2015  jpeach:remove that check
Fri Oct 23 15:47:04 2015  Amaryllis:i can test that now and see how it behaves
Fri Oct 23 15:47:19 2015  zwoop:jpeach I bet that will fail
Fri Oct 23 15:47:44 2015  jpeach:I'm not saying it works, just that we have all the machinery to not shoot ourselves in the foot :)
Fri Oct 23 15:47:57 2015  zwoop:we spent a lot of time on this
Fri Oct 23 15:48:12 2015  jpeach:Amaryllis: could you contribute a TSQA test to explore empty doc caching?
Fri Oct 23 15:49:16 2015  Amaryllis:sure
Fri Oct 23 15:51:14 2015  jpeach:zwoop: afaict Amaryllis' pull request does what I'm suggesting?
Fri Oct 23 15:52:13 2015  zwoop:jpeach I didn't look at it, but if it does, it can't go into 6.x
Fri Oct 23 15:52:17 2015  zwoop:well
Fri Oct 23 15:52:22 2015  zwoop:depends on what it removes :)
Fri Oct 23 15:52:33 2015  zwoop:it can't remove the config test
Fri Oct 23 15:52:35 2015  sudheerv:(Amaryllis's adding a new config setting)
Fri Oct 23 15:52:43 2015  sudheerv:or a new enum for the allow_empty_doc
Fri Oct 23 15:52:45 2015  sudheerv:value:2
Fri Oct 23 15:52:59 2015  sudheerv:to basically ignore the C-L
Fri Oct 23 15:53:12 2015  shinrich1:Joined the channel
Fri Oct 23 15:53:36 2015  Amaryllis:if this logic already exists (which i'm testing now) then the patch is simpler: it should just do if (enable_cache_empty_http_doc) f.allow_empty_doc = 1; else f.allow_empty_doc = 0;
Fri Oct 23 15:53:51 2015  Amaryllis:which can be removed entirely in 7.0
Fri Oct 23 15:53:51 2015  zwoop:i know there were cases which failed miserable when that check was not in place
Fri Oct 23 15:54:03 2015  jpeach:it effectively sets CacheVC::f.allow_empty_doc=1 on the cachevc in every case
Fri Oct 23 15:54:12 2015  zwoop:Amaryllis jpeach try with an HTTP/1.0 response, with no CL nor chunking, and an empty body
Fri Oct 23 15:54:54 2015  zwoop:there's a zillion possibilities here that'd have to be verified :/
Fri Oct 23 15:55:59 2015  jpeach:zwoop: I believe that is_http_server_eos_truncation would consider that response completed?
Fri Oct 23 15:56:00 2015  sudheerv:i agree with zwoop - removing this setting completely (right now) is asking for trouble :)
Fri Oct 23 15:56:17 2015  shinrich1:Joined the channel
Fri Oct 23 15:56:26 2015  zwoop:I'm not against the removal, but I know some very smart people (not me) tried this before.
Fri Oct 23 15:56:30 2015  zwoop:and failed.
Fri Oct 23 15:57:05 2015  jpeach:then the only way forward seems to be to write tests to explore the behaviour
Fri Oct 23 15:58:32 2015  zwoop:TS-621
Fri Oct 23 15:58:43 2015  zwoop:Amaryllis jpeach read the discussions there
Fri Oct 23 15:59:38 2015  zwoop:sounds like it could possibly break clustering, I don't remember any of these details though
Fri Oct 23 16:01:37 2015  es:Joined the channel
Fri Oct 23 16:02:27 2015  Amaryllis:zwoop, jpeach: this is with f.allow_empty_doc forced to 1: https://dpaste.de/dwQ4
Fri Oct 23 16:03:04 2015  gancho:Joined the channel
Fri Oct 23 16:03:10 2015  Amaryllis:so it caches the document with no CL and no TE, which isn't what we want
Fri Oct 23 16:03:24 2015  zwoop:right
Fri Oct 23 16:04:26 2015  jpeach:zwoop: I'm confused, are you saying we need to keep the setting? what about TS-3979?
Fri Oct 23 16:04:45 2015  zwoop:I'm saying, we should remove the setting, but keep the test for CL: 0
Fri Oct 23 16:05:02 2015  sudheerv:jpeach: TS-3979 is to improve the code to make sure it's safe to remove the setting
Fri Oct 23 16:05:02 2015  zwoop:and, figure out how to safely dechunk empty chunked responses and know that those can be cached too
Fri Oct 23 16:05:06 2015  sudheerv:for e.g something like amc mentioned above
Fri Oct 23 16:05:24 2015  sudheerv:someway for the higher layer to tell CacheVC that it's safe to cache
Fri Oct 23 16:05:29 2015  jpeach:ah
Fri Oct 23 16:05:34 2015  sudheerv:not just remove it without any changes :)
Fri Oct 23 16:06:37 2015  sudheerv:zwoop: actually, once we figure out (assuming it's possible) a safe way to cache any doc (including empty) - we can even consider removing the C-L:0 check?
Fri Oct 23 16:06:52 2015  zwoop:yeah sure
Fri Oct 23 16:06:54 2015  sudheerv:of course, that's a big ? - that we can indeed figure that out :)
Fri Oct 23 16:06:58 2015  sudheerv:cool
Fri Oct 23 16:07:05 2015  zwoop:that's the initial plan (TS-621), but we couldn't get it to work
Fri Oct 23 16:07:15 2015  zwoop:it was riddled with complexity and scary stuff all over the place :)
Fri Oct 23 16:07:18 2015  sudheerv:yeah, i see that..:)
Fri Oct 23 16:07:21 2015  sudheerv:lol
Fri Oct 23 16:07:28 2015  sudheerv:nothing new there..Cache is scary!
Fri Oct 23 16:07:37 2015  zwoop:it got worse too because of cache clsutering
Fri Oct 23 16:07:48 2015  sudheerv:yeah, now we don't have that problem to deal with, at least
Fri Oct 23 16:07:49 2015  sudheerv:thankfully
Fri Oct 23 16:07:51 2015  zwoop:odd things started happening in clustering, and we have zero tests on that now :/
Fri Oct 23 16:07:56 2015  Amaryllis:sudheerv: how come?
Fri Oct 23 16:08:01 2015  sudheerv:cluster is not supported anymore?
Fri Oct 23 16:08:04 2015  zwoop:I used to have a setup to test clustering in, but don't any more (I probably should)
Fri Oct 23 16:08:09 2015  Amaryllis:hm. we're using cluster extensively in production
Fri Oct 23 16:08:17 2015  Amaryllis:is that in 7.0?
Fri Oct 23 16:08:17 2015  sudheerv:oh..then sorry
Fri Oct 23 16:08:20 2015  zwoop:sudheerv well, it's not not supported...
Fri Oct 23 16:08:36 2015  zwoop:it's just that we don't have anyone who actively works on cache clustering
Fri Oct 23 16:08:37 2015  sudheerv:i thought, we stopped supporting clustering
Fri Oct 23 16:08:40 2015  zwoop:no
Fri Oct 23 16:08:42 2015  sudheerv:but i could be badly mistaken
Fri Oct 23 16:08:45 2015  sudheerv:yeah
Fri Oct 23 16:09:10 2015  sudheerv:Amaryllis: if you are using clustering, then all the more reason for you to trust zwoop's words of advice about the C-L:0 check :)
Fri Oct 23 16:09:35 2015  zwoop:sudheerv it's just been mothballed soort of
Fri Oct 23 16:09:42 2015  sudheerv:(we don't use clustering (and dont plan to even), so, i actually don't really mind that much)
Fri Oct 23 16:09:44 2015  sudheerv:yeah
Fri Oct 23 16:10:36 2015  zwoop:I'd like to have more development on cache clustering, we just don't have anyone that cares. It was also complicated before to get changes done to it that would improve situations, because of legacy systems that someone had. That's no longer an issue though, so we could definitely redesign / improve cache clustering much easier now.
Fri Oct 23 16:10:54 2015  Amaryllis:sudheerv: it looks like this would be broken anyway though, if cluster can't read an empty object from another machine
Fri Oct 23 16:11:13 2015  Amaryllis:even with the CL check
Fri Oct 23 16:11:29 2015  zwoop:for example, we could / should incorporate some of PSUdaemon's stuff into cache clustering, and ideally, ways for it to detect / decide upon "hot" content in an intelligent way (and not via hardcoded URL rules)
Fri Oct 23 16:11:47 2015  sudheerv:parent cache stuff?
Fri Oct 23 16:11:49 2015  zwoop:Amaryllis yeah, I'm unclear on that as well, maybe you can test that too
Fri Oct 23 16:12:11 2015  zwoop:sudheerv yeah, Comcast is making a lot of improvements to parent selection, some of which probably could get incorporated (replace) things in cache clsutering
Fri Oct 23 16:12:17 2015  sudheerv:cool
Fri Oct 23 16:12:35 2015  Amaryllis:zwoop: yeah, i can test this in production, let's see
Fri Oct 23 16:12:45 2015  zwoop: :)
Fri Oct 23 16:13:00 2015  zwoop:I was hoping you had a dev / staging environment, but wth, production is good
Fri Oct 23 16:13:09 2015  sudheerv:lol
Fri Oct 23 16:13:21 2015  zwoop:all I'm saying here really is, be careful here, we've been down this path before, and failed miserable.
Fri Oct 23 16:14:24 2015  Amaryllis:zwoop: yeah, it doesn't work: https://dpaste.de/cSA2
Fri Oct 23 16:14:32 2015  jrushford:Joined the channel
Fri Oct 23 16:14:37 2015  zwoop:doh
Fri Oct 23 16:14:43 2015  zwoop:Amaryllis fix it? :-)
Fri Oct 23 16:15:14 2015  jpeach:Amaryllis: that is the cache cluster?
Fri Oct 23 16:15:25 2015  Amaryllis:jpeach: yes
Fri Oct 23 16:15:39 2015  jpeach:honestly I'm surprised that clustering still works
Fri Oct 23 16:18:08 2015  Amaryllis:i'd switch to ICP (or whatever else) but i don't think it works for us at the moment... if a document is purged from one cache, the stale document could be retrieved from the other before it's purged there, and re-cached
Fri Oct 23 16:18:18 2015  Amaryllis:unless there's a way to make ICP not cache documents locally?
Fri Oct 23 16:21:27 2015  zwoop:ICP is inherently broken by design
Fri Oct 23 16:21:29 2015  jpeach:PSUdaemon: does Traffic Control deal with purges?
Fri Oct 23 16:21:43 2015  zwoop:if we're going to do some major work in this area, I hope we'll implement HTCP
Fri Oct 23 16:21:48 2015  sudheerv:jpeach: dcarlin does something similar with regex_revalidate
Fri Oct 23 16:22:13 2015  sudheerv:he has a nice centralized way of purging out of multiple caches
Fri Oct 23 16:22:17 2015  sudheerv:dcarlin: ^^ ?
Fri Oct 23 16:25:36 2015  zwoop:Amaryllis yeah, no way to work around that in ICP... What's worse is that ICP is broken in that when it asks its peers for a URL, it doesn't include any headers, so you can't negotiate alternates correctly. So you can get any random response (if there are multiple alternates) and just have to throw it away anyways :/
Fri Oct 23 16:27:11 2015  Amaryllis:so how do people do redundancy and keep decent cache efficiency?
Fri Oct 23 16:27:54 2015  sudheerv:Amaryllis: we use carp
Fri Oct 23 16:28:05 2015  Amaryllis:what does the CARP hashing? load balancer?
Fri Oct 23 16:28:27 2015  zwoop:CARP is very, very similar to our cache clustering
Fri Oct 23 16:28:30 2015  sudheerv:we have load balancers too, but, carp hashing is done directly amongst the peers themselves
Fri Oct 23 16:28:45 2015  zwoop:Flickr tried CARP with Squid, and the hashing algorithm was very poor
Fri Oct 23 16:28:59 2015  sudheerv:zwoop: yeah, we use carp for flickr too (ycs-ct)
Fri Oct 23 16:29:10 2015  sudheerv:mainly for long tail content
Fri Oct 23 16:29:25 2015  sudheerv:for other regular content, we just use vip/load balancers
Fri Oct 23 16:29:35 2015  sudheerv:expect the content to be cached all over typically
Fri Oct 23 16:30:24 2015  Amaryllis:we used CARP with Squid at Wikipedia but it required running two separates squids on each host, one to do CARP and one to cache
Fri Oct 23 16:37:54 2015  gancho:Joined the channel
Fri Oct 23 16:37:57 2015  zwoop:ah
Fri Oct 23 16:38:36 2015  sudheerv:we have a ats carp plugin that does the carp for us
Fri Oct 23 16:45:10 2015  jrushford:Joined the channel
Fri Oct 23 16:49:16 2015  gancho:Joined the channel
Fri Oct 23 16:54:39 2015  gancho:Joined the channel
Fri Oct 23 17:02:04 2015  es:Joined the channel
Fri Oct 23 17:04:42 2015  Becoming:Joined the channel
Fri Oct 23 17:06:22 2015  biilmann:Joined the channel
Fri Oct 23 17:11:15 2015  gancho:Joined the channel
Fri Oct 23 17:14:38 2015  gancho:Joined the channel
Fri Oct 23 17:38:03 2015  gancho:Joined the channel
Fri Oct 23 17:39:14 2015  _klk_:Joined the channel
Fri Oct 23 17:42:27 2015  _klk_:Joined the channel
Fri Oct 23 17:49:55 2015  gancho:Joined the channel
Fri Oct 23 17:54:05 2015  gancho:Joined the channel
Fri Oct 23 18:02:05 2015  _klk_1:Joined the channel
Fri Oct 23 18:04:32 2015  es:Joined the channel
Fri Oct 23 18:08:03 2015  gancho:Joined the channel
Fri Oct 23 18:13:06 2015  amc:Although these days I think you can do most of that with parent proxy config.
Fri Oct 23 18:20:26 2015  gancho:Joined the channel
Fri Oct 23 18:23:14 2015  gancho:Joined the channel
Fri Oct 23 18:55:01 2015  blattj:Joined the channel
Fri Oct 23 18:55:27 2015  jrushford:Joined the channel
Fri Oct 23 19:10:31 2015  esproul1:Joined the channel
Fri Oct 23 19:16:15 2015  gancho:Joined the channel
Fri Oct 23 19:35:38 2015  biilmann:Joined the channel
Fri Oct 23 19:37:33 2015  bahumbug:Joined the channel
Fri Oct 23 19:37:33 2015  bahumbug:Joined the channel
Fri Oct 23 19:42:52 2015  _klk_:Joined the channel
Fri Oct 23 19:59:58 2015  _klk_1:Joined the channel
Fri Oct 23 20:11:45 2015  oag:Joined the channel
Fri Oct 23 20:15:56 2015  jrickman:Joined the channel
Fri Oct 23 20:19:44 2015  msekimura:Joined the channel
Fri Oct 23 20:50:02 2015  psp:Joined the channel
Fri Oct 23 20:51:48 2015  psp1:Joined the channel
Fri Oct 23 20:54:42 2015  _klk_:Joined the channel
Fri Oct 23 21:10:14 2015  biilmann:Joined the channel
Fri Oct 23 22:47:19 2015  gancho:Joined the channel
Fri Oct 23 22:48:54 2015  niq:Joined the channel
Fri Oct 23 23:11:13 2015  _klk_:Joined the channel
Fri Oct 23 23:31:13 2015  _klk_:Joined the channel
Fri Oct 23 23:39:18 2015  _klk_:Joined the channel

Comments