You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by PSUdaemon <gi...@git.apache.org> on 2015/12/08 20:38:43 UTC

[GitHub] trafficserver pull request: TS-4030: Allow parent selection to ign...

GitHub user PSUdaemon opened a pull request:

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

    TS-4030: Allow parent selection to ignore query string

    This allows you to drop the query string when doing the consistent hash in parent selection. Default behavior is the same. Add `qstring=ignore` to the parent.config line to enable.

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

    $ git pull https://github.com/PSUdaemon/trafficserver parent_selection_ignore_qsting

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

    https://github.com/apache/trafficserver/pull/369.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 #369
    
----
commit 1777e18a95f096230b8895faf184f9ac164a6b73
Author: Phil Sorber <so...@apache.org>
Date:   2015-11-19T17:32:55Z

    TS-4030: Allow parent selection to ignore query string

----


---
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-4030: Allow parent selection to ign...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:

    https://github.com/apache/trafficserver/pull/369#issuecomment-163998438
  
    @jrushf1239k I think this will land before #359, so you will have to rebase.


---
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-4030: Allow parent selection to ign...

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

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


---
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-4030: Allow parent selection to ign...

Posted by zwoop <gi...@git.apache.org>.
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 not 100% sure, that this effectively always strips the query parameter. My reasoning for this is that this ought to always just return the path (without the query params):
    
        path = const_cast<char *>(hrdata->hdr->path_get(&plen));
    
    
    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 pull request: TS-4030: Allow parent selection to ign...

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

    https://github.com/apache/trafficserver/pull/369#issuecomment-164002946
  
    Updated with url_len removed.


---
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-4030: Allow parent selection to ign...

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

    https://github.com/apache/trafficserver/pull/369#issuecomment-163997300
  
    There was a lot of redundant or plainly wrong code in there. Like we treated round_robin as a boolean and as an enum.
    
    Also a little bit of added NULL checking.
    
    It's really not that much of you pull out stuff that was for consistent hashing and using the path hash instead of the url.


---
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-4030: Allow parent selection to ign...

Posted by zwoop <gi...@git.apache.org>.
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?


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