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.

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

Dont double-count queued / sent / received data when channel restarted #140

Merged
merged 2 commits into from
Feb 1, 2021

Conversation

dirkmc
Copy link
Contributor

@dirkmc dirkmc commented Jan 26, 2021

The data transfer module subscribes to hooks called by graphsync when a block is queued or sent.
When a data transfer is restarted, Graphsync calls these hooks again with all the blocks that have already been queued and sent, with a BlockSizeOnWire() of zero.

This PR modifies event handling such that

  • DataSent and DataQueued are only fired if block.BlockSizeOnWire() is non-zero
  • DataQueuedProgress, DataSentProgress and DataReceivedProgress are fired only the first time a block is queued / sent / received
  • The data transfer channel Queued, Sent and Received values are only increased for new blocks. eg if a block is sent twice it is not added to these values the second time.

In order to keep track of which blocks have been queued / sent / received, a new class has been added: CIDSetManager.
Each channel has a cid set for queued / sent / received.

@dirkmc dirkmc changed the title Check total blocks sent when theres a restart Dont double-count queued / sent / received blocks per channel Jan 29, 2021
@dirkmc dirkmc changed the title Dont double-count queued / sent / received blocks per channel Dont double-count queued / sent / received data when channel restarted Jan 29, 2021
Copy link
Collaborator

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

LGTM

// When a data transfer is restarted, the requester sends a list of CIDs
// that it already has. Graphsync calls the sent hook for all blocks even
// if they are in the list (meaning, they aren't actually sent over the
// wire). So here we check if the block was actually sent
Copy link
Collaborator

Choose a reason for hiding this comment

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

Given your CIDsets functionality this check is probably not necessary, FWIW.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It may make for greater optimization though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants