You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2020/07/07 20:25:17 UTC

[GitHub] [trafficserver] joelanders opened a new pull request #6979: hostdb: don't use next_sync_time - now() as TTL (it can be negative)

joelanders opened a new pull request #6979:
URL: https://github.com/apache/trafficserver/pull/6979


   `next_sync_time` gets set to `ink_time() + hostdb_hostfile_check_interval` when a host file is actually loaded and parsed.
   
   The host file is `stat()`ed every `hostdb_hostfile_check_interval`, and only if has been updated, is it loaded and parsed.
   
   So, if the host file is untouched for a while, `next_sync_time` is actually in the past.
   
   It's only actually used here:
   ```cpp
           HostDBInfo *r = lookup_done(IpAddr(find_result->second), hash.host_name, false,
                                       current_host_file_map->next_sync_time - ink_time(), nullptr);
   ```
   where it's inserting something into the HostDB with a potentially negative TTL. (but actually the TTL is treated as unsigned, so I was seeing a quite large TTL.)
   
   I think you probably want to replace that TTL with something like `hostdb_hostfile_check_interval` or maybe something fancier involving the last update time.
   
   But I've had a lot of coffee so please correct me if I'm wrong :)
   I was definitely seeing debug lines like:
   ```
   [Jul  7 21:20:15.191] Server {0x2b3aa8614700} DEBUG: <HostDB.cc:1122 (lookup_done)> (hostdb) done 127.0.0.1 TTL -6
   ```
   (that line number is from a different branch)
   
   Related question: is there a way to something like a config reload signal for all the hosts in the host file?
   I'm going to try to work around that by setting some small timeouts in the following:
   ```
   CONFIG proxy.config.hostdb.host_file.interval INT 10  # stat() the host file to see if it's been touched
   CONFIG proxy.config.cache.hostdb.sync_frequency INT 0  # don't persist hostdb to disk between restarts
   CONFIG proxy.config.hostdb.verify_after INT 10  #  refresh the cache from the last-read host file contents after this long
   ```
   
   Thank you!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] shinrich commented on pull request #6979: hostdb: don't use next_sync_time - now() as TTL (it can be negative)

Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #6979:
URL: https://github.com/apache/trafficserver/pull/6979#issuecomment-656712995


   I see there is a --clear-hostdb option for traffic_ctl server start/restart.  Seem like it could be added to config reload as well.  New PR of course. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] randall removed a comment on pull request #6979: hostdb: don't use next_sync_time - now() as TTL (it can be negative)

Posted by GitBox <gi...@apache.org>.
randall removed a comment on pull request #6979:
URL: https://github.com/apache/trafficserver/pull/6979#issuecomment-655649050






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] shinrich commented on pull request #6979: hostdb: don't use next_sync_time - now() as TTL (it can be negative)

Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #6979:
URL: https://github.com/apache/trafficserver/pull/6979#issuecomment-656404835


   Looking back into the history of this feature, it appears that the /etc/hosts info was stored in a separate table which had an effectively infinite TTL (or until the next time the hosts file was read).  So changing the TTL to infinite or update interval seems fine to me.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] joelanders commented on pull request #6979: hostdb: don't use next_sync_time - now() as TTL (it can be negative)

Posted by GitBox <gi...@apache.org>.
joelanders commented on pull request #6979:
URL: https://github.com/apache/trafficserver/pull/6979#issuecomment-656645152


   I guess in our use-case, the most convenient thing would be if there was a way to invalidate and reload the stuff loaded from the hosts file at the same time that we do a `traffic_ctl config reload`. Being able to configure a shortish TTL is ok, too :)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] randall commented on pull request #6979: hostdb: don't use next_sync_time - now() as TTL (it can be negative)

Posted by GitBox <gi...@apache.org>.
randall commented on pull request #6979:
URL: https://github.com/apache/trafficserver/pull/6979#issuecomment-655649050


   [approve ci]
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] randall commented on pull request #6979: hostdb: don't use next_sync_time - now() as TTL (it can be negative)

Posted by GitBox <gi...@apache.org>.
randall commented on pull request #6979:
URL: https://github.com/apache/trafficserver/pull/6979#issuecomment-655176156


   [approve ci]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] randall commented on pull request #6979: hostdb: don't use next_sync_time - now() as TTL (it can be negative)

Posted by GitBox <gi...@apache.org>.
randall commented on pull request #6979:
URL: https://github.com/apache/trafficserver/pull/6979#issuecomment-655728928


   [approve ci autest]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] shinrich merged pull request #6979: hostdb: don't use next_sync_time - now() as TTL (it can be negative)

Posted by GitBox <gi...@apache.org>.
shinrich merged pull request #6979:
URL: https://github.com/apache/trafficserver/pull/6979


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org