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

Rewrite internal logic of block sync client #2715

Merged
merged 17 commits into from
Aug 6, 2020
Merged

Rewrite internal logic of block sync client #2715

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
76d40ec
first pass, ready for review
schomatis Jul 27, 2020
0d77c96
extend maximum to fork length
schomatis Jul 30, 2020
ccaca48
temporarily make MaxRequestLength a var
schomatis Jul 31, 2020
0e4d5cb
document CompactedMessages
schomatis Jul 31, 2020
3f6c418
rename validatedResponse to private
schomatis Jul 31, 2020
97b3747
extra check in toFullTipSets
schomatis Jul 31, 2020
755772e
review client comments
schomatis Jul 31, 2020
fc32410
review server
schomatis Jul 31, 2020
c0c692f
fix cbor-gen
schomatis Aug 3, 2020
9f82e98
check parent height
schomatis Aug 4, 2020
1ec0ded
document NewTipSet checks
schomatis Aug 4, 2020
7982ab5
check connection between fetched segments during sync
schomatis Aug 4, 2020
6551219
clear deadline in server
schomatis Aug 5, 2020
8e044be
protocol: req: use TipSetKey comment
schomatis Aug 5, 2020
ac25502
go fmt
schomatis Aug 5, 2020
2c43138
record global time in client
schomatis Aug 6, 2020
7216363
log success fixme
schomatis Aug 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions chain/blocksync/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,14 +378,12 @@ func (client *BlockSync) sendRequestToPeer(
_ = stream.SetWriteDeadline(time.Now().Add(WRITE_REQ_DEADLINE))
if err := cborutil.WriteCborRPC(stream, req); err != nil {
_ = stream.SetWriteDeadline(time.Time{})
// FIXME: What's the point of setting a blank deadline that won't time out?
// Is this our way of clearing the old one?
client.peerTracker.logFailure(peer, build.Clock.Since(connectionStart))
// FIXME: Should we also remove peer here?
Copy link
Member

Choose a reason for hiding this comment

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

i currently assume the peer tracker won't try the peer again unless they are our only other option

return nil, err
}
// FIXME: Same, why are we doing this again here?
_ = stream.SetWriteDeadline(time.Time{})
_ = stream.SetWriteDeadline(time.Time{}) // clear deadline // FIXME: Needs
// its own API (/~https://github.com/libp2p/go-libp2p-core/issues/162).
Copy link
Contributor

Choose a reason for hiding this comment

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

Clearing Deadline by empty time is the Go's standard, I agree specific API might be nice.


// Read response.
var res Response
Expand Down
2 changes: 2 additions & 0 deletions chain/blocksync/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,12 @@ func (server *BlockSyncService) HandleStream(stream inet.Stream) {

_ = stream.SetDeadline(time.Now().Add(WRITE_RES_DEADLINE))
Copy link
Member

Choose a reason for hiding this comment

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

need to clear the deadline too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in 6551219

if err := cborutil.WriteCborRPC(stream, resp); err != nil {
_ = stream.SetDeadline(time.Time{})
log.Warnw("failed to write back response for handle stream",
"err", err, "peer", stream.Conn().RemotePeer())
return
}
_ = stream.SetDeadline(time.Time{})
}

// Validate and service the request. We return either a protocol
Expand Down