Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/206#issuecomment-107280577
I don't have any particular thoughts on this, other than if we do this, we
also add an API to query into this "source" of a particular config. E.g.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/225#issuecomment-112213434
Couple of nit-picks:
1) The documentations is slightly confusing now. What's a "variable" in the
XML context? What we should document
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/161#issuecomment-112871458
Please close this.
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/141#issuecomment-112871149
We've landed LinkedIn's framework, can you please close this pull request?
---
If your project is set up for it, you can reply to this email and have
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/189#issuecomment-112872925
Thomas: Should we start making Jiras's for these new tsqa features ? It'd
be nice to track what is being added outside of other code changes.
---
If yo
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/199#issuecomment-112874297
I'll take a look at this. Is there a Jira for this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/229#issuecomment-112961947
Please check with John (the current assignee of the TS-2150 Jira).
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/229#issuecomment-114597667
+1. No STL on the critical path.
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/245#issuecomment-120083151
Absolutely agree, lets not put more stuff into httpui, we should aim to
eliminate it for 7.0.0. Move things over to the management port and traffic_ctl
please
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/254#issuecomment-122645818
I have a few concerns with the code here actually. In general, we want more
configurations overridable, including cache configurations and network
configurations
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/258#issuecomment-123593668
Yeah, close TS-2152 as a dupe of TS-3780, since there's so much work done
on this PR already. In general, please file Jira's before work / pull requests
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/255#issuecomment-123612759
@bgaff Please review and commit. Also, make sure the mnemonics for this
new tag (and TS-3780) are in line with existing standards.
---
If your project is set up
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/262#issuecomment-126043948
I haven't had a chance to review all this, but reading the last few
comments, makes me wonder: If a user ends up in such a situation (no resolvers
available
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/203#issuecomment-126513715
Yeah, please submit a new pull request.
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/203#issuecomment-126513769
Also, since we closed TS-3632, please open a new Jira for this as well.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/268#issuecomment-127820259
Thanks Masaori and Thomas. I've merged this pull request.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitH
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/271#issuecomment-128445110
This seems reasonable to me as well. If I understand this, basically you
are saying, if we expect no body, also explicitly ignore any transfer encoding?
---
If
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/270#issuecomment-130126864
That seems draconian to rule out these methods only because they are not in
the main HTTP RFCs. Since ATS is a general forward (and transparent) proxy, it
seems
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/270#issuecomment-130127185
I should say, if there was a reasonable way to add functionality to
ip_allow.config via plugins, that would make a lot of sense. But, I don't think
the
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/270#issuecomment-130491241
@ushachar Ah, so the argument is to not add special handling for this, but
still allow them to be used in ip_allow.config? If so, that seems fine.
---
If your
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/270#issuecomment-134748516
Ok, I think the consensus is that we don't change anything. Sorry Meera,
but thanks for all the work. And I whole heartedly agree that we should
eliminate th
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/293#issuecomment-140170568
As discussed on the IRC, I think we should
1) Change the subject for this Jira, I thought isSSL was the name of the
new tag.
2) The description
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/293#discussion_r39430575
--- Diff: proxy/logging/Log.cc ---
@@ -478,7 +478,7 @@ Log::init_fields()
global_field_list.add(field, false);
ink_hash_table_insert
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/300#issuecomment-145669029
it ought to be possible to make a standalone test program, that exercises
the config loading / parsing, without having to run as a plugin. Basically,
something
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/302#issuecomment-146684009
This also must be run through clang-format first.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/302#issuecomment-146685122
Great, 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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/304#issuecomment-146882957
James, it does hold this lock in a lot of places. :) The crash seems to
happens when something (e.g. propstats) modifies librecords while at the same
time
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/199#issuecomment-151287464
Ping. Can we get a Jira for this? I'm ready to start working on this, sorry
for the delay.
---
If your project is set up for it, you can reply to this emai
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/199#issuecomment-151287788
In addition to the proposals here, I think we should generalize this
concept. There are many other use cases for this approach, such as replacing
the IP in the Via
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/330#issuecomment-157917453
Can you provide some example URLs where this fails proxying through ATS?
I'm trying t both reproduce and engage some friends over at Akamai, to fully
under
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/346
TS-3966 Allow for pre-origin requests to be denied in header_rewrite
This generalizes the concept of set-status, such that it also works in
remap read-request hooks. I.e. prior to having a
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/360#issuecomment-161801444
Hmmm, I'll have to review this further, but there seems to be a lot other
changes here in this PR, unrelated to this actual fix. Why is that? Why is so
much
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/361#issuecomment-161826127
Much better!
I'll have to check / read the code more thoroughly, but are you sure this
isn't now leaking the object when the freelis
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/361#issuecomment-162083854
Yeah, so there's no leak here (sorry, my bad), but I feel these checks are
unnecessary. I'd rather change this to have a release assert above
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/369#issuecomment-162997233
This mostly looks good, except in
+char *
+ParentRecord::getHashPath(RequestData *rdata, size_t *path_len)
+{
I think, but
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/369#issuecomment-163996272
Looks good to me now, +1. It's a little difficult to decipher the
refactoring in ParentRecord::FindParent, I assume that's mostly optimizations?
-
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/376#issuecomment-164572673
+1, please commit :).
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/371#issuecomment-164589466
I'm going to land this, unless there are other review concerns. (Gancho
fixed a few number of issues that was reported outside of this PR already).
---
If
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/377#issuecomment-164888496
+1
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/386#issuecomment-165671592
Looks good to me, ship it!
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/392#issuecomment-166458670
Yeah, looks good.
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/386#issuecomment-166989854
James, the problem is that if you do what you suggest, it likely will fail.
It will still find that OpenSSL include file (because it's likely to be on the
s
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/386#issuecomment-167193001
Yeah, that's a good idea (putting all SSL library weirdness in one file).
---
If your project is set up for it, you can reply to this email and have your
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/393#issuecomment-168111940
@bgaff This seems useful that it ought to be more than just an example, no
? Do we have a strategy / plan for exposing stable plugins to the community,
which are
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/417
TS-4060 Adds JSON support consistently for traffic_layout
This makes all output of traffic_layout support the --json option.
You can merge this pull request into a Git repository by running
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/418
TS-4081 Adds --pristine to use pristine URL on escalation
This new options lets the escalation plugin use the pristine (unmapped) URL
when retrying the request.
You can merge this pull
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/427
TS-4140 Fixes a coverity warning due to changes in TS-4106 (dead code)
This restores an older behavior, removing an unnecessary conditional (dead
code).q can never be NUL..
You can merge this
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/428
Ts 4142
This fixes builds on OmniOS. I split this up into two commits, since not
everything can be cherry-picked to 6.1.x
You can merge this pull request into a Git repository by running
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/434#issuecomment-173747195
Looks good, I'd like to get this in for a 6.1.0 respin.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitH
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/422#issuecomment-173770153
Did we resolve the issues around the .snap file compatibility issues?
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/440#issuecomment-175908728
I don't think you can just change the ':' to a ';'. The ':' is what the
Via: header will have, which means, you'd break com
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/441#issuecomment-176227717
+1
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/444#issuecomment-176838316
I'm mostly ok with this, except for the default value. You should for now
set it to a value (likely 0?) that is compatible with existing 6.x code. Then
f
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/447#issuecomment-178057937
+1, lets get this back ported to 6.1.1 as well (please mark the Jira
accordingly). Although this might also need that other patch for the new
timeout, right? If so
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/443#issuecomment-178061382
I'm having a major deja vu here :). We had this discussion at a summit, at
LinkedIn, didn't we? :) Yeah, you have to be careful caching these, for th
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/450#issuecomment-178270734
+1.
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/453#issuecomment-179534623
+1. I've tested a variation of this patch, and it let me do the leak
detection.
---
If your project is set up for it, you can reply to this email and have
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/453#issuecomment-179536276
Maybe some unlikely() spread in here, since those checks for the shutdown
is very rarely succeeding?
---
If your project is set up for it, you can reply to this
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/456#issuecomment-180551999
+1. Btw, is that really how clang-format indented it?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-180669497
Please squash this down to one commit IMO.
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-180905142
One question: Can we be sure this doesn't break anything "backwards
compatible" ? I.e. someone using the plugin today, it should just work the s
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-180911692
Changing to TSDebug is fine, I don't recall why printf() was used, probably
laziness :) I'll try to test this in our setup next week, but I agree that th
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-181594755
Couple of things on that last patch:
1) You have to run clang-format over the code, I'm fairly certain we would
never indent like this:
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-181604662
Yeah, the problem with it (and pretty much all of STL) is that it has
hidden (and sometimes) unpredictable memory allocations behavior. I did some
tests recently
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/463#issuecomment-182969455
+1
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/464#issuecomment-182975244
I don't quite understand it, but it seems fine and correct, and if it shuts
up Stroustrup, +1!
---
If your project is set up for it, you can reply to this
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/466#issuecomment-183162557
+1
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/467#issuecomment-183171794
+1.
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/472#issuecomment-183391081
+1, I like it!
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/473#issuecomment-183420183
Looks good, but I think it has to be run through clang-format.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/473#issuecomment-183423695
Cool. One (stupid) question: You remove the call to markParentUp(result),
is that being done elsewhere in the existing code paths?
---
If your project is set up
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/473#issuecomment-183425676
Sounds good, +1.
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/475#issuecomment-183515538
+1. Although, nothing uses this, right? :)
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/474#issuecomment-183515622
+1
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/479#issuecomment-184509159
+1
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/486#issuecomment-185819952
+1
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/393#issuecomment-186362474
Nit picking, but instead of BUILD_WEBP_TRANSFORM_PLUGIN, wouldn't it make
more sense to just say something like BUILD_HAS_IMAGEMAGICCPP or some such
(loo
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/491
TS-4158 Adds appropriate releases of URL MLocs
The missing releases are benign, this is merely for correctness. There are
two misplaces printf()'s as well that I removed.
You can merge
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/346
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/497#issuecomment-187996374
Does any of these flags change with the choice of compiler? LLVM / clang?
Also, FORTIFY_SOURCE is good, we've had it trip in prod at least once (which is
a
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/498#issuecomment-188494372
+1
---
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
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/500
TS-4228 Adds better error handling in the synthetic checks
In traffic_manager, the thread that handles the request from
traffic_cop (via traffic_server) does not deal well with various
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/507#issuecomment-190922574
Love the docs, good idea :). About rr: why not just use the is_rr()
function consistently, it was added a while ago.
Also, we're having some pretty se
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/509#issuecomment-191003854
+1
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/488#issuecomment-191056670
The latest patch looks good to me, +1.
Meera: In addition, can you make another PR to fix the documentation for
the metrics you found that are marked
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/507#issuecomment-191333818
It's a regression between 5.x and 6.x, it crashes infrequently in
production. The "fix" I put in indicates there is some issues going on with t
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/488#issuecomment-191992703
Ah good, I must have missed it. 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
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/500
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/513#issuecomment-192697452
+1. Should this be a back port to 6.1.2?
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/510#issuecomment-192981540
If this breaks throttling , should we back port to v6.1.2?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/514#issuecomment-192982825
Back port candidate? I wonder too if this is something we have other Jiras
for, I remember seeing crashes related to tunneling before.
---
If your project is set
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/517
TS-2642 Adds a new operator %{NOW} to header_rewrite
This can be used to make time based decisions, such as denying
requests prior to a certain date. Or changing QOS settings based
on
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/496#issuecomment-193402183
+1, lets land this. Gancho, you feel this should be back ported to 6.1.x?
How serious / likely is this to trip up?
---
If your project is set up for it, you can
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/510#issuecomment-193404766
+1
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/520#issuecomment-193804585
+1. We're sure this isn't causing any backwards compatibility issues, right?
---
If your project is set up for it, you can reply to this email and have
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/521
TS-4207 Adds better checks to avoid HostDB crashes in 6.x
As much as I dislike this bandaid fix, it does prevent the crashes that
we've
been experiencing since upgrading to 6.1.x.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/521#issuecomment-195044366
Should I land this? I understand it might hide an underlying problem, but
a) it does work and b) it does make sense to check the rr, we do that in other
places too
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/521#issuecomment-195107210
Ok. So we'll leave 6.x unusable for now. :). I understand you primary
concern, that it hides possibly another problem. But your r->round_robin
argument
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/521
---
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
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/523#issuecomment-195614866
As per the IRC discussion, this seems unlikely to be our issue, since we
don't enable the hosts files support on our boxes. Also, probably should use
ink
1 - 100 of 578 matches
Mail list logo
