[c10d] Added PrefixStore, pybind, test for group backward compatibility #10762
Conversation
teng-li
added
the
oncall: distributed
There was a problem hiding this comment.
I think we should approach this differently. Instead of each store itself supporting a prefix, we can add a PrefixStore that wraps an existing store and adds the prefix to the key for every function call. This is the same approach as taken in gloo itself and allows for reuse of the underlying store. For example the TCP store won't start twice on the same address, but if you keep the original one around and create N prefix stores on top of it, they'll all share the same underlying TCP store.
torch/lib/c10d/FileStore.cpp
Outdated
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
@pietern good point, let me add a prefix store for it |
teng-li
changed the title
pietern
changed the title
|
@pytorchbot retest this please |
teng-li
changed the title
|
@pytorchbot retest this please |
1 similar comment
|
@pytorchbot retest this please |
teng-li
changed the title
There was a problem hiding this comment.
teng-li has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@pietern stamp? |
…orch#10762) Summary: Added Prefix Store support. This will make group be backward compatible. Test is covered too. ``` tengli@devfair033:~/new_pytorch/pytorch/torch/lib/build/c10d/test$ ./FileStoreTest Using temporary file: /tmp/testoglRl4 Using temporary file: /tmp/testepZIpB Test succeeded tengli@devfair033:~/new_pytorch/pytorch/torch/lib/build/c10d/test$ ./TCPStoreTest Test succeeded ``` Pull Request resolved: pytorch#10762 Differential Revision: D9484032 Pulled By: teng-li fbshipit-source-id: 85754af91fe3f5605087c4a2f79ae930a9fd1387
4 participants
Added Prefix Store support.
This will make group be backward compatible.
Test is covered too.