Clean up a bunch of stuff inside of Axon #426

vEpiphyte · 2017-09-07T14:56:14Z

Add various docstrings
Make Axon configable
Make AxonHost configable
Add tracking to the axonhost the space used in a host
Update unit tests for Axons to account for configable options.
UnitTests for Axons were broken into four classes (for sanity) - AxonTest for testing the base Axon, AxonFSTest for testing the FS features, AxonHostTest for testing the AxonHost class, and AxonClusterTest for testing the AxonCluster class.
Fixed a bug in the daemon.py Onhelp() class which could cause a runtimewarning due to dictionary size changing.
Update SvcProxy.getSynSvcs to return a list instead of a dict_values
Randomize the axon's allocated too by calls to AxonCluster.alloc()
Add a test spinning up a svcbus and axonhost, tearing them down and spinning them back up.
Ensure the clone axon does not have any clones of its own.
If an axon is a clone, it doesn't need to find clones or fire other clones
Make Heap's reactor resize actually resize the underlying atom file.
Initialize the svcproxy local attributes prior to strapping in the event handlers. This prevents events firing during init before class attributes are set.
Have the axonhost advertise itself on the bus AFTER it has made or instantiated any local axons it needs to do. This allows axons to then find hosts on the bus after the axonhosts have finished doing their startup and then they can start doing clone operations.
Address a issue in the Axon datamodel where clone :host was not present
hashset's hashes are now immutable (tuple)
logging in tests is now run by envars and has useful messages format
make sure we check the guid in the axoncluster test.
Enable caching on the Axon cortices. This required updating some of the Axon FS APIs to not perform row-level operations.
Rewrite Cortex formTufoByTufo docstring, add formTufoByTufo test.
Add a Cortex test for delTufoProp with caching.
Misc test coverage improvements
Remove Axon.syncs() api. It is not compatible with delivering one sync event at a time from the persistence file, which requires updating the persistence file for each event sent.
Add a skipLongTests helper to the SynTest class to enable skipping tests which may run long. Run those only when SYN_RUN_LONG_TESTS envar is set.
Add -rs switch to print skip messages in pytest.

vertexmc · 2017-09-26T19:03:09Z

synapse/lib/heap.py

@@ -96,6 +96,7 @@ def _actSyncHeapWrite(self, mesg):

    def _actSyncHeapResize(self, mesg):
        size = mesg[1].get('size')
+        self.atom.resize(size)


vEpiphyte · 2017-10-06T20:52:10Z

synapse/axon.py

@@ -32,36 +34,34 @@

 _fs_attrs = ('st_mode', 'st_nlink', 'st_size', 'st_atime', 'st_ctime', 'st_mtime')

-class AxonHost(s_eventbus.EventBus):
+logging.basicConfig(level=logging.DEBUG,


vEpiphyte · 2017-10-06T20:52:57Z

synapse/axon.py

+                                  'doc': 'Max total allocations for Axons created by the host. '
+                                         'Only applies if set to a positive integer.'}),
+            ('axonhost:syncmax', {'type': 'int', 'defval': gigabyte * 10,
+                                  'doc': ''}),  # XXX Words


undefined option

vEpiphyte · 2017-10-06T21:01:14Z

synapse/axon.py

+        )
+        return confdefs
+
+    @s_eventbus.on('syn:conf:set')


decorator to nuke

vEpiphyte · 2017-10-06T21:02:31Z

synapse/axon.py

    @s_common.firethread
    def _fireAxonClones(self):

+        # If this axon is a clone, then don't try to make or find other clones


this is enforcing the idea that a clone can't have a clone.

vEpiphyte · 2017-10-06T21:08:13Z

synapse/axon.py

            except Exception as e:
-                logger.exception('findAxonClones')
+                logger.exception('Unhandled exception during _findAxonClones')

    def _findAxonClone(self):


This function gives preference to host's with the lowest # of axons. We still have a issue where, in a given set of axon hosts in a bus during startup; multiple hosts may slam the first host with a low count at first.

vEpiphyte · 2017-10-06T21:09:59Z

synapse/axon.py

        '''
        if htype == 'guid':
            blob = self.core.getTufoByProp('axon:blob', valu=hvalu)
        else:
            blob = self.core.getTufoByProp('axon:blob:%s' % htype, valu=hvalu)
-        return self.iterblob(blob)
+        if blob:


The previous behavior returned a generator; while the docs said it yielded bytes. This makes the behavior more explicit.

vEpiphyte · 2017-10-06T21:18:37Z

synapse/lib/service.py

@@ -124,6 +124,14 @@ class SvcProxy(s_eventbus.EventBus):
    def __init__(self, sbus, timeout=None):
        s_eventbus.EventBus.__init__(self)

+        self.byiden = {}


these had to be moved otherwise the service callbacks could fire prior to the service proxy having items to accessing

vEpiphyte · 2017-10-06T21:21:20Z

synapse/tests/test_axon.py

+            self.eq(axonbyts, cv)
+
+            # Wait for clones to react to the events from axon0
+            time.sleep(2)


can we replace this time.sleep with something else?

codecov · 2017-10-09T13:58:50Z

Codecov Report

Merging #426 into master will increase coverage by 0.28%.
The diff coverage is 98.7%.

@@            Coverage Diff             @@
##           master     #426      +/-   ##
==========================================
+ Coverage    90.7%   90.99%   +0.28%     
==========================================
  Files         123      123              
  Lines       14109    14166      +57     
==========================================
+ Hits        12798    12890      +92     
+ Misses       1311     1276      -35

Impacted Files	Coverage Δ
synapse/lib/persist.py	`91.48% <ø> (+1.06%)`	⬆️
synapse/models/axon.py	`100% <ø> (ø)`	⬆️
synapse/telepath.py	`91.45% <ø> (ø)`	⬆️
synapse/cores/common.py	`93.85% <100%> (+0.77%)`	⬆️
synapse/lib/atomfile.py	`92.7% <100%> (+3.57%)`	⬆️
synapse/lib/hashset.py	`95.83% <100%> (ø)`	⬆️
synapse/lib/heap.py	`97.89% <100%> (+1.08%)`	⬆️
synapse/lib/config.py	`97.82% <100%> (+0.04%)`	⬆️
synapse/exc.py	`94.3% <100%> (+0.19%)`	⬆️
synapse/daemon.py	`81.13% <100%> (ø)`	⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7fed0b...598cfa7. Read the comment docs.

invisig0th · 2017-09-08T13:56:14Z

synapse/axon.py

@@ -473,7 +534,7 @@ def __init__(self, axondir, **opts):
        self.axthrs = set()

        self.setAxonInfo('link', self.link)
-        self.setAxonInfo('opts', self.opts)
+        self.setAxonInfo('opts', {key: self.getConfOpt(key) for key, _ in self.getConfDefs().items()})


we could/would/should probably utilitize this dict comprehension so we can reuse it. perhaps something like getConfOpts() returning a dict.

invisig0th · 2017-09-08T13:58:12Z

synapse/axon.py

+                    fd.write(byts)
+
+        Returns:
+            bytes:  A chunk of bytes
        '''
        if htype == 'guid':


maybe we should just add axon:blob:guid as a convenience secondary prop to eliminate this if/else logic
(i know it's not part of these changes :) )

that is cheap to tweak and test

per discussion, we're not going to do this.

invisig0th · 2017-09-08T13:59:20Z

synapse/axon.py

+        if blob:
+            for byts in self.iterblob(blob):
+                yield byts
+        # XXX should throw an exception here?


probably should, yea.

invisig0th · 2017-09-08T14:00:51Z

synapse/axon.py


-            for byts in axon.iterAxonBlob(blob):
-                dostuff(byts)
+                byts = b''.join((_byts for _bytes in eaxon.iterAxonBlob(blob)))


_byts vs _bytes

@vEpiphyte @invisig0th - Does this still need to be changed?

invisig0th · 2017-09-08T14:02:26Z

synapse/axon.py


-            for byts in axon.iterAxonBlob(blob):
-                dostuff(byts)
+                byts = b''.join((_byts for _bytes in eaxon.iterAxonBlob(blob)))


Also, it's probably a bad precedent to encourage them to concat bytes together from an iterative API like this, since the design is meant to handle multi-gig files. Maybe an example more like

fd = file('foo.bin','wb') for byts in axon.iterAxonBlob(blob): fd.write(byts)

or some such...

@vEpiphyte @invisig0th - Does this still need to be changed?

fixed. (example clarified)

invisig0th · 2017-10-10T11:30:53Z

synapse/axon.py

+        # we're passing the latest axonbus to the Axon so it can register
+        # itself properly.
+        if 'axon:axonbus' in opts:
+            axonbus = opts.get('axon:axonbus')


This construct causes 2 hash calculations and tree lookups. Most of the rest of the code base does this like so:

axonbus = opts.get('axonbus') if axonbus is not None: dostuff()

invisig0th · 2017-10-10T11:32:44Z

synapse/axon.py

+            dict: Configable dict of valid options which can be passed to a Axon()
+        '''
+        ret = {}
+        for name, _ in self.getConfDefs().items():


we could avoid this if we made the axonhost config options that were specifically for individual axons start with just axon: and get passed through. They are essentially the "axon options known by axonhost" so it seems clean.

This is now cleaner but we still have to do some examination of the options since we can't just transparently pass options down to the axon.

invisig0th · 2017-10-10T11:33:33Z

synapse/axon.py

+                                  'doc': 'Max total allocations for Axons created by the host. '
+                                         'Only applies if set to a positive integer.'}),
+            ('axonhost:syncmax', {'type': 'int', 'defval': gigabyte * 10,
+                                  'doc': ''}),  # XXX Words


invisig0th · 2017-10-10T11:40:02Z

synapse/lib/service.py

@@ -124,6 +124,14 @@ class SvcProxy(s_eventbus.EventBus):
    def __init__(self, sbus, timeout=None):
        s_eventbus.EventBus.__init__(self)

+        self.byiden = {}


invisig0th · 2017-10-10T11:41:51Z

synapse/tests/common.py

@@ -81,6 +84,10 @@ def skipIfNoInternet(self):
        if os.getenv('SYN_TEST_NO_INTERNET'):
            raise unittest.SkipTest('no internet access')

+    def skipLongTests(self):
+        if not os.getenv('SYN_RUN_LONG_TESTS'):


This feels like it should default the other way? I'd rather a "check it out and run" tries to run everything, and we tweak them "off" when it's not right for the use case. Probably most intuitive to rename it to "SYN_TEST_SKIP_LONG" or something.

I'd like every test to run by default and have these test skip env vars all be SYN_SKIP_... like the "skip if no internet" fn in your other PR. I do like having a way to skip long tests though. We could have push builds skip them, but PR builds run them to free up some build server time.

This have been renamed to SYN_TEST_SKIP_LONG to explicitly skip long running tests. The .drone.yml was updated to set this envar to 1 on PUSH events.

vertexmc · 2017-10-10T13:04:22Z

synapse/lib/service.py

@@ -124,6 +124,14 @@ class SvcProxy(s_eventbus.EventBus):
    def __init__(self, sbus, timeout=None):
        s_eventbus.EventBus.__init__(self)

+        self.byiden = {}


vertexmc · 2017-10-10T13:10:21Z

synapse/tests/common.py

@@ -81,6 +84,10 @@ def skipIfNoInternet(self):
        if os.getenv('SYN_TEST_NO_INTERNET'):
            raise unittest.SkipTest('no internet access')

+    def skipLongTests(self):
+        if not os.getenv('SYN_RUN_LONG_TESTS'):


I'd like every test to run by default and have these test skip env vars all be SYN_SKIP_... like the "skip if no internet" fn in your other PR. I do like having a way to skip long tests though. We could have push builds skip them, but PR builds run them to free up some build server time.

vertexmc · 2017-10-10T13:30:42Z

synapse/axon.py

-            self.axonbus.runSynSvc(s_common.guid(), self)
+        # track the total number of bytes which may be used by axons
+        # associated with this axonhost (disregarding heap overhead)
+        self.usedspace = 0


Should we be tracking heap overhead?

think this is a old comment

clarified comment

vertexmc · 2017-10-10T14:20:45Z

synapse/tests/test_axon.py

-            axon = s_axon.Axon(axondir, syncsize=64)
+            # XXX It is not clear what this test is actually testing since syncsize is not used by anything inside of
+            # axon.py
+            axon = s_axon.Axon(axondir, **{'axon:syncsize': 64})


should syncsize be removed then?

we could probably rip it out

vertexmc

I reviewed again and added two comments. 👍 from me after they're resolved.

- Add various docstrings - Make Axon configable - Make AxonHost configable - Add tracking to the axon to track the space used in one / the space used in a axonhost - Update unit tests for Axons - Add axonhost:clones to set a default number of clones for a child axon - Update SvcProxy.getSynSvcs to return a list instead of a dict_values - Randomize the axon's allocated too by calls to AxonCluster.alloc() - Add a test spining up a svcbus and axonhost, tearing them down and spinning them back up. - The clone axon does not have any clones of its own. - If an axon is a clone, it doens't need to find clones or fire other clones - Make Heap's reactor resize actually resize the underlying atom file. - Initialize the svcproxy local attributes prior to strapping in the event handlers. - Have the axonhost advertise itself on the bus AFTER it has made or instantiated any local axons it needs to do. This allows axons to then find hosts on the bus after the axonhosts have finished doing their startup and then they can start doing clone operations. - Address a issue in the Axon datamodel where clone :host was not present - hashset's hashes are now immutable (tuple) - logging in tests is now run by envars and has useful messages format - make sure we check the guid in the axoncluster test. - Enable caching on the Axon cortices. This required updating some of the Axon FS APIs to not perform row-level operations. - Rewrite Cortex formTufoByTufo docstring, add formTufoByTufo test. - Add a Cortex test for delTufoProp with caching. - Misc test coverage improvements - Remove Axon.syncs() api. It is not compatible with delivering one sync event at a time from the persistence file, which requires updating the persistence file for each event sent. - Add a skipLongTests helper to the SynTest class to enable skipping tests which may run long. Run those only when SYN_RUN_LONG_TESTS envar is set. - Add -rs switch to print skip messages in pytest.

…eve, in one pass, all of the configable options and their values for an object.

…o a positive integer before skiping tests. Update .drone.yml to skip long tests on push events.

…he axon: namespace.

…les.

vEpiphyte added the enhancement label Sep 7, 2017

vEpiphyte changed the title ~~Clean up a bunch of stuff inside of Axon~~ WIP - Clean up a bunch of stuff inside of Axon Sep 7, 2017

vEpiphyte force-pushed the axon_docs branch 4 times, most recently from 6090827 to 2546fb3 Compare September 13, 2017 14:31

vertexproject deleted a comment from codecov bot Sep 13, 2017

vEpiphyte force-pushed the axon_docs branch 2 times, most recently from fc47a42 to 73bcfe3 Compare September 20, 2017 17:57

vEpiphyte force-pushed the axon_docs branch 2 times, most recently from 07cac3e to fecac52 Compare September 26, 2017 18:43

vertexmc reviewed Sep 26, 2017

View reviewed changes

vEpiphyte force-pushed the axon_docs branch 4 times, most recently from e3f7955 to 38ee54d Compare September 29, 2017 16:50

vEpiphyte force-pushed the axon_docs branch 4 times, most recently from d9d7ccf to 3231ab1 Compare October 6, 2017 20:51

vEpiphyte commented Oct 6, 2017

View reviewed changes

synapse/axon.py Outdated

)

return confdefs

@s_eventbus.on('syn:conf:set')

Copy link

Contributor Author

vEpiphyte Oct 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decorator to nuke

vEpiphyte commented Oct 6, 2017

View reviewed changes

vEpiphyte force-pushed the axon_docs branch from 604fcb6 to a7e993b Compare October 8, 2017 12:46

vertexproject deleted a comment from codecov bot Oct 8, 2017

vEpiphyte force-pushed the axon_docs branch 4 times, most recently from d1d4de6 to ac398bc Compare October 9, 2017 23:00

vEpiphyte changed the title ~~WIP - Clean up a bunch of stuff inside of Axon~~ Clean up a bunch of stuff inside of Axon Oct 10, 2017

invisig0th reviewed Oct 10, 2017

View reviewed changes

vertexmc reviewed Oct 10, 2017

View reviewed changes

vEpiphyte force-pushed the axon_docs branch 5 times, most recently from 43dc8d7 to 3d77567 Compare October 10, 2017 19:05

vertexmc reviewed Oct 11, 2017

View reviewed changes

vEpiphyte added 10 commits October 11, 2017 10:52

Add a getConfOpts() API to configable so that it can be used to retri…

ca2e9cc

…eve, in one pass, all of the configable options and their values for an object.

Rename SYN_TEST_RUN_LONG to SYN_TEST_SKIP_LONG and ensure it is set t…

b08b1b3

…o a positive integer before skiping tests. Update .drone.yml to skip long tests on push events.

Consistent code formatting

be34f96

Add a test for axon:blob model

712ae59

Fix axonbus reference.

443a8b9

Make the axonhost options which are directly passable to axon's use t…

d290d87

…he axon: namespace.

Clarify comment

8c069e0

Rip out all unused :sync options from Axon configables.

a2ce090

Update docstrings, make axon.bytes() throw an exception on missing fi…

598cfa7

…les.

vEpiphyte force-pushed the axon_docs branch from a5461fe to 598cfa7 Compare October 11, 2017 15:34

vertexmc approved these changes Oct 11, 2017

View reviewed changes

vEpiphyte added this to the v0.0.27 milestone Oct 11, 2017

vEpiphyte added the bug label Oct 11, 2017

vEpiphyte merged commit 0133563 into vertexproject:master Oct 11, 2017

vEpiphyte deleted the axon_docs branch October 11, 2017 15:57

Clean up a bunch of stuff inside of Axon #426

Clean up a bunch of stuff inside of Axon #426

Conversation

vEpiphyte commented Sep 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 9, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vertexmc left a comment

Choose a reason for hiding this comment

vEpiphyte commented Sep 7, 2017 •

edited

Loading

codecov bot commented Oct 9, 2017 •

edited

Loading