[fix] Fix custom_backend_meta related issue#47
Conversation
CLA Signature Passdpj135, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
Signed-off-by: dpj135 <958208521@qq.com>
CLA Signature Passdpj135, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
|
cc @0oshowero0 |
There was a problem hiding this comment.
Pull request overview
This PR improves key/value backend put_data correctness when writing partial/new fields by deriving key generation from the actual TensorDict contents (rather than metadata.field_names), and updates E2E lifecycle coverage to exercise additional backend/config and updated metadata-fetch flow.
Changes:
- Add batch-size consistency validation and generate KV keys using sorted
data.keys()inKVStorageManager.put_data. - Map returned
custom_backend_metausing the data-derived field ordering. - Extend E2E backend configurations with a
Yuanrongoption and adjust the cross-shard update test to re-fetch metadata for newly added fields.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| transfer_queue/storage/managers/base.py | Generate KV keys from TensorDict fields and add batch-size validation in put_data; adjust custom-backend-meta mapping accordingly. |
| tests/e2e/test_e2e_lifecycle_consistency.py | Add Yuanrong backend config and refactor metadata selection logic for new-field verification in cross-shard updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "Yuanrong": { | ||
| "controller": { | ||
| "polling_mode": True, | ||
| }, | ||
| "backend": { | ||
| "storage_backend": "Yuanrong", | ||
| "Yuanrong": { | ||
| "host": "127.0.0.1", | ||
| "port": 31501, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Ignore it temporarily
| }, | ||
| }, | ||
| }, | ||
| "Yuanrong": { |
There was a problem hiding this comment.
Please add this test to github actions
There was a problem hiding this comment.
There was a problem hiding this comment.
In currently, Yuanrong-datasystem need to be started by hands. We plan to support automatic startup of Yuanrong for tq.init() in next PR.
Signed-off-by: dpj135 <958208521@qq.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CLA Signature Guide@dpj135 , thanks for your pull request. The following commit(s) are not associated with a signed Contributor License Agreement (CLA).
To sign CLA, click here. To check if your email is configured correctly, refer to the FAQs. Once you've signed the CLA or updating your email, please comment |
1 similar comment
CLA Signature Guide@dpj135 , thanks for your pull request. The following commit(s) are not associated with a signed Contributor License Agreement (CLA).
To sign CLA, click here. To check if your email is configured correctly, refer to the FAQs. Once you've signed the CLA or updating your email, please comment |
Signed-off-by: dpj135 <958208521@qq.com>
CLA Signature Guide@dpj135 , thanks for your pull request. The following commit(s) are not associated with a signed Contributor License Agreement (CLA).
To sign CLA, click here. To check if your email is configured correctly, refer to the FAQs. Once you've signed the CLA or updating your email, please comment |
custom_backend_meta related issue
Description
Running
TQ_TEST_BACKEND=Yuanrong pytest -s tests/e2e/test_e2e_lifecycle_consistency.pyraise error.Main Changes
custom_backend_metaofextended_metawas missing intest_e2e_lifecycle_consistency.py::test_cross_shard_complex_update.KVStorageManager::put_dataused outdatedfield_namesof metadata.KVStorageManager::put_data.Other Infos
clear_partitionstill raises error intest_e2e_lifecycle_consistency.py::test_cross_shard_complex_update, because some keys generated by metadata were not put before.tq.init().