Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Read from DNS cache if within TTL #677

Merged
merged 4 commits into from Apr 8, 2016

Conversation

Projects
None yet
3 participants
Owner

erikjohnston commented Mar 31, 2016

No description provided.

@oddvar oddvar added the in progress label Mar 31, 2016

@NegativeMjark NegativeMjark commented on an outdated diff Mar 31, 2016

synapse/http/endpoint.py
@@ -154,6 +156,12 @@ def connect(self, protocolFactory):
@defer.inlineCallbacks
def resolve_service(service_name, dns_client=client, cache=SERVER_CACHE):
+ cache_entry = cache.get(service_name, None)
+ if cache_entry:
+ if all(s.expires > int(time.time()) for s in cache_entry):
@NegativeMjark

NegativeMjark Mar 31, 2016

Contributor

Hmm, I tend to prefer to pass in the current time to these things to make testing easier.

@NegativeMjark NegativeMjark commented on an outdated diff Mar 31, 2016

synapse/http/endpoint.py
- host=ip,
- port=int(payload.port),
- priority=int(payload.priority),
- weight=int(payload.weight)
- ))
+ for answer in answers:
+ if answer.type == dns.A and answer.payload:
+ ip = answer.payload.dottedQuad()
+ host_ttl = min(srv_ttl, answer.ttl)
+
+ servers.append(_Server(
+ host=ip,
+ port=int(payload.port),
+ priority=int(payload.priority),
+ weight=int(payload.weight),
+ expires=int(time.time()) + host_ttl,
@NegativeMjark

NegativeMjark Mar 31, 2016

Contributor

As above wrt to passing in the current time, or using a mockable clock for getting the current time.

@NegativeMjark NegativeMjark commented on the diff Mar 31, 2016

tests/test_dns.py
cache = {
- service_name: [object()]
+ service_name: [entry]
}
servers = yield resolve_service(
@NegativeMjark

NegativeMjark Mar 31, 2016

Contributor

Now that resolve_service takes a clock, it might be worth using a mock clock here?
Also might be nice to have a test that checks that things expire.

Contributor

NegativeMjark commented Apr 8, 2016

LGTM

@erikjohnston erikjohnston merged commit 79fc4ff into develop Apr 8, 2016

8 checks passed

Flake8 + Packaging (Commit) Build #321 origin/erikj/dns_cache succeeded in 28 sec
Details
Flake8 + Packaging (Merged PR) Build finished.
Details
Sytest Postgres (Commit) Build #318 origin/erikj/dns_cache succeeded in 5 min 40 sec
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Commit) Build #321 origin/erikj/dns_cache succeeded in 3 min 20 sec
Details
Sytest SQLite (Merged PR) Build finished.
Details
Unit Tests (Commit) Build #365 origin/erikj/dns_cache succeeded in 1 min 17 sec
Details
Unit Tests (Merged PR) Build finished.
Details

@richvdh richvdh deleted the erikj/dns_cache branch Dec 1, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment