BIND 10 trac1259, updated. 763a994cb14bb11ba823831f54d64071319bfac0 [1259] Compaction of the diff
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Sep 30 18:07:34 UTC 2011
The branch, trac1259 has been updated
via 763a994cb14bb11ba823831f54d64071319bfac0 (commit)
via b86d51b24e7d1bb4980426c9a74962628c096ba7 (commit)
via 48d5ac59277e2e8b43f697a0d1d4b0991a40caa0 (commit)
via c191f23dfc2b0179ec0a010a1ff00fa3ae1d9398 (commit)
via 8d2c46f19c1b4f435d7b9180ff6c2e8daf78ab2b (commit)
via 80319933903fbdb359ef9472573bfaceda7c8cd5 (commit)
from 9b23d60d6f58b18da3995dc3e090d7fd63233bcc (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 763a994cb14bb11ba823831f54d64071319bfac0
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Sep 30 20:07:21 2011 +0200
[1259] Compaction of the diff
commit b86d51b24e7d1bb4980426c9a74962628c096ba7
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Sep 30 20:07:03 2011 +0200
[1259] Test of compaction of the diff
commit 48d5ac59277e2e8b43f697a0d1d4b0991a40caa0
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Sep 30 18:08:40 2011 +0200
[1259] Comment what the compact method really does
The way it compacts is explained.
commit c191f23dfc2b0179ec0a010a1ff00fa3ae1d9398
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Sep 30 17:59:31 2011 +0200
[1259] Auto-apply every 100 rdata
commit 8d2c46f19c1b4f435d7b9180ff6c2e8daf78ab2b
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Sep 30 17:52:50 2011 +0200
[1259] Test for auto-apply every 100
When the diff grows to 100 RRs, the apply should be called by itself.
commit 80319933903fbdb359ef9472573bfaceda7c8cd5
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Sep 30 17:09:08 2011 +0200
[1259] Check that stuff is not called after commit
Because it just makes no sense. So we raise an exception.
-----------------------------------------------------------------------
Summary of changes:
src/lib/python/isc/xfrin/diff.py | 48 ++++++++++-
src/lib/python/isc/xfrin/tests/diff_tests.py | 116 ++++++++++++++++++++++++++
2 files changed, 159 insertions(+), 5 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/python/isc/xfrin/diff.py b/src/lib/python/isc/xfrin/diff.py
index 38b0e10..6c05401 100644
--- a/src/lib/python/isc/xfrin/diff.py
+++ b/src/lib/python/isc/xfrin/diff.py
@@ -18,6 +18,8 @@ This helps the XFR in process with accumulating parts of diff and applying
it to the datasource.
"""
+import isc.dns
+
class NoSuchZone(Exception):
"""
This is raised if a diff for non-existant zone is being created.
@@ -58,6 +60,15 @@ class Diff:
str(datasource))
self.__buffer = []
+ def __check_commited(self):
+ """
+ This checks if the diff is already commited. If it is, it raises
+ ValueError. This check is for methods that need to work only on
+ yet uncommited diffs.
+ """
+ if self.__updater is None:
+ raise ValueError("The diff is already commited, you come late")
+
def __data_common(self, rr, operation):
"""
Schedules an operation with rr.
@@ -65,10 +76,14 @@ class Diff:
It does all the real work of add_data and remove_data, including
all checks.
"""
+ self.__check_commited()
if rr.get_rdata_count() != 1:
raise ValueError('The rrset must contain exactly 1 Rdata, but ' +
'it holds ' + str(rr.get_rdata_count()))
self.__buffer.append((operation, rr))
+ if len(self.__buffer) >= 100:
+ # Time to auto-apply, so the data don't accumulate too much
+ self.apply()
def add_data(self, rr):
"""
@@ -95,11 +110,27 @@ class Diff:
Tries to compact the operations in buffer a little by putting some of
the operations together, forming RRsets with more than one RR.
- This is called by apply before putting the data into datasource.
-
- It is currently empty and needs implementing.
- """
- pass
+ This is called by apply before putting the data into datasource. You
+ may, but not have to, call this manually.
+
+ Currently it merges consecutive same operations on the same
+ domain/type. We could do more fancy things, like sorting by the domain
+ and do more merging, but such diffs should be rare in practice anyway,
+ so we don't bother and do it this simple way.
+ """
+ buf = []
+ for (op, rrset) in self.__buffer:
+ old = buf[-1][1] if len(buf) > 0 else None
+ if old is None or op != buf[-1][0] or \
+ rrset.get_name() != old.get_name() or \
+ rrset.get_type() != old.get_type():
+ buf.append((op, isc.dns.RRset(rrset.get_name(),
+ rrset.get_class(),
+ rrset.get_type(),
+ rrset.get_ttl())))
+ for rdatum in rrset.get_rdata():
+ buf[-1][1].add_rdata(rdatum)
+ self.__buffer = buf
def apply(self):
"""
@@ -117,6 +148,7 @@ class Diff:
It also can raise isc.datasrc.Error. If that happens, you should stop
using this object and abort the modification.
"""
+ self.__check_commited()
# First, compact the data
self.compact()
# Then pass the data inside the data source
@@ -138,8 +170,14 @@ class Diff:
This might raise isc.datasrc.Error.
"""
+ self.__check_commited()
+ # Push the data inside the data source
self.apply()
+ # Make sure they are visible.
self.__updater.commit()
+ # Remove the updater. That will free some resources for one, but
+ # mark this object as already commited, so we can check
+ self.__updater = None
def get_buffer(self):
"""
diff --git a/src/lib/python/isc/xfrin/tests/diff_tests.py b/src/lib/python/isc/xfrin/tests/diff_tests.py
index 2735401..963b16d 100644
--- a/src/lib/python/isc/xfrin/tests/diff_tests.py
+++ b/src/lib/python/isc/xfrin/tests/diff_tests.py
@@ -199,12 +199,128 @@ class DiffTest(unittest.TestCase):
"""
diff = Diff(self, Name('example.org.'))
diff.add_data(self.__rrset1)
+ orig_apply = diff.apply
diff.apply = self.__mock_apply
diff.commit()
self.assertTrue(self.__apply_called)
self.assertTrue(self.__commit_called)
# The data should be handled by apply which we replaced.
self.assertEqual([], self.__data_operations)
+ # Now check all range of other methods raise ValueError
+ self.assertRaises(ValueError, diff.commit)
+ self.assertRaises(ValueError, diff.add_data, self.__rrset2)
+ self.assertRaises(ValueError, diff.remove_data, self.__rrset1)
+ diff.apply = orig_apply
+ self.assertRaises(ValueError, diff.apply)
+ # This one does not state it should raise, so check it doesn't
+ # But it is NOP in this situation anyway
+ diff.compact()
+
+ def test_autoapply(self):
+ """
+ Test the apply is called all by itself after 100 tasks are added.
+ """
+ diff = Diff(self, Name('example.org.'))
+ # A method to check the apply is called _after_ the 100th element
+ # is added. We don't use it anywhere else, so we define it locally
+ # as lambda function
+ def check():
+ self.assertEqual(100, len(diff.get_buffer()))
+ self.__mock_apply()
+ orig_apply = diff.apply
+ diff.apply = check
+ # If we put 99, nothing happens yet
+ for i in range(0, 99):
+ diff.add_data(self.__rrset1)
+ expected = [('add', self.__rrset1)] * 99
+ self.assertEqual(expected, diff.get_buffer())
+ self.assertFalse(self.__apply_called)
+ # Now we push the 100th and it should call the apply method
+ # This will _not_ flush the data yet, as we replaced the method.
+ # It, however, would in the real life.
+ diff.add_data(self.__rrset1)
+ # Now the apply method (which is replaced by our check) should
+ # have been called. If it wasn't, this is false. If it was, but
+ # still with 99 elements, the check would complain
+ self.assertTrue(self.__apply_called)
+ # Reset the buffer by calling the original apply.
+ orig_apply()
+ self.assertEqual([], diff.get_buffer())
+ # Similar with remove
+ self.__apply_called = False
+ for i in range(0, 99):
+ diff.remove_data(self.__rrset2)
+ expected = [('remove', self.__rrset2)] * 99
+ self.assertEqual(expected, diff.get_buffer())
+ self.assertFalse(self.__apply_called)
+ diff.remove_data(self.__rrset2)
+ self.assertTrue(self.__apply_called)
+
+ def test_compact(self):
+ """
+ Test the compaction works as expected, eg. it compacts only consecutive
+ changes of the same operation and on the same domain/type.
+
+ The test case checks that it does merge them, but also puts some
+ different operations "in the middle", changes the type and name and
+ places the same kind of change further away of each other to see they
+ are not merged in that case.
+ """
+ diff = Diff(self, Name('example.org.'))
+ # Check we can do a compact on empty data, it shouldn't break
+ diff.compact()
+ self.assertEqual([], diff.get_buffer())
+ # This data is the way it should look like after the compact
+ # ('operation', 'domain.prefix', 'type', ['rdata', 'rdata'])
+ # The notes say why the each of consecutive can't be merged
+ data = [
+ ('add', 'a', 'A', ['192.0.2.1', '192.0.2.2']),
+ # Different type.
+ ('add', 'a', 'AAAA', ['2001:db8::1', '2001:db8::2']),
+ # Different operation
+ ('remove', 'a', 'AAAA', ['2001:db8::3']),
+ # Different domain
+ ('remove', 'b', 'AAAA', ['2001:db8::4']),
+ # This does not get merged with the first, even if logically
+ # possible. We just don't do this.
+ ('add', 'a', 'A', ['192.0.2.3'])
+ ]
+ # Now, fill the data into the diff, in a "flat" way, one by one
+ for (op, nprefix, rrtype, rdata) in data:
+ name = Name(nprefix + '.example.org.')
+ rrtype_obj = RRType(rrtype)
+ for rdatum in rdata:
+ rrset = RRset(name, self.__rrclass, rrtype_obj, self.__ttl)
+ rrset.add_rdata(Rdata(rrtype_obj, self.__rrclass, rdatum))
+ if op == 'add':
+ diff.add_data(rrset)
+ else:
+ diff.remove_data(rrset)
+ # Compact it
+ diff.compact()
+ # Now check they got compacted. They should be in the same order as
+ # pushed inside. So it should be the same as data modulo being in
+ # the rrsets and isc.dns objects.
+ def check():
+ buf = diff.get_buffer()
+ self.assertEqual(len(data), len(buf))
+ for (expected, received) in zip(data, buf):
+ (eop, ename, etype, edata) = expected
+ (rop, rrrset) = received
+ self.assertEqual(eop, rop)
+ ename_obj = Name(ename + '.example.org.')
+ self.assertEqual(ename_obj, rrrset.get_name())
+ # We check on names to make sure they are printed nicely
+ self.assertEqual(etype, str(rrrset.get_type()))
+ rdata = rrrset.get_rdata()
+ self.assertEqual(len(edata), len(rdata))
+ # It should also preserve the order
+ for (edatum, rdatum) in zip(edata, rdata):
+ self.assertEqual(edatum, str(rdatum))
+ check()
+ # Try another compact does nothing, but survives
+ diff.compact()
+ check()
if __name__ == "__main__":
isc.log.init("bind10")
More information about the bind10-changes
mailing list