You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by jacksontj <gi...@git.apache.org> on 2015/07/02 01:54:49 UTC

[GitHub] trafficserver pull request: Maintain and use a mapping of hostname...

GitHub user jacksontj opened a pull request:

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

    Maintain and use a mapping of hostname -> IP to implement host file support

    This allows the background thread to populate a map of overrides, which we simply inject in the lookup method-- in place of calling to DNS.
    
    I'm sure this implementation is riddled with holes, but it works in the most basic of cases. Pointers to get it cleaned up are much appreciated :)

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

    $ git pull https://github.com/jacksontj/trafficserver master

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

    https://github.com/apache/trafficserver/pull/240.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 #240
    
----
commit a29a9afd5c47b7b789f4b266e348770639e1fd9b
Author: Thomas Jackson <ja...@apache.org>
Date:   2015-07-01T23:52:55Z

    Maintain and use a mapping of hostname -> IP to implement host file support
    
    This allows the background thread to populate a map of overrides, which we simply inject in the lookup method-- in place of calling to DNS.

----


---
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: Maintain and use a mapping of hostname...

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

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


---
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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#discussion_r33820620
  
    --- Diff: iocore/hostdb/HostDB.cc ---
    @@ -659,6 +660,21 @@ db_mark_for(IpAddr const &ip)
     HostDBInfo *
     probe(ProxyMutex *mutex, HostDBMD5 const &md5, bool ignore_timeout)
     {
    +  // If we have an entry in our hosts file, we don't need to bother with DNS
    +  Ptr<RefCountedHostsFileMap> current_host_file_map = hostDB.hosts_file_ptr;
    +
    +  ts::ConstBuffer hname(md5.host_name, md5.host_len);
    +  std::_Rb_tree_iterator<std::pair<const ts::ConstBuffer, IpAddr> > find_result = current_host_file_map->hosts_file_map.find(hname);
    --- End diff --
    
    It's not a good idea to dig around in the internals of `std::map`. The official way to do this is `std::map<ts::ConstBuffer, IpAddr, CmpConstBufferCaseInsensitive>::iterator`.


---
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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#discussion_r33836997
  
    --- Diff: iocore/hostdb/HostDB.cc ---
    @@ -2833,15 +2650,8 @@ ParseHostFile(char const *path)
         }
       }
     
    -  if (!HostFilePairs.empty()) {
    -    // Need to sort by name so multiple address hosts are
    -    // contiguous.
    -    std::sort(HostFilePairs.begin(), HostFilePairs.end(), &CmpHostFilePair);
    -    HostDBFileContinuation::scheduleUpdate(0);
    -  } else if (!HostFileKeys.empty()) {
    -    HostDBFileContinuation::scheduleRemove(-1, 0);
    -  } else {
    -    // Nothing in new data, nothing in old data, just clean up.
    -    HostDBFileContinuation::finish(0);
    -  }
    +  // Swap out hostDB's map for ours
    +  hostDB.hosts_file_ptr = parsed_hosts_file_ptr;
    +  // Make a new map, so we can do it all again
    +  parsed_hosts_file_ptr = new RefCountedHostsFileMap();
    --- End diff --
    
    I had the same question when I saw that, and it's likely others will wonder the same thing looking at this code in the future. It might be clearer if you do it at the start rather than at the completion of parsing.


---
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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#discussion_r33810813
  
    --- Diff: iocore/hostdb/P_HostDBProcessor.h ---
    @@ -190,6 +190,14 @@ extern RecRawStatBlock *hostdb_rsb;
     #define HOSTDB_DECREMENT_THREAD_DYN_STAT(_s, _t) RecIncrRawStatSum(hostdb_rsb, _t, (int)_s, -1);
     
     
    +struct CmpStrCaseInsensitive {
    --- End diff --
    
    Would `ptr_len_casecmp


---
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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#discussion_r33824514
  
    --- Diff: iocore/hostdb/HostDB.cc ---
    @@ -2771,11 +2586,14 @@ ParseHostLine(char *l)
       int n_elts = elts.Initialize(l, SHARE_TOKS);
       // Elements should be the address then a list of host names.
       // Don't use RecHttpLoadIp because the address *must* be literal.
    -  HostFilePair item;
    -  if (n_elts > 1 && 0 == item.ip.load(elts[0]) && !item.ip.isLoopback()) {
    +  IpAddr ip;
    +  if (n_elts > 1 && 0 == ip.load(elts[0]) && !ip.isLoopback()) {
         for (int i = 1; i < n_elts; ++i) {
    -      item.name = elts[i];
    -      HostFilePairs.push_back(item);
    +      ts::ConstBuffer name(elts[i], strlen(elts[i]));
    +      // If we don't have an entry already (host files only support single IPs for a given name)
    --- End diff --
    
    Is this true? I've done quite a bit of research on this and can't verify that accuracy of that claim. I wrote the previous version on the presumption you could have round robin data for a host by using multiple lines with distinct IP addresses and the same FQDN.


---
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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#discussion_r33813578
  
    --- Diff: iocore/hostdb/HostDB.cc ---
    @@ -2833,15 +2650,8 @@ ParseHostFile(char const *path)
         }
       }
     
    -  if (!HostFilePairs.empty()) {
    -    // Need to sort by name so multiple address hosts are
    -    // contiguous.
    -    std::sort(HostFilePairs.begin(), HostFilePairs.end(), &CmpHostFilePair);
    -    HostDBFileContinuation::scheduleUpdate(0);
    -  } else if (!HostFileKeys.empty()) {
    -    HostDBFileContinuation::scheduleRemove(-1, 0);
    -  } else {
    -    // Nothing in new data, nothing in old data, just clean up.
    -    HostDBFileContinuation::finish(0);
    -  }
    +  // Swap out hostDB's map for ours
    +  hostDB.hosts_file_ptr = parsed_hosts_file_ptr;
    +  // Make a new map, so we can do it all again
    +  parsed_hosts_file_ptr = new RefCountedHostsFileMap();
    --- End diff --
    
    Shouldn't this be done before you put it in `HostDB.hosts_file_ptr`?


---
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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#issuecomment-117899443
  
    I reviewed this and iterated with @jacksontj and so obviously I give this a +1, @SolidWallOfCode can you please take a look at this latest version?


---
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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#discussion_r33837875
  
    --- Diff: iocore/hostdb/HostDB.cc ---
    @@ -2833,15 +2650,8 @@ ParseHostFile(char const *path)
         }
       }
     
    -  if (!HostFilePairs.empty()) {
    -    // Need to sort by name so multiple address hosts are
    -    // contiguous.
    -    std::sort(HostFilePairs.begin(), HostFilePairs.end(), &CmpHostFilePair);
    -    HostDBFileContinuation::scheduleUpdate(0);
    -  } else if (!HostFileKeys.empty()) {
    -    HostDBFileContinuation::scheduleRemove(-1, 0);
    -  } else {
    -    // Nothing in new data, nothing in old data, just clean up.
    -    HostDBFileContinuation::finish(0);
    -  }
    +  // Swap out hostDB's map for ours
    +  hostDB.hosts_file_ptr = parsed_hosts_file_ptr;
    +  // Make a new map, so we can do it all again
    +  parsed_hosts_file_ptr = new RefCountedHostsFileMap();
    --- End diff --
    
    Not to be a nuisance, but why is it more clear? The comment says exactly
    what its doing ;)
    
    On Thu, Jul 2, 2015 at 7:08 PM, Brian Geffon <no...@github.com>
    wrote:
    
    > In iocore/hostdb/HostDB.cc
    > <https://github.com/apache/trafficserver/pull/240#discussion_r33836997>:
    >
    > > @@ -2833,15 +2650,8 @@ ParseHostFile(char const *path)
    > >      }
    > >    }
    > >
    > > -  if (!HostFilePairs.empty()) {
    > > -    // Need to sort by name so multiple address hosts are
    > > -    // contiguous.
    > > -    std::sort(HostFilePairs.begin(), HostFilePairs.end(), &CmpHostFilePair);
    > > -    HostDBFileContinuation::scheduleUpdate(0);
    > > -  } else if (!HostFileKeys.empty()) {
    > > -    HostDBFileContinuation::scheduleRemove(-1, 0);
    > > -  } else {
    > > -    // Nothing in new data, nothing in old data, just clean up.
    > > -    HostDBFileContinuation::finish(0);
    > > -  }
    > > +  // Swap out hostDB's map for ours
    > > +  hostDB.hosts_file_ptr = parsed_hosts_file_ptr;
    > > +  // Make a new map, so we can do it all again
    > > +  parsed_hosts_file_ptr = new RefCountedHostsFileMap();
    >
    > I had the same question when I saw that, and it's likely others will
    > wonder the same thing looking at this code in the future. It might be
    > clearer if you do it at the start rather than at the completion of parsing.
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/trafficserver/pull/240/files#r33836997>.
    >



---
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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#discussion_r33810635
  
    --- Diff: iocore/hostdb/P_HostDBProcessor.h ---
    @@ -190,6 +190,14 @@ extern RecRawStatBlock *hostdb_rsb;
     #define HOSTDB_DECREMENT_THREAD_DYN_STAT(_s, _t) RecIncrRawStatSum(hostdb_rsb, _t, (int)_s, -1);
     
     
    +struct CmpStrCaseInsensitive {
    +  bool operator()(const std::string &a, const std::string &b) const { return strcasecmp(a.c_str(), b.c_str()) == 0; }
    +};
    +// A to hold a ref-counted map
    +struct RefCountedHostsFileMap : public RefCountObj {
    +  std::map<std::string, IpAddr, CmpStrCaseInsensitive> hosts_file_map;
    +};
    --- End diff --
    
    I'd do this differently. You have to read in the file. If you look at the original code that file buffer is kept for the lifetime of the use of the hosts names and the actual "strings" that are the host names are just pointers in to that buffer. This is a bit memory inefficient because you keep the address strings in memory but involves many fewer allocations because you don't have to copy or allocate for the strings and the memory management is easy since it's all one unit.
    
    In that case you 'd use `ts::ConstBuffer` as your string type and use an appropriate comparison functor.
    
    This also makes searching faster as you can use ts::ConstBuffer from the input name trivially and the lifetime of that is sufficient for the search. Look at PR 229, in `LogField.cc` around line 122, for an example that does exactly 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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#discussion_r33820528
  
    --- Diff: iocore/hostdb/P_HostDBProcessor.h ---
    @@ -190,6 +190,14 @@ extern RecRawStatBlock *hostdb_rsb;
     #define HOSTDB_DECREMENT_THREAD_DYN_STAT(_s, _t) RecIncrRawStatSum(hostdb_rsb, _t, (int)_s, -1);
     
     
    +struct CmpConstBuffferCaseInsensitive {
    +  bool operator()(ts::ConstBuffer a, ts::ConstBuffer b) { return ptr_len_casecmp(a._ptr, a._size, b._ptr, b._size) < 0;}
    +};
    +// A to hold a ref-counted map
    +struct RefCountedHostsFileMap : public RefCountObj {
    +  std::map<ts::ConstBuffer, IpAddr, CmpConstBuffferCaseInsensitive> hosts_file_map;
    --- End diff --
    
    You'll probably want `typedef std::map<ts::ConstBuffer, IpAddr, CmpConstBufferCaseInsensitive> MapType;' here. It's a handy typedef.


---
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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#discussion_r33817237
  
    --- Diff: iocore/hostdb/HostDB.cc ---
    @@ -2833,15 +2650,8 @@ ParseHostFile(char const *path)
         }
       }
     
    -  if (!HostFilePairs.empty()) {
    -    // Need to sort by name so multiple address hosts are
    -    // contiguous.
    -    std::sort(HostFilePairs.begin(), HostFilePairs.end(), &CmpHostFilePair);
    -    HostDBFileContinuation::scheduleUpdate(0);
    -  } else if (!HostFileKeys.empty()) {
    -    HostDBFileContinuation::scheduleRemove(-1, 0);
    -  } else {
    -    // Nothing in new data, nothing in old data, just clean up.
    -    HostDBFileContinuation::finish(0);
    -  }
    +  // Swap out hostDB's map for ours
    +  hostDB.hosts_file_ptr = parsed_hosts_file_ptr;
    +  // Make a new map, so we can do it all again
    +  parsed_hosts_file_ptr = new RefCountedHostsFileMap();
    --- End diff --
    
    I'm doing it so that the next load will have an empty one. Since the object is inited in the constructor, this seems like a sensible place to clear 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 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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#discussion_r33811976
  
    --- Diff: iocore/hostdb/HostDB.cc ---
    @@ -659,6 +660,20 @@ db_mark_for(IpAddr const &ip)
     HostDBInfo *
     probe(ProxyMutex *mutex, HostDBMD5 const &md5, bool ignore_timeout)
     {
    +  // If we have an entry in our hosts file, we don't need to bother with DNS
    +  Ptr<RefCountedHostsFileMap> current_host_file_map = hostDB.hosts_file_ptr;
    +
    +  std::string hname(md5.host_name);
    +  if (current_host_file_map->hosts_file_map.find(hname) != current_host_file_map->hosts_file_map.end()) {
    +    // TODO: Something to make this not local :/
    +    static HostDBInfo r;
    +    r.round_robin = false;
    +    r.reverse_dns = false;
    +    r.is_srv = false;
    +    ats_ip_set(r.ip(), current_host_file_map->hosts_file_map[hname]);
    --- End diff --
    
    Would that be better than worrying about concurrent access of said iterator?


---
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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#issuecomment-117899355
  
    @SolidWallOfCode This should fix your hosts file feature (TS-3725)


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

Re: [GitHub] trafficserver pull request: Maintain and use a mapping of hostname...

Posted by Brian Geffon <br...@gmail.com>.
I agree that this is the preferred pattern given this situation, the
message is very clear: this object is no longer in use and then when it's
assigned again at the start it's clear it's in use again.

On Mon, Jul 6, 2015 at 9:51 PM, SolidWallOfCode <gi...@git.apache.org> wrote:

> Github user SolidWallOfCode commented on a diff in the pull request:
>
>     https://github.com/apache/trafficserver/pull/240#discussion_r33935287
>
>     --- Diff: iocore/hostdb/HostDB.cc ---
>     @@ -2833,15 +2650,8 @@ ParseHostFile(char const *path)
>          }
>        }
>
>     -  if (!HostFilePairs.empty()) {
>     -    // Need to sort by name so multiple address hosts are
>     -    // contiguous.
>     -    std::sort(HostFilePairs.begin(), HostFilePairs.end(),
> &CmpHostFilePair);
>     -    HostDBFileContinuation::scheduleUpdate(0);
>     -  } else if (!HostFileKeys.empty()) {
>     -    HostDBFileContinuation::scheduleRemove(-1, 0);
>     -  } else {
>     -    // Nothing in new data, nothing in old data, just clean up.
>     -    HostDBFileContinuation::finish(0);
>     -  }
>     +  // Swap out hostDB's map for ours
>     +  hostDB.hosts_file_ptr = parsed_hosts_file_ptr;
>     +  // Make a new map, so we can do it all again
>     +  parsed_hosts_file_ptr = new RefCountedHostsFileMap();
>     --- End diff --
>
>     jacksont - I would have just assigned `NULL` to the pointer there and
> delayed the call to `new` until it was needed (at the start of the next
> load).
>
>
> ---
> 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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#discussion_r33935287
  
    --- Diff: iocore/hostdb/HostDB.cc ---
    @@ -2833,15 +2650,8 @@ ParseHostFile(char const *path)
         }
       }
     
    -  if (!HostFilePairs.empty()) {
    -    // Need to sort by name so multiple address hosts are
    -    // contiguous.
    -    std::sort(HostFilePairs.begin(), HostFilePairs.end(), &CmpHostFilePair);
    -    HostDBFileContinuation::scheduleUpdate(0);
    -  } else if (!HostFileKeys.empty()) {
    -    HostDBFileContinuation::scheduleRemove(-1, 0);
    -  } else {
    -    // Nothing in new data, nothing in old data, just clean up.
    -    HostDBFileContinuation::finish(0);
    -  }
    +  // Swap out hostDB's map for ours
    +  hostDB.hosts_file_ptr = parsed_hosts_file_ptr;
    +  // Make a new map, so we can do it all again
    +  parsed_hosts_file_ptr = new RefCountedHostsFileMap();
    --- End diff --
    
    jacksont - I would have just assigned `NULL` to the pointer there and delayed the call to `new` until it was needed (at the start of the next load).


---
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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#discussion_r33811033
  
    --- Diff: iocore/hostdb/HostDB.cc ---
    @@ -659,6 +660,20 @@ db_mark_for(IpAddr const &ip)
     HostDBInfo *
     probe(ProxyMutex *mutex, HostDBMD5 const &md5, bool ignore_timeout)
     {
    +  // If we have an entry in our hosts file, we don't need to bother with DNS
    +  Ptr<RefCountedHostsFileMap> current_host_file_map = hostDB.hosts_file_ptr;
    +
    +  std::string hname(md5.host_name);
    +  if (current_host_file_map->hosts_file_map.find(hname) != current_host_file_map->hosts_file_map.end()) {
    +    // TODO: Something to make this not local :/
    +    static HostDBInfo r;
    +    r.round_robin = false;
    +    r.reverse_dns = false;
    +    r.is_srv = false;
    +    ats_ip_set(r.ip(), current_host_file_map->hosts_file_map[hname]);
    --- End diff --
    
    No, save the iterator from the `find` so you don't have to search again.


---
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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#discussion_r33831438
  
    --- Diff: iocore/hostdb/HostDB.cc ---
    @@ -2771,11 +2586,14 @@ ParseHostLine(char *l)
       int n_elts = elts.Initialize(l, SHARE_TOKS);
       // Elements should be the address then a list of host names.
       // Don't use RecHttpLoadIp because the address *must* be literal.
    -  HostFilePair item;
    -  if (n_elts > 1 && 0 == item.ip.load(elts[0]) && !item.ip.isLoopback()) {
    +  IpAddr ip;
    +  if (n_elts > 1 && 0 == ip.load(elts[0]) && !ip.isLoopback()) {
         for (int i = 1; i < n_elts; ++i) {
    -      item.name = elts[i];
    -      HostFilePairs.push_back(item);
    +      ts::ConstBuffer name(elts[i], strlen(elts[i]));
    +      // If we don't have an entry already (host files only support single IPs for a given name)
    --- End diff --
    
    It is from my local testing. I also checked around on google and there are quite a few resources to back me up:
    
    - http://serverfault.com/questions/429839/assign-multiple-ips-to-1-entry-in-hosts-file
    - http://serverfault.com/questions/69836/point-multiple-ip-addresses-to-a-single-host-name
    - http://askubuntu.com/questions/48941/is-it-possible-to-give-two-ip-addresses-for-the-same-host-name


---
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: Maintain and use a mapping of hostname...

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

    https://github.com/apache/trafficserver/pull/240#discussion_r33820816
  
    --- Diff: iocore/hostdb/HostDB.cc ---
    @@ -2565,18 +2568,9 @@ int HostDBFileUpdateActive = 0;
     // Contents of the host file. We keep this around because other data
     // points in to it (to minimize allocations).
     ats_scoped_str HostFileText;
    --- End diff --
    
    This needs to be moved to the host file map, as it needs to have *exactly* the same life time as that map (as all of the `ConstBuffer` items are pointing in to this buffer).


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