Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add p2p daemon #164

Merged
merged 4 commits into from
Mar 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions hivemind/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from hivemind.client import *
from hivemind.dht import *
from hivemind.p2p import *
from hivemind.server import *
from hivemind.utils import *

Expand Down
1 change: 1 addition & 0 deletions hivemind/p2p/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from hivemind.p2p.p2p_daemon import P2P
45 changes: 45 additions & 0 deletions hivemind/p2p/p2p_daemon.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import subprocess
import typing as tp


class P2P(object):
"""
Forks a child process and executes p2pd command with given arguments.
Sends SIGKILL to the child in destructor and on exit from contextmanager.
"""

LIBP2P_CMD = 'p2pd'
Copy link
Member

Choose a reason for hiding this comment

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

@deniskamazur sanity check: can we guarantee that the p2pd you compile in #153 will be always accessible as P2Pd?

Copy link
Collaborator

@dvmazur dvmazur Mar 2, 2021

Choose a reason for hiding this comment

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

Yes, as long as the main package stays in the p2pd directory.
That's what go help install says:
Install compiles and installs the packages named by the import paths...

Copy link
Collaborator

Choose a reason for hiding this comment

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

It actually might be a good idea to download a specific version of libp2p-daemon rather than the latest version of the master branch to make sure refactorings of their codebase don't break our builds.


def __init__(self, *args, **kwargs):
self._child = subprocess.Popen(args=self._make_process_args(args, kwargs))
try:
stdout, stderr = self._child.communicate(timeout=0.2)
except subprocess.TimeoutExpired:
pass
Copy link
Member

Choose a reason for hiding this comment

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

This will do for now, but the waiting time may become an issue.

  • if the process hangs (e.g. slow CPU), we may erroneously trigger TimeoutExpired
  • if we want to spawn multiple P2P daemons, sleep may be a problem

After we add client interaction with p2pd, i'd recommend ping-ing P2PD with the client, and check for errors if that fails.

else:
raise RuntimeError(f'p2p daemon exited with stderr: {stderr}')

def __enter__(self):
return self._child

def __exit__(self, exc_type, exc_val, exc_tb):
self._kill_child()

def __del__(self):
self._kill_child()
Copy link
Member

Choose a reason for hiding this comment

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

[Nitpicking] This will handle del/dereferencing, but not sigkill. Is there perhaps a way to create a non-unix-daemon (aka python daemon) that is guaranteed to die with the host process?


def _kill_child(self):
if self._child.poll() is None:
self._child.kill()
self._child.wait()

def _make_process_args(self, args: tp.Tuple[tp.Any],
kwargs: tp.Dict[str, tp.Any]) -> tp.List[str]:
proc_args = [self.LIBP2P_CMD]
proc_args.extend(
str(entry) for entry in args
)
proc_args.extend(
f'-{key}={str(value)}' for key, value in kwargs.items()
)
return proc_args
55 changes: 55 additions & 0 deletions tests/test_p2p_daemon.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import subprocess
from time import perf_counter

import pytest

import hivemind.p2p
from hivemind.p2p import P2P

RUNNING = 'running'
NOT_RUNNING = 'not running'
CHECK_PID_CMD = '''
if ps -p {0} > /dev/null;
then
echo "{1}"
else
echo "{2}"
fi
'''


def is_process_running(pid: int) -> bool:
cmd = CHECK_PID_CMD.format(pid, RUNNING, NOT_RUNNING)
return subprocess.check_output(cmd, shell=True).decode('utf-8').strip() == RUNNING


@pytest.fixture()
def mock_p2p_class():
P2P.LIBP2P_CMD = "sleep"


def test_daemon_killed_on_del(mock_p2p_class):
start = perf_counter()
p2p_daemon = P2P('10s')

child_pid = p2p_daemon._child.pid
assert is_process_running(child_pid)

del p2p_daemon
assert not is_process_running(child_pid)
assert perf_counter() - start < 1


def test_daemon_killed_on_exit(mock_p2p_class):
start = perf_counter()
with P2P('10s') as daemon:
child_pid = daemon.pid
assert is_process_running(child_pid)

assert not is_process_running(child_pid)
assert perf_counter() - start < 1


def test_daemon_raises_on_faulty_args():
with pytest.raises(RuntimeError):
P2P(faulty='argument')