-
Notifications
You must be signed in to change notification settings - Fork 30
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.
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
Enhancement/lp refactor download #753
Conversation
Vertical can have multiple blocks per read marker Horizontal can have only one block per read mearker
Define errors
Define more errors Refactor StreamDownld structure
Get refs with path hash and authticket as well
Few fixes and optimization Remove print statements
Remove rx pay
Remove rx-pay
return 0, err | ||
} | ||
|
||
offset := sd.offset % int64(effectiveChunkSize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what does the mod do here. And seems it could cause data lose. Imagine we request block_1, the offset is 109, and effectiveChunkSize is 100, then we will copy the data[9:]
in block_1 to b
. So where's the data[:9]
going.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peterlimg If client requests with offset 109, then it means client does not want or care about data[:9]
. So for example, if client already had stored 109 bytes of data then for requesting other data offset would be set to 109. Since data is requested in block level(100 bytes in above example), 109 bytes means client has stored 100 bytes from block_0 and 9 bytes from block_1, so now after getting block_1, it should skip data[:9]
otherwise data will be corrupted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Changes
Following changes are added:
Fixes
Tests
Tasks to complete before merging PR:
Associated PRs (Link as appropriate):