You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by shinrich <gi...@git.apache.org> on 2016/06/29 03:03:46 UTC

[GitHub] trafficserver pull request #763: TS-4610: Fix HTTP and HTTP2 stats

GitHub user shinrich opened a pull request:

    https://github.com/apache/trafficserver/pull/763

    TS-4610: Fix HTTP and HTTP2 stats

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/shinrich/trafficserver ts-4610

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/trafficserver/pull/763.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 #763
    
----
commit 0443713c63651247d423bbb7b6a3cf56567fb54a
Author: Susan Hinrichs <sh...@ieee.org>
Date:   2016-06-29T02:58:42Z

    TS-4610: Fix HTTP and HTTP2 stats

----


---
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 issue #763: TS-4610: Fix HTTP and HTTP2 stats

Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the issue:

    https://github.com/apache/trafficserver/pull/763
  
    So, I tried this patch, and it does not fix the problem I'm running into, where proxy.process.http.current_active_client_connections is seemingly never decremented. I had a merge conflict, but I'm pretty sure I corrected it 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 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 issue #763: TS-4610: Fix HTTP and HTTP2 stats

Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the issue:

    https://github.com/apache/trafficserver/pull/763
  
    This might be above my Git skill level. I did try to pull the PR into my tree, but it's so far behind that it made some crazy changes / conflicts :-/. Manually applying the "diff" from this PR, and fix the conflicts was much more doable.


---
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 issue #763: TS-4610: Fix HTTP and HTTP2 stats

Posted by masaori335 <gi...@git.apache.org>.
Github user masaori335 commented on the issue:

    https://github.com/apache/trafficserver/pull/763
  
    @zwoop Looks good. We should land this to add a option to get metrics per protocol.


---
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 issue #763: TS-4610: Fix HTTP and HTTP2 stats

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/763
  
    FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/378/ for details.
     



---
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 issue #763: TS-4610: Fix HTTP and HTTP2 stats

Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the issue:

    https://github.com/apache/trafficserver/pull/763
  
    Some sort of odd race condition here maybe. If I run single requests using curl, I'm not seeing the problem. But if I blast it with "ab -c 10 -n 10000" the counter permanently bumps up by 10,000.


---
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 #763: TS-4610: Fix HTTP and HTTP2 stats

Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/763#discussion_r69666098
  
    --- Diff: proxy/http/HttpSM.cc ---
    @@ -200,7 +200,8 @@ HttpVCTable::cleanup_entry(HttpVCTableEntry *e)
         // Update stats
         switch (e->vc_type) {
         case HTTP_UA_VC:
    -      //              HTTP_DECREMENT_DYN_STAT(http_current_client_transactions_stat);
    +      // Moved to HttpSM::destroy
    +      // HTTP_DECREMENT_DYN_STAT(http_current_client_transactions_stat);
    --- End diff --
    
    I don't know if we need to keep these commented out, but maybe  just a comment saying "Metrics are updated in ::destroy".


---
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 issue #763: TS-4610: Fix HTTP and HTTP2 stats

Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the issue:

    https://github.com/apache/trafficserver/pull/763
  
    I resolved the conflicts, I think: http://paste.fedoraproject.org/389174/80381301/


---
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 issue #763: TS-4610: Fix HTTP and HTTP2 stats

Posted by PSUdaemon <gi...@git.apache.org>.
Github user PSUdaemon commented on the issue:

    https://github.com/apache/trafficserver/pull/763
  
    Yes, merge and mark what files you resolved conflicts in. Git normally puts this in the commented part of the commit message so just make sure you uncomment before 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 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 #763: TS-4610: Fix HTTP and HTTP2 stats

Posted by zwoop <gi...@git.apache.org>.
Github user zwoop closed the pull request at:

    https://github.com/apache/trafficserver/pull/763


---
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 issue #763: TS-4610: Fix HTTP and HTTP2 stats

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/763
  
    Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/274/ for details.
     



---
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 issue #763: TS-4610: Fix HTTP and HTTP2 stats

Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on the issue:

    https://github.com/apache/trafficserver/pull/763
  
    Yes, that fix does not address active client connection. It wasn't immediately clear to me what that meant for HTTP2. So I fixed the others but left active client connections alone.
    
    On Tue, Jul 05, 2016 at 8:05 PM, Leif Hedstrom < notifications@github.com [notifications@github.com] > wrote:
    So, I tried this patch, and it does not fix the problem I'm running into, where proxy.process.http.current_active_client_connections is seemingly never decremented. I had a merge conflict, but I'm pretty sure I corrected it right.
    
    \u2014
    You are receiving this because you authored the thread.
    Reply to this email directly, view it on GitHub [https://github.com/apache/trafficserver/pull/763#issuecomment-230653648] , or mute the thread [https://github.com/notifications/unsubscribe/AAUamPmpGbRMxNlOUZcZjuBqSbKrzhfzks5qSw2DgaJpZM4JAupl] .


---
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 issue #763: TS-4610: Fix HTTP and HTTP2 stats

Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the issue:

    https://github.com/apache/trafficserver/pull/763
  
    It does have merge conflicts, so maybe I should resolve that locally, and commit it with --author=susan ? @PSUdaemon  wdyt?


---
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 issue #763: TS-4610: Fix HTTP and HTTP2 stats

Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the issue:

    https://github.com/apache/trafficserver/pull/763
  
    @shinrich @masaori335  Should we land this patch though? I'm landing my other fix for the active client conns.


---
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.
---