refactor merkledag fetching methods #2384
Conversation
License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>
|
this should also make pinning suck less (hopefully) |
|
That's nice. It might also be interesting to deprecate the AddRecursive() and RemoveRecursive() methods to use something like this instead, but what you did was a good first step. |
|
@mildred oh yeah, those recursive methods arent even used, are they? |
License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>
|
mmm, this ones not quite ready yet. now that things can fail instead of hang, its flushing out new failures in the pinning sharness tests: https://travis-ci.org/ipfs/go-ipfs/jobs/110680509 |
|
The |
| return | ||
| } | ||
| select { | ||
| case out <- nd: |
There was a problem hiding this comment.
This could be a common go-ism, but humour me: why not use a buffered channel for out? This secondary select would be unnecessary.
There was a problem hiding this comment.
hrm, i guess the improved simplicity there would outweigh any performance gains this might have. I generally avoid buffering channels, but it makes sense here
ghost
mentioned this pull request
License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>
| case b, ok := <-blocks: | ||
| if !ok { | ||
| if count != len(keys) { | ||
| errs <- fmt.Errorf("failed to fetch all nodes") |
There was a problem hiding this comment.
go's error string convention is lower case with no punctuation: https://github.com/golang/lint/blob/master/lint.go#L1173
|
🐴 LGTM |
License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>
|
Looks good |
refactor merkledag fetching methods
I refactored the dagservice have one new
GetManymethod that just mimics the interface of the blockservices similar method. It takes some keys and returns a channel of nodes in no particular order.I then removed the
GetDAGandGetNodesmethods from the dagservice interface and refactored them toGetMany.finally, I refactored
FetchGraphto useGetMany, and in doing so, it has better error cases (can actually fail through from the blockservice layer) and uses wayyyy fewer goroutines. (though still a good number of them).cc @noffle @lgierth for CR
cc @mildred since youre working on similar cleanup type stuff