Support changing manual configuration while running #383
Conversation
ianpaul10
force-pushed
the
feat/manual-disc-follow-up
branch
5 times, most recently
from
d551110
to
2f955cf
Compare
ianpaul10
force-pushed
the
feat/manual-disc-follow-up
branch
from
2f955cf
to
43faa3b
Compare
ianpaul10
force-pushed
the
feat/manual-disc-follow-up
branch
from
7812943
to
69e0eb1
Compare
|
@AlexCheema This PR should address the follow-up to updating the manual config while exo is running. Please take a look when you get a chance! |
|
|
||
| def _get_peers(self): | ||
| try: | ||
| current_mtime = os.path.getmtime(self.network_config_path) |
There was a problem hiding this comment.
Isn't this blocking?
Blocking every 1 second is going to slow exo down significantly since it runs on one thread.
Perhaps we can move these operations to a thread pool executor?
There was a problem hiding this comment.
Yes, good catch. I updated all calls to self.network_config_path to run within a single thread pool executor. I think things should now be non-blocking and thread safe
| if self._cached_peers is not None and self._last_modified_time is not None and current_mtime <= self._last_modified_time: | ||
| return self._cached_peers | ||
|
|
||
| topology = NetworkTopology.from_path(self.network_config_path) |
There was a problem hiding this comment.
Is from_path blocking? Same comment as above. Can we make all I/O operations to self.network_config_path happen on a thread pool executor with one thread?
ianpaul10
force-pushed
the
feat/manual-disc-follow-up
branch
from
69e0eb1
to
76b105b
Compare
|
@AlexCheema Thanks for the notes, reading from the config file should now be non-blocking |
| async def task_clean_up_peers_from_config(self): | ||
| if DEBUG_DISCOVERY >= 2: print("Starting task to clean up peers from config...") | ||
| while True: | ||
| peers_from_config = await self._get_peers() | ||
| if peers_from_config: | ||
| peers_to_remove = [peer for peer in self.known_peers.keys() if peer not in peers_from_config] | ||
|
|
||
| for peer in peers_to_remove: | ||
| if DEBUG_DISCOVERY >= 2: print(f"{peer} is no longer found in the config but is currently a known peer. Removing from known peers...") | ||
| self.known_peers.pop(peer, None) | ||
|
|
||
| await asyncio.sleep(1.0) |
There was a problem hiding this comment.
I'm not sure why this is needed. I think this may be overcomplicating it.
The way StandardNode is set up, it should be possible to simply return the latest set of peers, i.e. your _get_peers is enough.
There was a problem hiding this comment.
Ah I see, TIL a bit more about StandardNode, thanks!
I thought it was necessary to handle cleaning up nodes that were originally added to the config file (and thus added to self.known_peers) but removed afterwards.
re-load manual discovery file for each runthrough of the peer network, allowing incremental updates to the peer file even when exo is running
…ts updated accordingly
ianpaul10
force-pushed
the
feat/manual-disc-follow-up
branch
from
4219536
to
1edd066
Compare
ianpaul10
force-pushed
the
feat/manual-disc-follow-up
branch
from
1edd066
to
4b5a8d4
Compare
Ref #380
findtaskgetmtimehas been updated, otherwise return the cached peersknown_peerslist if they no-longer exist in the dict but still exist in theknown_peerslist