Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CAS client API refactor; Shard and CAS object format changes to use chunk indices #29

Merged
merged 21 commits into from
Oct 4, 2024
Merged

CAS client API refactor; Shard and CAS object format changes to use chunk indices #29

merged 21 commits into from
Oct 4, 2024

Conversation

seanses
Copy link
Collaborator

@seanses seanses commented Sep 27, 2024 •
edited
Loading

  • Delete grpc cas and shard code.
  • Rewrite CAS client api as there will be no API to get cas block by hash.
  • Delete CachingClient and StagingClient (need to rewrite them in the future).
  • Use chunk indices in FileDataSequenceEntry and cas object format.
  • Misc code clean up and reorganization.

Related server-side PR:

@seanses seanses changed the title [WIP] Shard and CAS object format change to use chunk indices [WIP] Shard and CAS object format change to use chunk indices; Rewrites CAS client API Sep 27, 2024
seanses and others added 10 commits September 27, 2024 16:21
@seanses
update
0eb6c31
@rajatarya
Making old validate hash function private
45d0a3e
@rajatarya
Partial CasObject changes
8e6e84a
@rajatarya
Update CasObject format v3
682ff7a 
- Removed CasChunkInfo
- Now tracking chunk boundary offsets & chunk hashes
@rajatarya
Updating comments
e483528
@rajatarya
Reformatted code, add validation
0d4f2e3 
- Moved static functions before methods.
- Moved pub functions & methods before non-public functions.
- Added CasObjectInfo basic validation on object methods.
@rajatarya
Basic generate_chunk_range_hash
951a464 
- includes basic unit-test
- created validate_cas_object_info method
- todos for additional tests to write
@seanses
more updates and api changes
8b5f344
@seanses
Merge branch 'main' into di/use-chunk-index
1b3d88d
@seanses
fix bugs and clean up
71348d2
@seanses seanses marked this pull request as ready for review October 2, 2024 21:24
@seanses seanses changed the title [WIP] Shard and CAS object format change to use chunk indices; Rewrites CAS client API CAS client API; Shard and CAS object format changes to use chunk indices Oct 2, 2024
@seanses seanses changed the title CAS client API; Shard and CAS object format changes to use chunk indices CAS client API refactor; Shard and CAS object format changes to use chunk indices Oct 2, 2024
@seanses seanses requested review from hoytak and rajatarya October 2, 2024 21:33
@seanses seanses requested a review from assafvayner October 2, 2024 22:34
rajatarya
rajatarya reviewed Oct 3, 2024
View reviewed changes
Copy link
Collaborator

@rajatarya rajatarya left a comment

Choose a reason for hiding this comment

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

Really only one comment, this code looks good and I appreciate the comments and restructuring of files.

cas_client/src/remote_client.rs Show resolved Hide resolved
shard_client/src/http_shard_client.rs Show resolved Hide resolved
utils/src/singleflight.rs Outdated Show resolved Hide resolved
hoytak
hoytak approved these changes Oct 4, 2024
View reviewed changes
Copy link
Collaborator

@hoytak hoytak left a comment

Choose a reason for hiding this comment

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

LGTM!

@seanses seanses merged commit 55973a1 into main Oct 4, 2024
1 check passed
@seanses seanses deleted the di/use-chunk-index branch October 4, 2024 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@assafvayner assafvayner assafvayner left review comments

@rajatarya rajatarya rajatarya left review comments

@hoytak hoytak hoytak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@seanses @hoytak @rajatarya @assafvayner