Github user sudheerv commented on the issue:
https://github.com/apache/trafficserver/pull/744
looks good to me :)
+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 sudheerv commented on the issue:
https://github.com/apache/trafficserver/pull/685
ð
---
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
Github user sudheerv commented on the issue:
https://github.com/apache/trafficserver/pull/685
@shukitchan : is the "deleted" state protecting against multiple threads
calling the cleanup? If so, it may still not be enough, without a mutex of some
sort?
---
If your project
Github user sudheerv closed the pull request at:
https://github.com/apache/trafficserver/pull/645
---
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
GitHub user sudheerv opened a pull request:
https://github.com/apache/trafficserver/pull/645
[TS-4452] Fix the type for open_write_fail_action to MgmtByte.
@zwoop pls review
You can merge this pull request into a Git repository by running:
$ git pull https://github.com
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-219075656
Just fyi -
https://mail-archives.apache.org/mod_mbox/trafficserver-dev/201507.mbox/%3CA16410D3-9E7B-4042-A819-F361BFD5002C%40apache.org%3E
---
If
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-218950857
Are we now okay with letting the use of STL in the core?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/609#issuecomment-215995836
In the HTTP equivalent, returning stale data on error is not
stale-while-revalidate (SWR); it is a somewhat related, but, independent
feature called stale-if
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/554#issuecomment-214694979
Oh, in any case, I haven't really checked carefully, but I actually wonder
if the milestone (server begin write) in question here is reset for "each&qu
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/554#issuecomment-214693606
The problem is that you are trying to "mix" the internals of a completely
independent/different feature to use for an unrelated/unintended purposes. Th
Github user sudheerv closed the pull request at:
https://github.com/apache/trafficserver/pull/573
---
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
GitHub user sudheerv opened a pull request:
https://github.com/apache/trafficserver/pull/573
Dummy commit to test github email.
Dummy commit to test email address for github
You can merge this pull request into a Git repository by running:
$ git pull https://github.com
Github user sudheerv closed the pull request at:
https://github.com/apache/trafficserver/pull/567
---
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
GitHub user sudheerv opened a pull request:
https://github.com/apache/trafficserver/pull/567
[TS-3857] Change ERROR default to not log into syslogs.
@zwoop pls review
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sudheerv
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/556#issuecomment-207486646
Wait, I thought Spdy was the only odd case that wasn't using iobufs already
(due to the history of starting it out as a plugin and merging with the core
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/549#issuecomment-205342076
:+1: from me
---
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 sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/508#issuecomment-190946514
:+1:
Thanks @shinrich! didn't realize it was not compiling. the crash seems odd,
i don't see that happening in my testing, but, your fix
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/505#issuecomment-190326183
Thanks, @jpeach : updated with the comments and squashed the commits.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/505#issuecomment-190321453
@jpeach: clang-formatted the source file. Was planning to add docs (will
need to spend some time) in a separate commit after the plugin gets in. Would
you
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/505#issuecomment-190316226
@zwoop : could you pls review?
---
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 sudheerv opened a pull request:
https://github.com/apache/trafficserver/pull/505
[TS-4243] Collapsed Forwarding Plugin based on Open Write Fail Actionâ¦
⦠feature.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com
GitHub user sudheerv opened a pull request:
https://github.com/apache/trafficserver/pull/495
[TS-4222] Add check to see if SSLConfigParams::load_ssl_file_cb is inâ¦
â¦itialized.
@jpeach : pls review
You can merge this pull request into a Git repository by running
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/491#issuecomment-187231165
sorry for coming late to the party, but, lgtm as well, can't believe the
printfs :)
---
If your project is set up for it, you can reply to this email and
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/443#issuecomment-178572302
Yeah, I thought about that, but, once you pull the handles you'd need to
keep track of it, and reset/reinit them subsequently anyway. So, I tried to
ke
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/443#issuecomment-178570076
Yeah, that was the case before this PR, but, the handles are now
reinitialized at each hook (along with the URL and header objects), no?
---
If your project is
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/443#issuecomment-178564856
Does it not affect the plugins that use API such as Request::getUrl() or
Request::getHeaders() etc which rely on the cached handles at the moment? OR
Are you
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/443#issuecomment-178064049
Agree - I don't particularly like caching these either, given that there's
no guarantee offered (by the TS API) on the scope of these (the C plugins
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/443#issuecomment-177752158
Bryan, the problem is that a lot of these handles are destroyed/recreated
at various points in the core. Resetting them at each possible hook allows to
have the
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/442#issuecomment-177043261
:+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 sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/442#issuecomment-177007742
Would it be better to move the reset() into the getter() inside the
operator()?
---
If your project is set up for it, you can reply to this email and have your
Github user sudheerv commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/442#discussion_r51277409
--- Diff: lib/atscppapi/src/Transaction.cc ---
@@ -433,14 +411,12 @@ class initializeHandles
initializeHandles(GetterFunction getter) : getter_
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/443#issuecomment-176523513
@bgaff can you pls merge this and #442 together, once you review.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/442#issuecomment-176516828
:+1: but, need to go along with
https://github.com/apache/trafficserver/pull/443
---
If your project is set up for it, you can reply to this email and have
GitHub user sudheerv opened a pull request:
https://github.com/apache/trafficserver/pull/443
[TS-4160] Reset the txn request/response handles at each hook, since â¦
â¦core may destroy them.
@bgaff @myraid @boaz-r : please review
You can merge this pull request into a Git
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/442#issuecomment-176504036
please review https://github.com/apache/trafficserver/pull/443
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/442#issuecomment-176500044
This patch does not solve the issue completely (for example, the issues
raised in https://issues.apache.org/jira/browse/TS-4160). You will also need to
reset
GitHub user sudheerv opened a pull request:
https://github.com/apache/trafficserver/pull/429
[TS-3881] Fix inconsistent return value and method name.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/trafficserver ts3881
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/351#issuecomment-164477764
Cool, 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 sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/351#issuecomment-164477130
@bgaff - Thanks, I'd have to double check though that the change made
automatically handles a H2/SPDY upload. There's no Content-Length, nor even a
TE
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/350#issuecomment-161377617
Uri - I agree with your point in general as well, but, I think it's
arguable whether to treat this as a *feature* that isn't needed in the core. I
can
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/351#issuecomment-160778560
+1 on the approach.
A specific comment on a quick scan of the code - it seems like the patch
only handles the case with *Content-Length* header, what
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/350#issuecomment-159640428
We can use the PR for voting? If an email is needed, perhaps, @shinrich
might send a vote request, given that she did the current patch.
In any case, my
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/346#issuecomment-158543436
lgtm
---
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 sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/310#issuecomment-150595466
Hmm..the requirement in the jira certainly seems reasonable, but, the
current state of this is mainly to ensure ATS is not caching broken responses
(connection
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/298#issuecomment-142013274
lgtm.
---
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 sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/297#issuecomment-141528147
Ah, I see - you are basically suggesting to rename Range to @Range
temporarily and re-add @Range as Range. Of the top of my head, I don't know if
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/297#issuecomment-141525465
@jpeach : I agree - but, one issue I could see in renaming Range to @Range,
is that, it's not transparent to custom logging configuration (the operators
ne
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/297#issuecomment-141509823
thanks, @jrushf1239k - merging 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
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/297#issuecomment-141507617
@jrushf1239k : lgtm - pls run clang format on the patch, so, I can merge it.
---
If your project is set up for it, you can reply to this email and have your
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/284#issuecomment-135915883
lgtm
---
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 sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/253#issuecomment-135914324
Patch applied to master.
---
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
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/280#issuecomment-134423458
Some questions/comments:
1) What happens when one or more disks fail during run time? The setting
name *proxy.config.http.cache.required* seems too confusing
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/270#issuecomment-130071743
@meeramn : I agree with the comments that this change seems like something
that can (and should?) be done in a plugin rather than in the core. It should
be
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/271#issuecomment-128529543
Thanks @zizhong - that's nice to hear. Needless to say, I'm :+1: on the
patch :)
---
If your project is set up for it, you can reply to this emai
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/271#issuecomment-128454326
I've a small concern/question on the patch.
the RFC for HEAD method says:
"The server SHOULD send the same header fields in response
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/262#issuecomment-125255226
As long as it's a WARNING (and doesn't block TS from coming up), I'm fine
with that as well.
OTOH if a crash/abort is involved, i
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/262#issuecomment-125251555
@bgaff : Also, pls see my comment above - we do have some use cases, where
there's no origin communication involved.
---
If your project is set up for it
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/262#issuecomment-125249706
@bgaff : How does this work when I have a set up that terminates all
connections locally (i.e. no origins to talk to)? We do have some use cases (w/
and w/o
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/254#issuecomment-122609479
Agree with @ushachar -
Transaction and Session/connection are not interchangeable (at least, not
how I see it). Keep-Alive is a *transaction* level
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/254#issuecomment-122599209
+1 to @jpeach 's point - SSL Hostname verification should be associated
with an origin and not per remap/transaction.
---
If your project is set up for it
Github user sudheerv commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/249#discussion_r34520227
--- Diff: proxy/http/HttpSM.cc ---
@@ -6560,6 +6559,12 @@ HttpSM::kill_this()
reentrancy_count--;
ink_release_assert(reentrancy_count
Github user sudheerv commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/249#discussion_r34511471
--- Diff: proxy/http/HttpSM.cc ---
@@ -6560,6 +6559,12 @@ HttpSM::kill_this()
reentrancy_count--;
ink_release_assert(reentrancy_count
Github user sudheerv commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/249#discussion_r34510052
--- Diff: proxy/http/HttpSM.cc ---
@@ -6560,6 +6559,12 @@ HttpSM::kill_this()
reentrancy_count--;
ink_release_assert(reentrancy_count
Github user sudheerv closed the pull request at:
https://github.com/apache/trafficserver/pull/237
---
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
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/237#issuecomment-117333885
The patch handles both cases - non-zero bytes received in SSL_read
immediately after handshake and no bytes. Unless there are specific comments,
I'm goi
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/237#issuecomment-117284351
No - I don't have the setup for that. I've tested it on y! prod 5.0.x.
For EOS, it's both.
---
If your project is set up for it, you ca
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/237#issuecomment-117221177
The problem with that approach is that, if there was an EOS on the fd right
after handshake (which I noticed seems to happen quite a bit), it is not caught
GitHub user sudheerv opened a pull request:
https://github.com/apache/trafficserver/pull/237
[TS-3714]: Summary of changes below:
a) Issue a SSL_read right after SSL handshake to ensure
data already in the SSL buffers is not lost.
b) Add vc to net thread's read_ready
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/215#issuecomment-112150121
This is not a real UA we are talking about. It's a plugin IIUC.
If you are talking about real UA, fyi, what ATS does with internal redirect
i
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/216#issuecomment-111531431
@ffcai: I'm a little concerned about this change - this would mean that
requests that would otherwise return an error would always return a "100 CONT&q
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/216#issuecomment-57498
@ffcai : I think you should delay the sending of "100 CONT" to at least
until after calling *is_request_valid()*, which occurs in
*HttpTransact::Han
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/216#issuecomment-110778191
IIRC, the solution to TS-1125 was to check for some request headers (e.g
*Content-Length* > 0) before sending the dummy "100 CONT". I don't
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/216#issuecomment-110775146
I need to review this more - Shouldn't the patch *at least* check that the
method is *POST*?
---
If your project is set up for it, you can reply to this
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/216#issuecomment-110774240
@ffcai is right about how the config works.
Basically, when enabled, ATS would send a dummy "100 CONT" to the clients on
receiving POST requests.
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/215#issuecomment-110417011
Can you ask the customer/user not to use redirect with POST (with the
supporting material from above) I believe I've done that on more than one
occasion i
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/215#issuecomment-110415828
AFAIK, we have always maintained (within our prod at least) that the
behavior of redirect follow w/ POST is not guaranteed. As such, the current
code of not
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/215#issuecomment-110394004
Do we claim to support redirection for POST? POST is not an idempotent
method and AFAIK, it is highly unreliable and not recommended to use
redirection with
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/206#issuecomment-107653612
I don't think there was a consensus yet (at least, not from my side).
AFAIK, my question above on persisting the source as traffic_line was not
addresse
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/206#issuecomment-107253803
A config set via traffic_line gets added to records.config as well, so the
source of the config changes after a TS restart. Depending on how that info is
used
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/194#issuecomment-105570499
Yes, but, you would need to know/use the cert to do that, which is not
always straightforward - as I said, "the problem with existing tools is that
they ca
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/198#issuecomment-101064719
It seems more consistent with the general TXN usage (also refer
https://issues.apache.org/jira/browse/TS-3450) and shorter to type too :=)
---
If your project
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/198#issuecomment-101064001
Should the name be *TXN-COUNT*?
---
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 sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/194#issuecomment-97845539
I agree with @jpeach on the TCP_INFO usage. Are the TCP_INFO traces even
part of the wire trace feature? Besides being non-portable, isn't there
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/162#issuecomment-69951456
hmm, can it not be used as a bit-map for other kinds of transparency?
---
If your project is set up for it, you can reply to this email and have your
reply
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/162#issuecomment-69947824
Isn't there a "NetVConnection::is_transparent" already in the netvc?
---
If your project is set up for it, you can reply to this email and have yo
85 matches
Mail list logo
