{"action":"created","issue":{"url":"https://api.github.com/repos/Metaswitch/cpp-common/issues/231","labels_url":"https://api.github.com/repos/Metaswitch/cpp-common/issues/231/labels{/name}","comments_url":"https://api.github.com/repos/Metaswitch/cpp-common/issues/231/comments","events_url":"https://api.github.com/repos/Metaswitch/cpp-common/issues/231/events","html_url":"https://github.com/Metaswitch/cpp-common/pull/231","id":54636057,"number":231,"title":"[Reviewer: Andy] Preserve old DNS records if the server doesn't respond","user":{"login":"rkd-msw","id":4363492,"avatar_url":"https://avatars.githubusercontent.com/u/4363492?v=3","gravatar_id":"","url":"https://api.github.com/users/rkd-msw","html_url":"https://github.com/rkd-msw","followers_url":"https://api.github.com/users/rkd-msw/followers","following_url":"https://api.github.com/users/rkd-msw/following{/other_user}","gists_url":"https://api.github.com/users/rkd-msw/gists{/gist_id}","starred_url":"https://api.github.com/users/rkd-msw/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rkd-msw/subscriptions","organizations_url":"https://api.github.com/users/rkd-msw/orgs","repos_url":"https://api.github.com/users/rkd-msw/repos","events_url":"https://api.github.com/users/rkd-msw/events{/privacy}","received_events_url":"https://api.github.com/users/rkd-msw/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"rkd-msw","id":4363492,"avatar_url":"https://avatars.githubusercontent.com/u/4363492?v=3","gravatar_id":"","url":"https://api.github.com/users/rkd-msw","html_url":"https://github.com/rkd-msw","followers_url":"https://api.github.com/users/rkd-msw/followers","following_url":"https://api.github.com/users/rkd-msw/following{/other_user}","gists_url":"https://api.github.com/users/rkd-msw/gists{/gist_id}","starred_url":"https://api.github.com/users/rkd-msw/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rkd-msw/subscriptions","organizations_url":"https://api.github.com/users/rkd-msw/orgs","repos_url":"https://api.github.com/users/rkd-msw/repos","events_url":"https://api.github.com/users/rkd-msw/events{/privacy}","received_events_url":"https://api.github.com/users/rkd-msw/received_events","type":"User","site_admin":false},"milestone":null,"comments":3,"created_at":"2015-01-16T23:03:21Z","updated_at":"2015-02-16T17:39:31Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/Metaswitch/cpp-common/pulls/231","html_url":"https://github.com/Metaswitch/cpp-common/pull/231","diff_url":"https://github.com/Metaswitch/cpp-common/pull/231.diff","patch_url":"https://github.com/Metaswitch/cpp-common/pull/231.patch"},"body":"We discussed earlier how the existing code probably doesn't behave how it's meant to (because records expire on schedule, so by the time we've realised that the DNS server hasn't responded, we've deleted the old records). This should improve that.\r\n\r\nThe basic change is that we add 5 minutes to the expiry time when working out when to free records, so there's a period where it has expired (and we'll re-query the DNS server) but is still in memory (and so we can re-use it if the server is down).\r\n\r\nI also spotted that the existing code sets `ce->expires = 30` but I think it should be an epoch timestamp - I added a check to `add_to_expiry_list` to ensure this.\r\n\r\nI haven't tested this yet (or even compiled it), but wanted to get it out for review to check the approach. In particular, this is meant to preserve existing records indefinitely if the server stays down - do you think that's right?\r\n\r\nI'll test by:\r\n* checking that my new warning isn't triggered during normal operation\r\n* checking that DNS records are re-queried and updated during normal operation\r\n* checking that if a DNS server goes down, the DNS record is preserved in the cache\r\n* checking that if the server comes back with a different DNS record, that's picked up\r\n* checking that NXDOMAIN responses don't preserve the existing cached record\r\n* checking that if a record doesn't exist at start-of-day, everything still works"},"comment":{"url":"https://api.github.com/repos/Metaswitch/cpp-common/issues/comments/74545753","html_url":"https://github.com/Metaswitch/cpp-common/pull/231#issuecomment-74545753","issue_url":"https://api.github.com/repos/Metaswitch/cpp-common/issues/231","id":74545753,"user":{"login":"rkd-msw","id":4363492,"avatar_url":"https://avatars.githubusercontent.com/u/4363492?v=3","gravatar_id":"","url":"https://api.github.com/users/rkd-msw","html_url":"https://github.com/rkd-msw","followers_url":"https://api.github.com/users/rkd-msw/followers","following_url":"https://api.github.com/users/rkd-msw/following{/other_user}","gists_url":"https://api.github.com/users/rkd-msw/gists{/gist_id}","starred_url":"https://api.github.com/users/rkd-msw/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rkd-msw/subscriptions","organizations_url":"https://api.github.com/users/rkd-msw/orgs","repos_url":"https://api.github.com/users/rkd-msw/repos","events_url":"https://api.github.com/users/rkd-msw/events{/privacy}","received_events_url":"https://api.github.com/users/rkd-msw/received_events","type":"User","site_admin":false},"created_at":"2015-02-16T17:39:31Z","updated_at":"2015-02-16T17:39:31Z","body":"Tested and fixed - can you re-review? Changes since the initial version are:\r\n* special handling of NXDOMAIN\r\n* moving the code into a common function, because it was duplicated"}}