You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by ericcarlschwartz <gi...@git.apache.org> on 2016/04/23 01:14:26 UTC
[GitHub] trafficserver pull request: [YTSATS-676] Add log field for Server ...
GitHub user ericcarlschwartz opened a pull request:
https://github.com/apache/trafficserver/pull/598
[YTSATS-676] Add log field for Server Connection Count
https://issues.apache.org/jira/browse/TS-4379
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ericcarlschwartz/trafficserver ts-4379
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/598.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #598
----
commit 5e71bf8465cc17532f147befbe284c0b00c29d6e
Author: Eric Schwartz <es...@unknown-192-168-202-141.yahoo.com>
Date: 2015-11-30T23:39:25Z
[YTSATS-676] Add log field for Server Connection 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 does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-219191868
Can one of the admins verify this patch? Only approve PRs which have been reviewed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [YTSATS-676] Add log field for Server ...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-213629092
Please fix the commit message to refer to TS-4379. The commit message should also contain some more information, take a look [this](http://chris.beams.io/posts/git-commit/).
The logic here needs to be aligned with #554, please sync up with @jacksontj and @bgaff.
Has this been benchmarked? ``ConnectionCount::getCount()`` effectively takes a global lock, so I would expect some performance impact.
Are logs really the right way to expose this information? Maybe it would be better to be able to extract more general statistics about how Traffic Server is utilizing origins?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-214452613
@bryancall My main point here is that putting this in the access logs isn't the right place to put it as the transactions have little to do with the connections. I also want this metric, but the only way to be even close to correct is to do one of the 2 options I mentioned above (ATS metric, or http UI interface thing).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-214463886
@jacksontj the way we are using it on the log entries is as a boolean (zero or non-zero) to understand whether that request was able to reuse an existing origin connection. We are doing log analysis at various levels (requesting clients, domains, etc) and merging up attributes.
@jpeach I think I had done some performance analysis on this a while back, and it didn't seem to have a significant impact. I'll repeat that analysis and provide more concrete impact #'s.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-214451815
Gotcha. Maybe that should be a regular metric instead then? Per-origin metric? Although, that has to be configurable to turn off, because it could explode in your face (e.g. if you run as a forward proxy).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-220376302
.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-214471698
I agree with @jacksontj , having an API here makes sense. Then you defer any penalty cost to the actual usage of such APIs as well (so no cost when not used).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [YTSATS-676] Add log field for Server ...
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-214429372
Yeah. We could look at something similar to the milestone work, a generic tag that lets you pull out a metric.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by bryancall <gi...@git.apache.org>.
Github user bryancall commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-214469357
@jacksontj Having it part of the existing metrics would mean you would have a metric per origin. We went down that road before and created a stats v2 which was a mess.
A better design might be to have framework for having metrics in the server connection pool and have a management API to query this information. Also, it can be logged like @zwoop mentioned above.
The number of requests on the connection could be another metric that would be interesting.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [YTSATS-676] Add log field for Server ...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-214427434
I think I agree with Thomas. This sets a bit of a bad precedence, treating one metric as special over all other metrics by also giving it a log tag. I'd much prefer something more generic, at least long term, such that all metrics could also be logged using a generic tag. E.g.
%<{proxy.process.http.total_server_connections}met>
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by bryancall <gi...@git.apache.org>.
Github user bryancall commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-214452145
It is configurable to turn it off and on.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-219194917
Can one of the admins verify this patch? Only approve PRs which have been reviewed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [YTSATS-676] Add log field for Server ...
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-214360346
IMO logging the connection count isn't the right way to go-- as the connections (number of them and lifecycle) are independent of the actual transactions that go across them (separate timers, min_origin_connections, etc.). As far as logging is concerned we already have a field for number of requests on the connection-- which is the best we can do (as thats the only overlap between the two).
I'm not sure how we want to expose this metric, but I'm thinking we either add it to the regular stats interface (my concern being duplicating the metric), or make something like the web interfaces for connection counts (http API to get the table dumped directly), thoughts?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-218782575
Can one of the admins verify this patch? Only approve PRs which have been reviewed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-214471034
Right, I think the correct way was my option #2-- which would be to have an API for getting this info (basically another mgmt API endpoint like hostdb, stats, etc.).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [YTSATS-676] Add log field for Server ...
Posted by bryancall <gi...@git.apache.org>.
Github user bryancall commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-214451166
@zwoop This is a value per origin, not a single number that can be represented in the metrics/stats.
I agree, having something that can generically log stats would be helpful, but it couldn't be used in this case.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-218525630
Can one of the admins verify this patch?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by JamesPeach <gi...@git.apache.org>.
Github user JamesPeach commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-214463141
@bryancall AFAICT ``server_connection_count`` is always populated, which is hitting a global lock. If there's no consensus that this is the right approach then I would prefer to not merge 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 this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-219186494
Can one of the admins verify this patch? Only approve PRs which have been reviewed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by ericcarlschwartz <gi...@git.apache.org>.
Github user ericcarlschwartz closed the pull request at:
https://github.com/apache/trafficserver/pull/598
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-219190142
Can one of the admins verify this patch? Only approve PRs which have been reviewed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by JamesPeach <gi...@git.apache.org>.
Github user JamesPeach commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-214464758
A log field to mark whether a server session was re-used seems helpful, but a very different patch from what is being proposed here.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: [TS-4379] Add log field for Server Con...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/598#issuecomment-214452495
Right, I meant, maybe it should be an option to enable these per origin metrics in the core. At which point, the generic "metric" log tag would work, right?
But, I don't want to stand in the way of progress here. I'm ok with landing this, but we should file appropriate Jira's such that we can do these metrics and log tags generically. And then deprecate these features here.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---