Clean up RemoteCache classes #134032
Closed
Clean up RemoteCache classes #134032
Conversation
This file contains hidden or 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
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/134032
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit f972b3b with merge base 5dad6a5 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
This pull request was exported from Phabricator. Differential Revision: D61178859 |
|
This pull request was exported from Phabricator. Differential Revision: D61178859 |
aorenste
force-pushed
the
export-D61178859
branch
from
d16a589 to
c3e2b73
Compare
aorenste
added a commit
to aorenste/pytorch
that referenced
this pull request
Aug 21, 2024
Summary: Pull Request resolved: pytorch#134032 The existing RemoteCacheBackend classes were a bit haphazard - some of them accepted bytes only, some accepted objects, some returned different types of objects than were passed in. Update them to be more consistent: 1. RemoteCacheBackend is an implementation of a backend: Redis, Memcache, Manifold, LocalFile 2. RemoteCacheSerde is an implementation of a serde protocol - to turn structured objects (dict, list, etc) into bytes: RemoteCacheJsonSerde (json encoding), RemoteCachePassthroughSerde (strictly bytes only) 3. RemoteCache is the cache implementation itself, mixing a RemoteCacheBackend along with an RemoteCacheSerde to provide structured caching. Other than simply reorganizing the existing cache code this also fixes the Redis autotune caching for OSS. Test Plan: unit tests Reviewed By: oulgen Differential Revision: D61178859
oulgen
approved these changes
Aug 27, 2024
|
This was suggest for |
That does seem like a problem fixed by this PR - but the PR isn't ready to land yet (hoping today or tomorrow) so definitely don't cherry-pick it until it lands. |
|
This pull request was exported from Phabricator. Differential Revision: D61178859 |
aorenste
force-pushed
the
export-D61178859
branch
from
c3e2b73 to
38bf864
Compare
|
This pull request was exported from Phabricator. Differential Revision: D61178859 |
aorenste
force-pushed
the
export-D61178859
branch
from
38bf864 to
fac8a91
Compare
aorenste
force-pushed
the
export-D61178859
branch
from
fac8a91 to
57cebcf
Compare
|
This pull request was exported from Phabricator. Differential Revision: D61178859 |
Yep, it does, I confirm the issue So I hope this PR fixes it! |
bhack
reviewed
Aug 28, 2024
aorenste
force-pushed
the
export-D61178859
branch
from
57cebcf to
d9af9e6
Compare
|
This pull request was exported from Phabricator. Differential Revision: D61178859 |
1 similar comment
|
This pull request was exported from Phabricator. Differential Revision: D61178859 |
aorenste
force-pushed
the
export-D61178859
branch
from
d9af9e6 to
6fe3518
Compare
|
This pull request was exported from Phabricator. Differential Revision: D61178859 |
aorenste
force-pushed
the
export-D61178859
branch
from
1a10bb1 to
0514dd6
Compare
|
This pull request was exported from Phabricator. Differential Revision: D61178859 |
aorenste
force-pushed
the
export-D61178859
branch
from
0514dd6 to
e6a7d4d
Compare
|
|
|
Passes all tests - ready for review. |
|
Looks good to me, you can go ahead and land it, already approved |
bhack
pushed a commit
to bhack/pytorch
that referenced
this pull request
Aug 30, 2024
Summary: Pull Request resolved: pytorch#134032 The existing RemoteCacheBackend classes were a bit haphazard - some of them accepted bytes only, some accepted objects, some returned different types of objects than were passed in. Update them to be more consistent: 1. RemoteCacheBackend is an implementation of a backend: Redis, Memcache, Manifold, LocalFile 2. RemoteCacheSerde is an implementation of a serde protocol - to turn structured objects (dict, list, etc) into bytes: RemoteCacheJsonSerde (json encoding), RemoteCachePassthroughSerde (strictly bytes only) 3. RemoteCache is the cache implementation itself, mixing a RemoteCacheBackend along with an RemoteCacheSerde to provide structured caching. Other than simply reorganizing the existing cache code this also fixes the Redis autotune caching for OSS. Test Plan: unit tests Reviewed By: oulgen Differential Revision: D61178859
bhack
approved these changes
Aug 30, 2024
aorenste
force-pushed
the
export-D61178859
branch
from
e6a7d4d to
225945e
Compare
|
This pull request was exported from Phabricator. Differential Revision: D61178859 |
Summary: Pull Request resolved: pytorch#134032 The existing RemoteCacheBackend classes were a bit haphazard - some of them accepted bytes only, some accepted objects, some returned different types of objects than were passed in. Update them to be more consistent: 1. RemoteCacheBackend is an implementation of a backend: Redis, Memcache, Manifold, LocalFile 2. RemoteCacheSerde is an implementation of a serde protocol - to turn structured objects (dict, list, etc) into bytes: RemoteCacheJsonSerde (json encoding), RemoteCachePassthroughSerde (strictly bytes only) 3. RemoteCache is the cache implementation itself, mixing a RemoteCacheBackend along with an RemoteCacheSerde to provide structured caching. Other than simply reorganizing the existing cache code this also fixes the Redis autotune caching for OSS. Test Plan: unit tests Reviewed By: oulgen Differential Revision: D61178859
aorenste
force-pushed
the
export-D61178859
branch
from
225945e to
f972b3b
Compare
|
This pull request was exported from Phabricator. Differential Revision: D61178859 |
|
@pytorchbot merge -f 'Landed internally' (Initiating merge automatically since Phabricator Diff has merged, using force because this PR might not pass merge_rules.json but landed internally) |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
tolleybot
pushed a commit
to tolleybot/pytorch
that referenced
this pull request
Sep 14, 2024
Summary: The existing RemoteCacheBackend classes were a bit haphazard - some of them accepted bytes only, some accepted objects, some returned different types of objects than were passed in. Update them to be more consistent: 1. RemoteCacheBackend is an implementation of a backend: Redis, Memcache, Manifold, LocalFile 2. RemoteCacheSerde is an implementation of a serde protocol - to turn structured objects (dict, list, etc) into bytes: RemoteCacheJsonSerde (json encoding), RemoteCachePassthroughSerde (strictly bytes only) 3. RemoteCache is the cache implementation itself, mixing a RemoteCacheBackend along with an RemoteCacheSerde to provide structured caching. Other than simply reorganizing the existing cache code this also fixes the Redis autotune caching for OSS. Test Plan: unit tests Reviewed By: oulgen Differential Revision: D61178859 Pull Request resolved: pytorch#134032 Approved by: https://github.com/oulgen, https://github.com/bhack
Chao1Han
pushed a commit
to Chao1Han/pytorch
that referenced
this pull request
Sep 20, 2024
Summary: The existing RemoteCacheBackend classes were a bit haphazard - some of them accepted bytes only, some accepted objects, some returned different types of objects than were passed in. Update them to be more consistent: 1. RemoteCacheBackend is an implementation of a backend: Redis, Memcache, Manifold, LocalFile 2. RemoteCacheSerde is an implementation of a serde protocol - to turn structured objects (dict, list, etc) into bytes: RemoteCacheJsonSerde (json encoding), RemoteCachePassthroughSerde (strictly bytes only) 3. RemoteCache is the cache implementation itself, mixing a RemoteCacheBackend along with an RemoteCacheSerde to provide structured caching. Other than simply reorganizing the existing cache code this also fixes the Redis autotune caching for OSS. Test Plan: unit tests Reviewed By: oulgen Differential Revision: D61178859 Pull Request resolved: pytorch#134032 Approved by: https://github.com/oulgen, https://github.com/bhack
56 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
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.
Summary:
The existing RemoteCacheBackend classes were a bit haphazard - some of them accepted bytes only, some accepted objects, some returned different types of objects than were passed in.
Update them to be more consistent:
RemoteCacheBackend is an implementation of a backend: Redis, Memcache, Manifold, LocalFile
RemoteCacheSerde is an implementation of a serde protocol - to turn structured objects (dict, list, etc) into bytes: RemoteCacheJsonSerde (json encoding), RemoteCachePassthroughSerde (strictly bytes only)
RemoteCache is the cache implementation itself, mixing a RemoteCacheBackend along with an RemoteCacheSerde to provide structured caching.
Other than simply reorganizing the existing cache code this also fixes the Redis autotune caching for OSS.
Test Plan: unit tests
Reviewed By: oulgen
Differential Revision: D61178859
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang