You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/11/17 18:19:13 UTC
[GitHub] [tvm] Icemist opened a new pull request, #13420: [RPC] Fix tracker connection termination
Icemist opened a new pull request, #13420:
URL: https://github.com/apache/tvm/pull/13420
In a wireless connection situation (android example below) we have a device key containing a colon character(**:**).
_ANDROID_SERIAL_NUMBER = 192.168.0.143:5555_
```
C:\Users\icemist>docker exec -it ice_tvm_container adb devices
List of devices attached
192.168.0.143:5555 device
```
In this case we get an error:
```
ERROR:asyncio:Exception in callback None()
handle: <Handle cancelled>
Traceback (most recent call last):
File "/usr/lib/python3.8/asyncio/events.py", line 81, in _run
self._context.run(self._callback, *self._args)
File "/venv/apache-tvm-py3.8/lib/python3.8/site-packages/tornado/platform/asyncio.py", line 206, in _handle_events
handler_func(fileobj, events)
File "/git/tvm/python/tvm/rpc/tornado_util.py", line 41, in _event_handler
self._event_handler(events)
File "/git/tvm/python/tvm/rpc/tornado_util.py", line 79, in _event_handler
if self._update_read() and (events & self._ioloop.WRITE):
File "/git/tvm/python/tvm/rpc/tornado_util.py", line 121, in _update_read
self.close()
File "/git/tvm/python/tvm/rpc/tornado_util.py", line 67, in close
self.on_close()
File "/git/tvm/python/tvm/rpc/tracker.py", line 298, in on_close
self._tracker.close(self)
File "/git/tvm/python/tvm/rpc/tracker.py", line 353, in close
self._scheduler_map[rpc_key].remove(value)
KeyError: '127.0.0.1'
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on a diff in pull request #13420: [RPC] Fix tracker connection termination
Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13420:
URL: https://github.com/apache/tvm/pull/13420#discussion_r1026456535
##########
tests/python/unittest/test_runtime_rpc.py:
##########
@@ -448,10 +448,10 @@ def check_remote():
@tvm.testing.requires_rpc
-def test_rpc_tracker_register():
+@pytest.mark.parametrize("device_key", ["test_device", "127.0.0.1:5555"])
Review Comment:
Thank you for clarification! In verbal discussion, decided to fix all usage of key split in TVM code, create a function which have to do this split and write simple unit tests on it. To show that the key might has three parts separated by `:`: `host:port:unique_random_key`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Icemist commented on a diff in pull request #13420: [RPC] Fix tracker connection termination
Posted by GitBox <gi...@apache.org>.
Icemist commented on code in PR #13420:
URL: https://github.com/apache/tvm/pull/13420#discussion_r1025776485
##########
tests/python/unittest/test_runtime_rpc.py:
##########
@@ -448,10 +448,10 @@ def check_remote():
@tvm.testing.requires_rpc
-def test_rpc_tracker_register():
+@pytest.mark.parametrize("device_key", ["test_device", "127.0.0.1:5555"])
Review Comment:
I'm not sure. I want to fix exactly the current case.
The issue in that case is in format of key `hexagon-dev.192.168.0.143:5555:0.513619`.
Before the fix this test was falling with "127.0.0.1:5555", after it works
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Icemist commented on pull request #13420: [RPC] Fix tracker connection termination
Posted by GitBox <gi...@apache.org>.
Icemist commented on PR #13420:
URL: https://github.com/apache/tvm/pull/13420#issuecomment-1319033079
@echuraev @apeskov @areusch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Icemist commented on a diff in pull request #13420: [RPC] Fix tracker connection termination
Posted by GitBox <gi...@apache.org>.
Icemist commented on code in PR #13420:
URL: https://github.com/apache/tvm/pull/13420#discussion_r1027823511
##########
python/tvm/rpc/base.py:
##########
@@ -136,13 +139,30 @@ def random_key(prefix, cmap=None):
key : str
The generated random key
"""
- if cmap:
- while True:
- key = prefix + str(random.random())
- if key not in cmap:
- return key
- else:
- return prefix + str(random.random())
+ while True:
+ key = "{}{}{}".format(prefix, delimiter, random.random())
+ if not cmap or key not in cmap:
+ break
Review Comment:
there is no saving of the variable key between iterations of the loop. there is a test for this situation. tests/python/unittest/test_rpc_base.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev merged pull request #13420: [RPC] Fix tracker connection termination
Posted by GitBox <gi...@apache.org>.
echuraev merged PR #13420:
URL: https://github.com/apache/tvm/pull/13420
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tvm-bot commented on pull request #13420: [RPC] Fix tracker connection termination
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13420:
URL: https://github.com/apache/tvm/pull/13420#issuecomment-1319029692
<!---bot-comment-->
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* No users to tag found in teams: `rpc` <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on a diff in pull request #13420: [RPC] Fix tracker connection termination
Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13420:
URL: https://github.com/apache/tvm/pull/13420#discussion_r1027918168
##########
python/tvm/rpc/base.py:
##########
@@ -136,13 +139,30 @@ def random_key(prefix, cmap=None):
key : str
The generated random key
"""
- if cmap:
- while True:
- key = prefix + str(random.random())
- if key not in cmap:
- return key
- else:
- return prefix + str(random.random())
+ while True:
+ key = "{}{}{}".format(prefix, delimiter, random.random())
+ if not cmap or key not in cmap:
+ break
Review Comment:
Ups, sorry, my bad. Didn't notice that. Thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Icemist commented on a diff in pull request #13420: [RPC] Fix tracker connection termination
Posted by GitBox <gi...@apache.org>.
Icemist commented on code in PR #13420:
URL: https://github.com/apache/tvm/pull/13420#discussion_r1027823736
##########
python/tvm/rpc/proxy.py:
##########
@@ -319,7 +319,7 @@ def _regenerate_server_keys(self, keys):
new_keys = []
# re-generate the server match key, so old information is invalidated.
for key in keys:
- rpc_key, _ = key.split(":")
+ rpc_key, _ = base.split_random_key(key)
handle = self._server_pool[key]
del self._server_pool[key]
new_key = base.random_key(rpc_key + ":", keyset)
Review Comment:
thanks. updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on a diff in pull request #13420: [RPC] Fix tracker connection termination
Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13420:
URL: https://github.com/apache/tvm/pull/13420#discussion_r1025656941
##########
tests/python/unittest/test_runtime_rpc.py:
##########
@@ -448,10 +448,10 @@ def check_remote():
@tvm.testing.requires_rpc
-def test_rpc_tracker_register():
+@pytest.mark.parametrize("device_key", ["test_device", "127.0.0.1:5555"])
Review Comment:
Is it a complete list with all possible keys which `adb devices` can return? Not clear from this added key, why previous version with split doesn't work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Icemist commented on a diff in pull request #13420: [RPC] Fix tracker connection termination
Posted by GitBox <gi...@apache.org>.
Icemist commented on code in PR #13420:
URL: https://github.com/apache/tvm/pull/13420#discussion_r1026401405
##########
tests/python/unittest/test_runtime_rpc.py:
##########
@@ -448,10 +448,10 @@ def check_remote():
@tvm.testing.requires_rpc
-def test_rpc_tracker_register():
+@pytest.mark.parametrize("device_key", ["test_device", "127.0.0.1:5555"])
Review Comment:
> What is the problem to add one more key, e.g. hexagon-dev.192.168.0.143:5555:0.513619?
It is not device key, it is internal representation inside implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Icemist commented on a diff in pull request #13420: [RPC] Fix tracker connection termination
Posted by GitBox <gi...@apache.org>.
Icemist commented on code in PR #13420:
URL: https://github.com/apache/tvm/pull/13420#discussion_r1026401405
##########
tests/python/unittest/test_runtime_rpc.py:
##########
@@ -448,10 +448,10 @@ def check_remote():
@tvm.testing.requires_rpc
-def test_rpc_tracker_register():
+@pytest.mark.parametrize("device_key", ["test_device", "127.0.0.1:5555"])
Review Comment:
> What is the problem to add one more key, e.g. hexagon-dev.192.168.0.143:5555:0.513619?
It is not device key, it is internal representation inside implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on a diff in pull request #13420: [RPC] Fix tracker connection termination
Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13420:
URL: https://github.com/apache/tvm/pull/13420#discussion_r1025986836
##########
tests/python/unittest/test_runtime_rpc.py:
##########
@@ -448,10 +448,10 @@ def check_remote():
@tvm.testing.requires_rpc
-def test_rpc_tracker_register():
+@pytest.mark.parametrize("device_key", ["test_device", "127.0.0.1:5555"])
Review Comment:
Hm... It looks strange that the test has failed before with "127.0.0.1:5555". I wrote the following code:
```python
key = "127.0.0.1:5555"
print(key.split(":")[0])
print(key.rsplit(":", 1)[0])
```
And expectedly received the following result:
```
127.0.0.1
127.0.0.1
```
What is the problem to add one more key, e.g. `hexagon-dev.192.168.0.143:5555:0.513619`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on a diff in pull request #13420: [RPC] Fix tracker connection termination
Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13420:
URL: https://github.com/apache/tvm/pull/13420#discussion_r1027592851
##########
python/tvm/rpc/base.py:
##########
@@ -136,13 +139,30 @@ def random_key(prefix, cmap=None):
key : str
The generated random key
"""
- if cmap:
- while True:
- key = prefix + str(random.random())
- if key not in cmap:
- return key
- else:
- return prefix + str(random.random())
+ while True:
+ key = "{}{}{}".format(prefix, delimiter, random.random())
+ if not cmap or key not in cmap:
+ break
Review Comment:
Is it expected that if the `key` is found in `cmap` then on the next iteration it contains more delimiters than only one?
E.g.:
- First iteration:
```
prefix = "127.0.0.1:5555"
key = "127.0.0.1:5555:123456"
```
`cmap` has already contained this key `127.0.0.1:5555:123456`. We will iterate once again.
- Second iteration:
```
key = "127.0.0.1:5555:123456:654321"
```
If we split this key after the second iteration, then we get `127.0.0.1:5555:123456` but it is not the key.
Probably this change will help us to avoid this problem?
```suggestion
key = "{}{}{}".format(prefix, delimiter, random.random())
if not cmap or key not in cmap:
break
delimiter = ""
```
Or maybe I missed something and everything should work?
##########
python/tvm/rpc/proxy.py:
##########
@@ -319,7 +319,7 @@ def _regenerate_server_keys(self, keys):
new_keys = []
# re-generate the server match key, so old information is invalidated.
for key in keys:
- rpc_key, _ = key.split(":")
+ rpc_key, _ = base.split_random_key(key)
handle = self._server_pool[key]
del self._server_pool[key]
new_key = base.random_key(rpc_key + ":", keyset)
Review Comment:
Should we also update this line? Because variable `keyset` is not on the right place in the list of arguments and I expect that after first iteration we will have something like that: `127.0.0.1:5555::123456`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org