TS-4531: Clarify time unit confusion in HostDB sync interval. #704
Conversation
|
Linux build failed! See https://ci.trafficserver.apache.org/job/Github-Linux/124/ for details. |
Commit d48b76e tried to fix some of the time unit conversions in HostBD, but didn't notice that RefCountedHostsFileMap::next_sync_time was getting initialized to a nanosecond timestamp + an interval in seconds. This clarifies most of the timestamps uses in hosts file update checking, which are all in Unix epoch seconds. We remove the HOST_DB_TIMEOUT_INTERVAL definition since that interval is not really a changeable (all the code assumes it is 1 sec).
|
FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/227/ for details. |
|
Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/125/ for details. |
|
FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/228/ for details. |
|
I looked at the code and I am not of the opinion the code assumes |
| ++hostdb_current_interval; | ||
| // No nothing if hosts file checking is not enabled. | ||
| if (hostdb_hostfile_check_interval == 0) { | ||
| return EVENT_CONT; |
There was a problem hiding this comment.
Shouldn't this be EVENT_DONE? Because it's done, there is no continuing operation.
There was a problem hiding this comment.
The previous logic was to return EVENT_CONT. The background event is written to trigger every second but we only check the actual changes after the check interval. I assumed that it was done this way so that you can update the check interval without having to wait for it to expire.
|
|
Right, the interval is presumed to be seconds, which is why |
|
Overall it seems fine to get rid of it. |
|
Oh, I see. I thought that |
|
@AMC Sounds like you are OK this this? If so I'll merge tomorrow. |
Making restricted_api false by default will make ATS enforce the permissions rather than the DAC of the socket. ATS will restrict access to privileged users for write operations while allowing broader access for read-only options. This default should be reasonable since that is how 9.0 used to behave.
Commit d48b76e tried to fix some of the time unit conversions in
HostBD, but didn't notice that RefCountedHostsFileMap::next_sync_time
was getting initialized to a nanosecond timestamp + an interval in
seconds.
This clarifies most of the timestamps uses in hosts file update
checking, which are all in Unix epoch seconds. We remove the
HOST_DB_TIMEOUT_INTERVAL definition since that interval is not
really a changeable (all the code assumes it is 1 sec).