Skip to content
This repository has been archived by the owner on Jun 26, 2023. It is now read-only.

Commit

Permalink
merge closeChildUpdate with flushCurrentNode
Browse files Browse the repository at this point in the history
Without the `sync` logic (now removed) the code is simplified for these tightly
coupled methods (the first one was the only caller of the second) to be merged
in a single method, correctly redistributing the use of the lock.
  • Loading branch information
schomatis committed Dec 21, 2018
1 parent 1bbc52d commit 1226f22
Showing 1 changed file with 10 additions and 17 deletions.
27 changes: 10 additions & 17 deletions dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ func (d *Directory) updateChildEntry(c child) error {

// This method implements the part of `updateChildEntry` that needs
// to be locked around: in charge of updating the UnixFS layer and
// generating the new node reflecting the update.
// generating the new node reflecting the update. It also stores the
// new node in the DAG layer.
func (d *Directory) closeChildUpdate(c child) (*dag.ProtoNode, error) {
d.lock.Lock()
defer d.lock.Unlock()

err := d.updateChild(c)
if err != nil {
Expand All @@ -116,31 +116,24 @@ func (d *Directory) closeChildUpdate(c child) (*dag.ProtoNode, error) {
// TODO: Clearly define how are we propagating changes to lower layers
// like UnixFS.

return d.flushCurrentNode()
}

// Recreate the underlying UnixFS directory node and save it in the DAG layer.
func (d *Directory) flushCurrentNode() (*dag.ProtoNode, error) {
nd, err := d.unixfsDir.GetNode()
if err != nil {
return nil, err
}

err = d.dagService.Add(d.ctx, nd)
if err != nil {
return nil, err
}
// TODO: This method is called in `closeChildUpdate` while the lock is
// taken, we need the lock while operating on `unixfsDir` to create the
// new node but do we also need to keep the lock while adding it to the
// DAG service? Evaluate refactoring these two methods together and better
// redistributing the node.
d.lock.Unlock()
// TODO: This part of the method locked around could be now extracted
// separately.

pbnd, ok := nd.(*dag.ProtoNode)
if !ok {
return nil, dag.ErrNotProtobuf
}
// TODO: Why do we check the node *after* adding it to the DAG service?

err = d.dagService.Add(d.ctx, nd)
if err != nil {
return nil, err
}

return pbnd.Copy().(*dag.ProtoNode), nil
// TODO: Why do we need a copy?
Expand Down

0 comments on commit 1226f22

Please sign in to comment.