Skip to content

Conversation

@rohan-varma
Copy link
Contributor

@rohan-varma rohan-varma commented Oct 23, 2019

Stack from ghstack:

Closes #28124. This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.

Differential Revision: D18079212

…ned up in

the case of nested rpcs

This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.

Differential Revision: [D18079212](https://our.internmc.facebook.com/intern/diff/D18079212/)

[ghstack-poisoned]
rohan-varma added a commit that referenced this pull request Oct 23, 2019
…ned up in

the case of nested rpcs

This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.

Differential Revision: [D18079212](https://our.internmc.facebook.com/intern/diff/D18079212/)

ghstack-source-id: 92433816
Pull Request resolved: #28485
…ts are cleaned up in

the case of nested rpcs"


This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.

Differential Revision: [D18079212](https://our.internmc.facebook.com/intern/diff/D18079212/)

[ghstack-poisoned]
…ts are cleaned up in

the case of nested rpcs"

the case of nested rpcs**

This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.

Differential Revision: [D18079212](https://our.internmc.facebook.com/intern/diff/D18079212/)

[ghstack-poisoned]
…ts are cleaned up in

the case of nested rpcs"

the case of nested rpcs**

Closes #28124. This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.

Differential Revision: [D18079212](https://our.internmc.facebook.com/intern/diff/D18079212/)

[ghstack-poisoned]
…ts are cleaned up in

the case of nested rpcs"

the case of nested rpcs**
the case of nested rpcs**

Closes #28124. This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.

Differential Revision: [D18079212](https://our.internmc.facebook.com/intern/diff/D18079212/)

[ghstack-poisoned]
rohan-varma added a commit that referenced this pull request Oct 23, 2019
…ned up incase of nested rpcs

Pull Request resolved: #28485

This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.
ghstack-source-id: 92440587

Differential Revision: [D18079212](https://our.internmc.facebook.com/intern/diff/D18079212/)
…ts are cleaned up in

the case of nested rpcs"



Closes #28124. This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.

Differential Revision: [D18079212](https://our.internmc.facebook.com/intern/diff/D18079212/)

[ghstack-poisoned]
rohan-varma added a commit that referenced this pull request Oct 24, 2019
…ned up incase of nested rpcs

Pull Request resolved: #28485

This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.
ghstack-source-id: 92527553

Differential Revision: [D18079212](https://our.internmc.facebook.com/intern/diff/D18079212/)
Copy link
Contributor

@zhaojuanmao zhaojuanmao left a comment

Choose a reason for hiding this comment

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

overall look good, although feel like we can merge _set_rpc_done and _store_context_id and clean them up

…ts are cleaned up in

the case of nested rpcs"

the case of nested rpcs**


Closes #28124. This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.

Differential Revision: [D18079212](https://our.internmc.facebook.com/intern/diff/D18079212/)

[ghstack-poisoned]
…ts are cleaned up in

the case of nested rpcs"

the case of nested rpcs**
the case of nested rpcs**


Closes #28124. This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.

Differential Revision: [D18079212](https://our.internmc.facebook.com/intern/diff/D18079212/)

[ghstack-poisoned]
rohan-varma added a commit that referenced this pull request Oct 24, 2019
…ned up incase of nested rpcs

Pull Request resolved: #28485

This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.
ghstack-source-id: 92589157

Differential Revision: [D18079212](https://our.internmc.facebook.com/intern/diff/D18079212/)
@rohan-varma
Copy link
Contributor Author

@zhaojuanmao updated the PR to remove _store_context_id as suggested

…ts are cleaned up in

the case of nested rpcs"


Closes #28124. This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.

Differential Revision: [D18079212](https://our.internmc.facebook.com/intern/diff/D18079212/)

[ghstack-poisoned]
…ts are cleaned up in

the case of nested rpcs"

the case of nested rpcs**

Closes #28124. This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.

Differential Revision: [D18079212](https://our.internmc.facebook.com/intern/diff/D18079212/)

[ghstack-poisoned]
rohan-varma added a commit that referenced this pull request Oct 25, 2019
…ned up incase of nested rpcs

Pull Request resolved: #28485

This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.
ghstack-source-id: 92611018

Differential Revision: [D18079212](https://our.internmc.facebook.com/intern/diff/D18079212/)
@zhaojuanmao
Copy link
Contributor

sounds good!

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 4242385.

@facebook-github-bot facebook-github-bot deleted the gh/rohan-varma/24/head branch October 28, 2019 22:19
thiagocrepaldi pushed a commit to thiagocrepaldi/pytorch that referenced this pull request Feb 4, 2020
…of nested rpcs (pytorch#28485)

Summary:
Pull Request resolved: pytorch#28485

This diff adds a test to ensure that when we have multiple nested RPCs
inside a dist autograd context, the context that is created as a result of a
nested rpc is cleaned up after the node creating the context exits the context
manager. For example, worker 0 might send an rpc to worker 1 that results in an
rpc to worker 2, so worker 2 will have 0's context, even though worker 0 never
directly talked to 2. This test ensures that the context on 2 would also be
cleaned up.
ghstack-source-id: 92611018

Test Plan: Ran the unit test.

Differential Revision: D18079212

fbshipit-source-id: d49f0cda0bf2908747546e5c8a967256c848c685
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants