mirror/github-octodns
1
0
Fork 0
mirror of https://github.com/github/octodns.git synced 2024-05-11 05:55:00 +00:00
Code Issues Releases Activity

Fix major performance issue with add_record O(N^2)

Browse Source 
Before, 1-2k record took ~10s and more than that was just painful, 5k took
forever. This records things to keep a dict of nodes with a set of records so
that we can quickly "jump" to the point we're looking for without having to
search. 10k records now takes ~5s.
This commit is contained in:
Ross McFarland
2017-07-02 18:23:45 -07:00
parent 69817ab465
commit 908698da49
8 changed files with 44 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
octodns/__init__.py
View File

@@ -1,6 +1,4 @@
'''
OctoDNS: DNS as code - Tools for managing DNS across multiple providers
'''
'OctoDNS: DNS as code - Tools for managing DNS across multiple providers'
from __future__ import absolute_import, division, print_function, \
unicode_literals

39
octodns/zone.py
View File

@@ -5,6 +5,7 @@
from __future__ import absolute_import, division, print_function, \
unicode_literals
from collections import defaultdict
from logging import getLogger
import re
@@ -39,13 +40,19 @@ class Zone(object):
# Force everyting to lowercase just to be safe
self.name = str(name).lower() if name else name
self.sub_zones = sub_zones
self.records = set()
# We're grouping by node, it allows us to efficently search for
# duplicates and detect when CNAMEs co-exist with other records
self._records = defaultdict(set)
# optional leading . to match empty hostname
# optional trailing . b/c some sources don't have it on their fqdn
self._name_re = re.compile('\.?{}?$'.format(name))
self.log.debug('__init__: zone=%s, sub_zones=%s', self, sub_zones)
@property
def records(self):
return set([r for _, node in self._records.items() for r in node])
def hostname_from_fqdn(self, fqdn):
return self._name_re.sub('', fqdn)
@@ -53,9 +60,6 @@ class Zone(object):
name = record.name
last = name.split('.')[-1]
if replace and record in self.records:
self.records.remove(record)
if last in self.sub_zones:
if name != last:
# it's a record for something under a sub-zone
@@ -67,19 +71,30 @@ class Zone(object):
raise SubzoneRecordException('Record {} a managed sub-zone '
'and not of type NS'
.format(record.fqdn))
# TODO: this is pretty inefficent
for existing in self.records:
if record == existing:
if replace:
# will remove it if it exists
self._records[name].discard(record)
node = self._records[name]
if record in node:
# We already have a record at this node of this type
raise DuplicateRecordException('Duplicate record {}, type {}'
.format(record.fqdn,
record._type))
elif name == existing.name and (record._type == 'CNAME' or
existing._type == 'CNAME'):
raise InvalidNodeException('Invalid state, CNAME at {} '
'cannot coexist with other records'
elif ((record._type == 'CNAME' and len(node) > 0) or
('CNAME' in map(lambda r: r._type, node))):
# We're adding a CNAME to existing records or adding to an existing
# CNAME
raise InvalidNodeException('Invalid state, CNAME at {} cannot '
'coexist with other records'
.format(record.fqdn))
self.records.add(record)
node.add(record)
def _remove_record(self, record):
'Only for use in tests'
self._records[record.name].discard(record)
def changes(self, desired, target):
self.log.debug('changes: zone=%s, target=%s', self, target)

2
tests/test_octodns_provider_cloudflare.py
View File

@@ -33,7 +33,7 @@ class TestCloudflareProvider(TestCase):
}))
for record in list(expected.records):
if record.name == 'sub' and record._type == 'NS':
expected.records.remove(record)
expected._remove_record(record)
break
empty = {'result': [], 'result_info': {'count': 0, 'per_page': 0}}

2
tests/test_octodns_provider_dnsimple.py
View File

@@ -33,7 +33,7 @@ class TestDnsimpleProvider(TestCase):
}))
for record in list(expected.records):
if record.name == 'sub' and record._type == 'NS':
expected.records.remove(record)
expected._remove_record(record)
break
def test_populate(self):

3
tests/test_octodns_provider_ns1.py
View File

@@ -196,7 +196,8 @@ class TestNs1Provider(TestCase):
provider = Ns1Provider('test', 'api-key')
desired = Zone('unit.tests.', [])
desired.records.update(self.expected)
for r in self.expected:
desired.add_record(r)
plan = provider.plan(desired)
# everything except the root NS

2
tests/test_octodns_provider_powerdns.py
View File

@@ -253,7 +253,7 @@ class TestPowerDnsProvider(TestCase):
plan = provider.plan(expected)
self.assertFalse(plan)
# remove it now that we don't need the unrelated change any longer
expected.records.remove(unrelated_record)
expected._remove_record(unrelated_record)
# ttl diff
with requests_mock() as mock:

12
tests/test_octodns_provider_route53.py
View File

@@ -372,11 +372,11 @@ class TestRoute53Provider(TestCase):
# Delete by monkey patching in a populate that includes an extra record
def add_extra_populate(existing, target, lenient):
for record in self.expected.records:
existing.records.add(record)
existing.add_record(record)
record = Record.new(existing, 'extra',
{'ttl': 99, 'type': 'A',
'values': ['9.9.9.9']})
existing.records.add(record)
existing.add_record(record)
provider.populate = add_extra_populate
change_resource_record_sets_params = {
@@ -409,7 +409,7 @@ class TestRoute53Provider(TestCase):
def mod_geo_populate(existing, target, lenient):
for record in self.expected.records:
if record._type != 'A' or not record.geo:
existing.records.add(record)
existing.add_record(record)
record = Record.new(existing, '', {
'ttl': 61,
'type': 'A',
@@ -420,7 +420,7 @@ class TestRoute53Provider(TestCase):
'NA-US-KY': ['7.2.3.4']
}
})
existing.records.add(record)
existing.add_record(record)
provider.populate = mod_geo_populate
change_resource_record_sets_params = {
@@ -505,7 +505,7 @@ class TestRoute53Provider(TestCase):
def mod_add_geo_populate(existing, target, lenient):
for record in self.expected.records:
if record._type != 'A' or record.geo:
existing.records.add(record)
existing.add_record(record)
record = Record.new(existing, 'simple', {
'ttl': 61,
'type': 'A',
@@ -514,7 +514,7 @@ class TestRoute53Provider(TestCase):
'OC': ['3.2.3.4', '4.2.3.4'],
}
})
existing.records.add(record)
existing.add_record(record)
provider.populate = mod_add_geo_populate
change_resource_record_sets_params = {

2
tests/test_octodns_zone.py
View File

@@ -77,7 +77,7 @@ class TestZone(TestCase):
# add a record, delete a record -> [Delete, Create]
c = ARecord(before, 'c', {'ttl': 42, 'value': '1.1.1.1'})
after.add_record(c)
after.records.remove(b)
after._remove_record(b)
self.assertEquals(after.records, set([a, c]))
changes = before.changes(after, target)
self.assertEquals(2, len(changes))