-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add a DAG walker with support for IPLD Node
s
#39
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,149 @@ | ||
package format | ||
|
||
import ( | ||
"context" | ||
|
||
cid "github.com/ipfs/go-cid" | ||
) | ||
|
||
// NavigableIPLDNode implements the `NavigableNode` interface wrapping | ||
// an IPLD `Node` and providing support for node promises. | ||
type NavigableIPLDNode struct { | ||
node Node | ||
|
||
// The CID of each child of the node. | ||
childCIDs []cid.Cid | ||
|
||
// Node promises for child nodes requested. | ||
childPromises []*NodePromise | ||
// TODO: Consider encapsulating it in a single structure alongside `childCIDs`. | ||
|
||
nodeGetter NodeGetter | ||
// TODO: Should this be stored in the `Walker`'s context to avoid passing | ||
// it along to every node? It seems like a structure that doesn't need | ||
// to be replicated (the entire DAG will use the same `NodeGetter`). | ||
} | ||
|
||
// NewNavigableIPLDNode returns a `NavigableIPLDNode` wrapping the provided | ||
// `node`. | ||
func NewNavigableIPLDNode(node Node, nodeGetter NodeGetter) *NavigableIPLDNode { | ||
nn := &NavigableIPLDNode{ | ||
node: node, | ||
nodeGetter: nodeGetter, | ||
} | ||
|
||
nn.childCIDs = getLinkCids(node) | ||
nn.childPromises = make([]*NodePromise, len(nn.childCIDs)) | ||
|
||
return nn | ||
} | ||
|
||
// FetchChild implements the `NavigableNode` interface using node promises | ||
// to preload the following child nodes to `childIndex` leaving them ready | ||
// for subsequent `FetchChild` calls. | ||
func (nn *NavigableIPLDNode) FetchChild(ctx context.Context, childIndex uint) (NavigableNode, error) { | ||
// This function doesn't check that `childIndex` is valid, that's | ||
// the `Walker` responsibility. | ||
|
||
// If we drop to <= preloadSize/2 preloading nodes, preload the next 10. | ||
for i := childIndex; i < childIndex+preloadSize/2 && i < uint(len(nn.childPromises)); i++ { | ||
// TODO: Check if canceled. | ||
if nn.childPromises[i] == nil { | ||
nn.preload(ctx, i) | ||
break | ||
} | ||
} | ||
|
||
child, err := nn.getPromiseValue(ctx, childIndex) | ||
|
||
switch err { | ||
case nil: | ||
case context.DeadlineExceeded, context.Canceled: | ||
if ctx.Err() != nil { | ||
return nil, ctx.Err() | ||
} | ||
|
||
// In this case, the context used to *preload* the node (in a previous | ||
// `FetchChild` call) has been canceled. We need to retry the load with | ||
// the current context and we might as well preload some extra nodes | ||
// while we're at it. | ||
nn.preload(ctx, childIndex) | ||
child, err = nn.getPromiseValue(ctx, childIndex) | ||
if err != nil { | ||
return nil, err | ||
} | ||
default: | ||
return nil, err | ||
} | ||
|
||
return NewNavigableIPLDNode(child, nn.nodeGetter), nil | ||
} | ||
|
||
// Number of nodes to preload every time a child is requested. | ||
// TODO: Give more visibility to this constant, it could be an attribute | ||
// set in the `Walker` context that gets passed in `FetchChild`. | ||
const preloadSize = 10 | ||
|
||
// Preload at most `preloadSize` child nodes from `beg` through promises | ||
// created using this `ctx`. | ||
func (nn *NavigableIPLDNode) preload(ctx context.Context, beg uint) { | ||
end := beg + preloadSize | ||
if end >= uint(len(nn.childCIDs)) { | ||
end = uint(len(nn.childCIDs)) | ||
} | ||
|
||
copy(nn.childPromises[beg:], GetNodes(ctx, nn.nodeGetter, nn.childCIDs[beg:end])) | ||
} | ||
|
||
// Fetch the actual node (this is the blocking part of the mechanism) | ||
// and invalidate the promise. `preload` should always be called first | ||
// for the `childIndex` being fetch. | ||
// | ||
// TODO: Include `preload` into the beginning of this function? | ||
// (And collapse the two calls in `FetchChild`). | ||
func (nn *NavigableIPLDNode) getPromiseValue(ctx context.Context, childIndex uint) (Node, error) { | ||
value, err := nn.childPromises[childIndex].Get(ctx) | ||
nn.childPromises[childIndex] = nil | ||
return value, err | ||
} | ||
|
||
// Get the CID of all the links of this `node`. | ||
func getLinkCids(node Node) []cid.Cid { | ||
links := node.Links() | ||
out := make([]cid.Cid, 0, len(links)) | ||
|
||
for _, l := range links { | ||
out = append(out, l.Cid) | ||
} | ||
return out | ||
} | ||
|
||
// GetIPLDNode returns the IPLD `Node` wrapped into this structure. | ||
func (nn *NavigableIPLDNode) GetIPLDNode() Node { | ||
return nn.node | ||
} | ||
|
||
// ChildTotal implements the `NavigableNode` returning the number | ||
// of links (of child nodes) in this node. | ||
func (nn *NavigableIPLDNode) ChildTotal() uint { | ||
return uint(len(nn.GetIPLDNode().Links())) | ||
} | ||
|
||
// ExtractIPLDNode is a helper function that takes a `NavigableNode` | ||
// and returns the IPLD `Node` wrapped inside. Used in the `Visitor` | ||
// function. | ||
// TODO: Check for errors to avoid a panic? | ||
func ExtractIPLDNode(node NavigableNode) Node { | ||
return node.(*NavigableIPLDNode).GetIPLDNode() | ||
} | ||
|
||
// TODO: `Cleanup` is not supported at the moment in the `Walker`. | ||
// | ||
// Called in `Walker.up()` when the node is not part of the path anymore. | ||
//func (nn *NavigableIPLDNode) Cleanup() { | ||
// // TODO: Ideally this would be the place to issue a context `cancel()` | ||
// // but since the DAG reader uses multiple contexts in the same session | ||
// // (through `Read` and `CtxReadFull`) we would need to store an array | ||
// // with the multiple contexts in `NavigableIPLDNode` with its corresponding | ||
// // cancel functions. | ||
//} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
If I’m reading right, it seems like it would be easy to wind up preloading a too many nodes by calling
FetchChild()
out of order. For example, if I had a node with 30 children and calledFetchChild(ctx, 20)
followed byFetchChild(ctx, 10)
, this check would act as if nothing was preloading at all and start loading children 10-19, even though it’s already loading children 20-29.Not sure how big a concern that is right now, but if the eventual future of
Walker
is async, it doesn’t seem like we should expect ordering to be reliable here.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.
Yes, this logic was inherited from the DAG reader and I need @Stebalien to check it.