feat: add p2p daemon #164
Merged
feat: add p2p daemon #164
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
justheuristic
requested changes
Mar 2, 2021
| try: | ||
| stdout, stderr = self._child.communicate(timeout=0.2) | ||
| except subprocess.TimeoutExpired: | ||
| pass |
There was a problem hiding this comment.
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.
| self._kill_child() | ||
|
|
||
| def __del__(self): | ||
| self._kill_child() |
There was a problem hiding this comment.
[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?
| Sends SIGKILL to the child in destructor and on exit from contextmanager. | ||
| """ | ||
|
|
||
| LIBP2P_CMD = 'p2pd' |
There was a problem hiding this comment.
@deniskamazur sanity check: can we guarantee that the p2pd you compile in #153 will be always accessible as P2Pd?
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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.
justheuristic
approved these changes
Mar 2, 2021
dvmazur
pushed a commit
that referenced
this pull request
Mar 10, 2021
* Add p2p daemon * Test p2p daemon exits correctly * Impose restriction on elapsed time Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com>
justheuristic
added a commit
that referenced
this pull request
Mar 10, 2021
* copytree implementation for py37 compatibility (#162) * copytree implementation for py37 compatibility * Running tests for python3.7 * Increment version * Python3.7 notions * Remove pickle.loads in averager (#160) * Security update: remove pickle.loads in averager * add py37 to circleci config Co-authored-by: Alexander Borzunov <borzunov.alexander@gmail.com> Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> * Support edge cases for DHT key/subkey/value, add tests, update .gitignore for pb2 (#167) * fix bug with subkey equals zero * add autogenerated protobuf files to .gitignore * test store and get "tricky" values in dht * Fix the remaining tests for py37 (#166) * DecentralizedAverager is now compatible with python37's acyncio exception * the problem was: grpc.aio with python37 raised concurrent.futures.CancelledError in some cases; * we relied on isinstance(asyncio.CancelledError, Exception) == False * but isinstance(concurrent.futures.CancelledError, Exception) == True * DecentralizedAverager now shuts down if dereferenced in the main process * though it won't shutdown if dereferenced in forks for obvious reasons * HIVEMIND_THREADS now actually works * test_averaging now shuts down dht and averager instances to avoid leaking processes Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> * Move Averager metadata serialization out of user scope (#168) * move metadata serialization outside user scope * test_overcrowded: reduce the default number of peers * Handle edge cases in DecentralizedAverager (#171) * move metadata serialization outside user scope * retry averager.step on network errors * raise AllreduceException on partial tensor * test split/combine tensors, combine corrupted stream Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> * Fix a typo in quickstart.md (#174) * Serialize DHTID source with msgpack (#172) * Change DHTID serializer * Remove unused serializers * Add msgpack tuple serialization * Move CLI server launch script to hivemind/hivemind_cli (#173) * Cast environment variables to correct types * Compiling libp2p daemon on setup (#153) * add setup.py prototype * refactor * feat: add p2p daemon (#164) * Add p2p daemon * Test p2p daemon exits correctly * Impose restriction on elapsed time Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com> * compare golang versions using packaging.version * fix typo Co-authored-by: justheuristic <justheuristic@gmail.com> * move p2pd executable to hivemind/hivemind_cli Co-authored-by: Alexey Bukhtiyarov <a.bukhtiyarov@yandex.ru> Co-authored-by: justheuristic <justheuristic@gmail.com> Co-authored-by: Alexander Borzunov <borzunov.alexander@gmail.com> Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> Co-authored-by: Michael Diskin <yhn112@users.noreply.github.com> Co-authored-by: romakail <36082689+romakail@users.noreply.github.com> Co-authored-by: Ilya <37004806+skobellev@users.noreply.github.com> Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com>
justheuristic
pushed a commit
that referenced
this pull request
Apr 13, 2021
* Add p2p daemon * Test p2p daemon exits correctly * Impose restriction on elapsed time Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com>
justheuristic
added a commit
that referenced
this pull request
Apr 13, 2021
* copytree implementation for py37 compatibility (#162) * copytree implementation for py37 compatibility * Running tests for python3.7 * Increment version * Python3.7 notions * Remove pickle.loads in averager (#160) * Security update: remove pickle.loads in averager * add py37 to circleci config Co-authored-by: Alexander Borzunov <borzunov.alexander@gmail.com> Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> * Support edge cases for DHT key/subkey/value, add tests, update .gitignore for pb2 (#167) * fix bug with subkey equals zero * add autogenerated protobuf files to .gitignore * test store and get "tricky" values in dht * Fix the remaining tests for py37 (#166) * DecentralizedAverager is now compatible with python37's acyncio exception * the problem was: grpc.aio with python37 raised concurrent.futures.CancelledError in some cases; * we relied on isinstance(asyncio.CancelledError, Exception) == False * but isinstance(concurrent.futures.CancelledError, Exception) == True * DecentralizedAverager now shuts down if dereferenced in the main process * though it won't shutdown if dereferenced in forks for obvious reasons * HIVEMIND_THREADS now actually works * test_averaging now shuts down dht and averager instances to avoid leaking processes Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> * Move Averager metadata serialization out of user scope (#168) * move metadata serialization outside user scope * test_overcrowded: reduce the default number of peers * Handle edge cases in DecentralizedAverager (#171) * move metadata serialization outside user scope * retry averager.step on network errors * raise AllreduceException on partial tensor * test split/combine tensors, combine corrupted stream Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> * Fix a typo in quickstart.md (#174) * Serialize DHTID source with msgpack (#172) * Change DHTID serializer * Remove unused serializers * Add msgpack tuple serialization * Move CLI server launch script to hivemind/hivemind_cli (#173) * Cast environment variables to correct types * Compiling libp2p daemon on setup (#153) * add setup.py prototype * refactor * feat: add p2p daemon (#164) * Add p2p daemon * Test p2p daemon exits correctly * Impose restriction on elapsed time Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com> * compare golang versions using packaging.version * fix typo Co-authored-by: justheuristic <justheuristic@gmail.com> * move p2pd executable to hivemind/hivemind_cli Co-authored-by: Alexey Bukhtiyarov <a.bukhtiyarov@yandex.ru> Co-authored-by: justheuristic <justheuristic@gmail.com> Co-authored-by: Alexander Borzunov <borzunov.alexander@gmail.com> Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> Co-authored-by: Michael Diskin <yhn112@users.noreply.github.com> Co-authored-by: romakail <36082689+romakail@users.noreply.github.com> Co-authored-by: Ilya <37004806+skobellev@users.noreply.github.com> Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com>
MaximKsh
pushed a commit
that referenced
this pull request
Apr 20, 2021
* Add p2p daemon * Test p2p daemon exits correctly * Impose restriction on elapsed time Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com>
MaximKsh
pushed a commit
that referenced
this pull request
Apr 20, 2021
* copytree implementation for py37 compatibility (#162) * copytree implementation for py37 compatibility * Running tests for python3.7 * Increment version * Python3.7 notions * Remove pickle.loads in averager (#160) * Security update: remove pickle.loads in averager * add py37 to circleci config Co-authored-by: Alexander Borzunov <borzunov.alexander@gmail.com> Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> * Support edge cases for DHT key/subkey/value, add tests, update .gitignore for pb2 (#167) * fix bug with subkey equals zero * add autogenerated protobuf files to .gitignore * test store and get "tricky" values in dht * Fix the remaining tests for py37 (#166) * DecentralizedAverager is now compatible with python37's acyncio exception * the problem was: grpc.aio with python37 raised concurrent.futures.CancelledError in some cases; * we relied on isinstance(asyncio.CancelledError, Exception) == False * but isinstance(concurrent.futures.CancelledError, Exception) == True * DecentralizedAverager now shuts down if dereferenced in the main process * though it won't shutdown if dereferenced in forks for obvious reasons * HIVEMIND_THREADS now actually works * test_averaging now shuts down dht and averager instances to avoid leaking processes Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> * Move Averager metadata serialization out of user scope (#168) * move metadata serialization outside user scope * test_overcrowded: reduce the default number of peers * Handle edge cases in DecentralizedAverager (#171) * move metadata serialization outside user scope * retry averager.step on network errors * raise AllreduceException on partial tensor * test split/combine tensors, combine corrupted stream Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> * Fix a typo in quickstart.md (#174) * Serialize DHTID source with msgpack (#172) * Change DHTID serializer * Remove unused serializers * Add msgpack tuple serialization * Move CLI server launch script to hivemind/hivemind_cli (#173) * Cast environment variables to correct types * Compiling libp2p daemon on setup (#153) * add setup.py prototype * refactor * feat: add p2p daemon (#164) * Add p2p daemon * Test p2p daemon exits correctly * Impose restriction on elapsed time Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com> * compare golang versions using packaging.version * fix typo Co-authored-by: justheuristic <justheuristic@gmail.com> * move p2pd executable to hivemind/hivemind_cli Co-authored-by: Alexey Bukhtiyarov <a.bukhtiyarov@yandex.ru> Co-authored-by: justheuristic <justheuristic@gmail.com> Co-authored-by: Alexander Borzunov <borzunov.alexander@gmail.com> Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> Co-authored-by: Michael Diskin <yhn112@users.noreply.github.com> Co-authored-by: romakail <36082689+romakail@users.noreply.github.com> Co-authored-by: Ilya <37004806+skobellev@users.noreply.github.com> Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com>
MaximKsh
pushed a commit
that referenced
this pull request
May 14, 2021
* Add p2p daemon * Test p2p daemon exits correctly * Impose restriction on elapsed time Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com>
MaximKsh
pushed a commit
that referenced
this pull request
May 14, 2021
* copytree implementation for py37 compatibility (#162) * copytree implementation for py37 compatibility * Running tests for python3.7 * Increment version * Python3.7 notions * Remove pickle.loads in averager (#160) * Security update: remove pickle.loads in averager * add py37 to circleci config Co-authored-by: Alexander Borzunov <borzunov.alexander@gmail.com> Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> * Support edge cases for DHT key/subkey/value, add tests, update .gitignore for pb2 (#167) * fix bug with subkey equals zero * add autogenerated protobuf files to .gitignore * test store and get "tricky" values in dht * Fix the remaining tests for py37 (#166) * DecentralizedAverager is now compatible with python37's acyncio exception * the problem was: grpc.aio with python37 raised concurrent.futures.CancelledError in some cases; * we relied on isinstance(asyncio.CancelledError, Exception) == False * but isinstance(concurrent.futures.CancelledError, Exception) == True * DecentralizedAverager now shuts down if dereferenced in the main process * though it won't shutdown if dereferenced in forks for obvious reasons * HIVEMIND_THREADS now actually works * test_averaging now shuts down dht and averager instances to avoid leaking processes Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> * Move Averager metadata serialization out of user scope (#168) * move metadata serialization outside user scope * test_overcrowded: reduce the default number of peers * Handle edge cases in DecentralizedAverager (#171) * move metadata serialization outside user scope * retry averager.step on network errors * raise AllreduceException on partial tensor * test split/combine tensors, combine corrupted stream Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> * Fix a typo in quickstart.md (#174) * Serialize DHTID source with msgpack (#172) * Change DHTID serializer * Remove unused serializers * Add msgpack tuple serialization * Move CLI server launch script to hivemind/hivemind_cli (#173) * Cast environment variables to correct types * Compiling libp2p daemon on setup (#153) * add setup.py prototype * refactor * feat: add p2p daemon (#164) * Add p2p daemon * Test p2p daemon exits correctly * Impose restriction on elapsed time Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com> * compare golang versions using packaging.version * fix typo Co-authored-by: justheuristic <justheuristic@gmail.com> * move p2pd executable to hivemind/hivemind_cli Co-authored-by: Alexey Bukhtiyarov <a.bukhtiyarov@yandex.ru> Co-authored-by: justheuristic <justheuristic@gmail.com> Co-authored-by: Alexander Borzunov <borzunov.alexander@gmail.com> Co-authored-by: Max Ryabinin <mryabinin0@gmail.com> Co-authored-by: Michael Diskin <yhn112@users.noreply.github.com> Co-authored-by: romakail <36082689+romakail@users.noreply.github.com> Co-authored-by: Ilya <37004806+skobellev@users.noreply.github.com> Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add P2P class to manage p2p daemon and ensure it is terminated correctly.