From 83601a1488d61e39fd3cbd5c991a0c6de5b08055 Mon Sep 17 00:00:00 2001 From: Pauline Middelink Date: Sun, 12 Mar 2017 15:53:52 +0100 Subject: [PATCH] Optimize by keeping node.ExtendedAttributes as map, not as list Make access to the extended attributes slightly easier by handeling them as a map (each attribute name can only exist once anyway). Handle backwards compatibility by unmarshaling of the old json data and converting to the new one when needed. Mark node.Content as optional in json data, to suppress "content":null fragments when not needed. It seems for a single check, the code is already Content == nil resistant. NOTICE: These json changes affect the content of a Treeblob and therefore its ID. So while a backup against a parent before this change will not store the content of dirs/files again, each directory will create a new treeblob. --- src/restic/checker/checker.go | 4 - src/restic/fuse/dir.go | 9 +- src/restic/fuse/file.go | 9 +- src/restic/node.go | 131 +++++++++-------------- src/restic/node_test.go | 23 ++++ src/restic/testdata/used_blobs_snapshot0 | 8 +- src/restic/testdata/used_blobs_snapshot1 | 6 +- 7 files changed, 91 insertions(+), 99 deletions(-) diff --git a/src/restic/checker/checker.go b/src/restic/checker/checker.go index 1fd73888c..a57a235e7 100644 --- a/src/restic/checker/checker.go +++ b/src/restic/checker/checker.go @@ -587,10 +587,6 @@ func (c *Checker) checkTree(id restic.ID, tree *restic.Tree) (errs []error) { for _, node := range tree.Nodes { switch node.Type { case "file": - if node.Content == nil { - errs = append(errs, Error{TreeID: id, Err: errors.Errorf("file %q has nil blob list", node.Name)}) - } - for b, blobID := range node.Content { if blobID.IsNull() { errs = append(errs, Error{TreeID: id, Err: errors.Errorf("file %q blob %d has null ID", node.Name, b)}) diff --git a/src/restic/fuse/dir.go b/src/restic/fuse/dir.go index 7968a2ee5..1805f24d3 100644 --- a/src/restic/fuse/dir.go +++ b/src/restic/fuse/dir.go @@ -180,17 +180,16 @@ func (d *dir) Lookup(ctx context.Context, name string) (fs.Node, error) { func (d *dir) Listxattr(ctx context.Context, req *fuse.ListxattrRequest, resp *fuse.ListxattrResponse) error { debug.Log("Listxattr(%v, %v)", d.node.Name, req.Size) - for _, attr := range d.node.ExtendedAttributes { - resp.Append(attr.Name) + for k := range d.node.ExtendedAttributes { + resp.Append(k) } return nil } func (d *dir) Getxattr(ctx context.Context, req *fuse.GetxattrRequest, resp *fuse.GetxattrResponse) error { debug.Log("Getxattr(%v, %v, %v)", d.node.Name, req.Name, req.Size) - attrval := d.node.GetExtendedAttribute(req.Name) - if attrval != nil { - resp.Xattr = attrval + if val, ok := d.node.ExtendedAttributes[req.Name]; ok { + resp.Xattr = val return nil } return fuse.ErrNoXattr diff --git a/src/restic/fuse/file.go b/src/restic/fuse/file.go index dfc8aa308..0222829e1 100644 --- a/src/restic/fuse/file.go +++ b/src/restic/fuse/file.go @@ -167,17 +167,16 @@ func (f *file) Release(ctx context.Context, req *fuse.ReleaseRequest) error { func (f *file) Listxattr(ctx context.Context, req *fuse.ListxattrRequest, resp *fuse.ListxattrResponse) error { debug.Log("Listxattr(%v, %v)", f.node.Name, req.Size) - for _, attr := range f.node.ExtendedAttributes { - resp.Append(attr.Name) + for k := range f.node.ExtendedAttributes { + resp.Append(k) } return nil } func (f *file) Getxattr(ctx context.Context, req *fuse.GetxattrRequest, resp *fuse.GetxattrResponse) error { debug.Log("Getxattr(%v, %v, %v)", f.node.Name, req.Name, req.Size) - attrval := f.node.GetExtendedAttribute(req.Name) - if attrval != nil { - resp.Xattr = attrval + if val, ok := f.node.ExtendedAttributes[req.Name]; ok { + resp.Xattr = val return nil } return fuse.ErrNoXattr diff --git a/src/restic/node.go b/src/restic/node.go index b634ed34b..30e933bf7 100644 --- a/src/restic/node.go +++ b/src/restic/node.go @@ -18,36 +18,29 @@ import ( "runtime" ) -// ExtendedAttribute is a tuple storing the xattr name and value. -type ExtendedAttribute struct { - Name string `json:"name"` - Value []byte `json:"value"` -} - // Node is a file, directory or other item in a backup. type Node struct { - Name string `json:"name"` - Type string `json:"type"` - Mode os.FileMode `json:"mode,omitempty"` - ModTime time.Time `json:"mtime,omitempty"` - AccessTime time.Time `json:"atime,omitempty"` - ChangeTime time.Time `json:"ctime,omitempty"` - UID uint32 `json:"uid"` - GID uint32 `json:"gid"` - User string `json:"user,omitempty"` - Group string `json:"group,omitempty"` - Inode uint64 `json:"inode,omitempty"` - Size uint64 `json:"size,omitempty"` - Links uint64 `json:"links,omitempty"` - LinkTarget string `json:"linktarget,omitempty"` - ExtendedAttributes []ExtendedAttribute `json:"extended_attributes,omitempty"` - Device uint64 `json:"device,omitempty"` - Content IDs `json:"content"` - Subtree *ID `json:"subtree,omitempty"` + Name string `json:"name"` + Type string `json:"type"` + Mode os.FileMode `json:"mode,omitempty"` + ModTime time.Time `json:"mtime,omitempty"` + AccessTime time.Time `json:"atime,omitempty"` + ChangeTime time.Time `json:"ctime,omitempty"` + UID uint32 `json:"uid"` + GID uint32 `json:"gid"` + User string `json:"user,omitempty"` + Group string `json:"group,omitempty"` + Inode uint64 `json:"inode,omitempty"` + Size uint64 `json:"size,omitempty"` + Links uint64 `json:"links,omitempty"` + LinkTarget string `json:"linktarget,omitempty"` + ExtendedAttributes map[string][]byte `json:"extended_attributes,omitempty"` + Device uint64 `json:"device,omitempty"` + Content IDs `json:"content,omitempty"` + Subtree *ID `json:"subtree,omitempty"` Error string `json:"error,omitempty"` - - Path string `json:"-"` + Path string `json:"-"` } func (node Node) String() string { @@ -106,10 +99,8 @@ func nodeTypeFromFileInfo(fi os.FileInfo) string { // GetExtendedAttribute gets the extended attribute. func (node Node) GetExtendedAttribute(a string) []byte { - for _, attr := range node.ExtendedAttributes { - if attr.Name == a { - return attr.Value - } + if v, ok := node.ExtendedAttributes[a]; ok { + return v } return nil } @@ -192,9 +183,8 @@ func (node Node) restoreMetadata(path string) error { } func (node Node) restoreExtendedAttributes(path string) error { - for _, attr := range node.ExtendedAttributes { - err := Setxattr(path, attr.Name, attr.Value) - if err != nil { + for name, value := range node.ExtendedAttributes { + if err := Setxattr(path, name, value); err != nil { return err } } @@ -333,11 +323,26 @@ func (node *Node) UnmarshalJSON(data []byte) error { type nodeJSON Node nj := (*nodeJSON)(node) - err := json.Unmarshal(data, nj) - if err != nil { - return errors.Wrap(err, "Unmarshal") + if err := json.Unmarshal(data, nj); err != nil { + // Retry conversion with list of ExtendedAttributes + var old struct { + nodeJSON + ExtendedAttributes []struct { + Name string + Value []byte + } `json:"extended_attributes,omitempty"` + } + if err := json.Unmarshal(data, &old); err != nil { + return errors.Wrap(err, "Unmarshal") + } + old.nodeJSON.ExtendedAttributes = make(map[string][]byte, len(old.ExtendedAttributes)) + for _, attr := range old.ExtendedAttributes { + old.nodeJSON.ExtendedAttributes[attr.Name] = attr.Value + } + *nj = old.nodeJSON } + var err error nj.Name, err = strconv.Unquote(`"` + nj.Name + `"`) return errors.Wrap(err, "Unquote") } @@ -439,45 +444,20 @@ func (node Node) sameExtendedAttributes(other Node) bool { if len(node.ExtendedAttributes) != len(other.ExtendedAttributes) { return false } - - // build a set of all attributes that node has - type mapvalue struct { - value []byte - present bool - } - attributes := make(map[string]mapvalue) - for _, attr := range node.ExtendedAttributes { - attributes[attr.Name] = mapvalue{value: attr.Value} - } - - for _, attr := range other.ExtendedAttributes { - v, ok := attributes[attr.Name] - if !ok { - // extended attribute is not set for node - debug.Log("other node has attribute %v, which is not present in node", attr.Name) - return false - - } - - if !bytes.Equal(v.value, attr.Value) { + for k, v := range node.ExtendedAttributes { + if w, ok := other.ExtendedAttributes[k]; !ok || !bytes.Equal(v, w) { + if !ok { + // extended attribute is not set for othernode + debug.Log("node has attribute %v, which is not present in othernode", k) + return false + } // attribute has different value - debug.Log("attribute %v has different value", attr.Name) - return false - } - - // remember that this attribute is present in other. - v.present = true - attributes[attr.Name] = v - } - - // check for attributes that are not present in other - for name, v := range attributes { - if !v.present { - debug.Log("attribute %v not present in other node", name) + debug.Log("attribute %v has different value", k) return false } } - + // If lengths are the same AND all of node's ext. attributes are in + // othernode ext. attributes => they are the same. return true } @@ -618,19 +598,14 @@ func (node *Node) fillExtendedAttributes(path string) error { return err } - node.ExtendedAttributes = make([]ExtendedAttribute, 0, len(xattrs)) + node.ExtendedAttributes = make(map[string][]byte, len(xattrs)) for _, attr := range xattrs { attrVal, err := Getxattr(path, attr) if err != nil { fmt.Fprintf(os.Stderr, "can not obtain extended attribute %v for %v:\n", attr, path) continue } - attr := ExtendedAttribute{ - Name: attr, - Value: attrVal, - } - - node.ExtendedAttributes = append(node.ExtendedAttributes, attr) + node.ExtendedAttributes[attr] = attrVal } return nil diff --git a/src/restic/node_test.go b/src/restic/node_test.go index 16414150f..3468c1f99 100644 --- a/src/restic/node_test.go +++ b/src/restic/node_test.go @@ -241,3 +241,26 @@ func AssertFsTimeEqual(t *testing.T, label string, nodeType string, t1 time.Time Assert(t, equal, "%s: %s doesn't match (%v != %v)", label, nodeType, t1, t2) } + +func TestNodeSameAttributes(t *testing.T) { + golden := restic.Node{ExtendedAttributes: map[string][]byte{"a": []byte{0x12, 0x34}, "b": []byte{0x56, 0x78}}} + testdata := []struct { + name string + actual restic.Node + expected bool + }{ + {"empty", restic.Node{}, false}, + {"same", restic.Node{ExtendedAttributes: map[string][]byte{"a": []byte{0x12, 0x34}, "b": []byte{0x56, 0x78}}}, true}, + {"almost", restic.Node{ExtendedAttributes: map[string][]byte{"a": []byte{0x12, 0x34}, "c": []byte{0x56, 0x78}}}, false}, + {"value", restic.Node{ExtendedAttributes: map[string][]byte{"a": []byte{0x12, 0x34}, "b": []byte{0x56, 0x9a}}}, false}, + {"missing", restic.Node{ExtendedAttributes: map[string][]byte{"a": []byte{0x12, 0x34}}}, false}, + {"toomuch", restic.Node{ExtendedAttributes: map[string][]byte{"a": []byte{0x12, 0x34}, "b": []byte{0x56, 0x78}, "c": []byte{0x9a, 0xbc}}}, false}, + } + for _, data := range testdata { + t.Run(data.name, func(t *testing.T) { + if golden.Equals(data.actual) != data.expected { + t.Errorf("extended attribute mismatch: %#v vs %#v, expected %v", golden.ExtendedAttributes, data.actual.ExtendedAttributes, data.expected) + } + }) + } +} diff --git a/src/restic/testdata/used_blobs_snapshot0 b/src/restic/testdata/used_blobs_snapshot0 index 9443e1e16..0ddd6ecee 100644 --- a/src/restic/testdata/used_blobs_snapshot0 +++ b/src/restic/testdata/used_blobs_snapshot0 @@ -1,7 +1,9 @@ {"ID":"087e8d5f45f93a78e52a938ac0b7864f92f8910091c0da69201a156242df3b78","Type":"data"} {"ID":"0bf505951741c44714527d252313b6959ce4f19d2e5512fca1c1b2da14424da3","Type":"data"} {"ID":"0c82d00e6ee78b48559cda2f9cc909beeb8769183b115dfda0a5767832accc8d","Type":"data"} +{"ID":"2807748845f8160620248c2429c0f968caaa8bd30e7b7f45c9018bb0bed24d04","Type":"tree"} {"ID":"2941bfd03b8933bb150b085a2252b69675495af64523bf8d38e67429e7cccb45","Type":"data"} +{"ID":"325dd83a3e0e15e61704b3005d8e06ddc7104a88a56b4240650f091096d3cc68","Type":"tree"} {"ID":"378a9b6862c8fa5c6915f158d16e4416243159bb9da44c564896c065bc6c1cf4","Type":"data"} {"ID":"3ffcf5128fc404c2a363e3e8a8d4c8a7ae8c36fcacba7fdfe71ec9dabcadd567","Type":"data"} {"ID":"40f5ca234e5eed1dc967c83fa99076ef636619148082f300cf877676728ebf14","Type":"data"} @@ -15,11 +17,9 @@ {"ID":"80ba9a145bf46cae605e911c18165c02213e8d11d68dc5b7824f259d17b7b6d0","Type":"data"} {"ID":"86af714d79d18be1c9c0ae23cca9dbd7cef44530e253e80af5bd5c34eab09714","Type":"data"} {"ID":"8a445cf5b6313cbe3b5872a55adde52aa8d1ae188f41d56f176e40a3137ac058","Type":"data"} -{"ID":"8e171f7367d1b68012ed1ceec8f54b7b9b8654ebaf63a760017c34d761b17878","Type":"tree"} {"ID":"8e98f35e65fb42c85eb4a2ab4793e294148e3f318252cb850a896274d2aa90bc","Type":"data"} {"ID":"9d65ba6443863394a8c6582fef4a8aaab2fb46417eef41f1792cdbdb38ee0b4c","Type":"data"} {"ID":"9da502ea8e7a768ee0dbafdc613db3df4a7cd9c98af08328265c4d2e953e8efa","Type":"data"} -{"ID":"9f2899688d2f23391cfd86e7b6d326a54f352bb294160878178639aab4aa378f","Type":"tree"} {"ID":"a2f3ccf973b3600c06c42dc3b867b263a788c18aa57f4448fea2525b7cbfd784","Type":"data"} {"ID":"b2deaf9174086129ec3b9f79e05401fdb3baf8b75335addffac1950182d779df","Type":"data"} {"ID":"b81870ebe27b98f6b8746349e8ea444c96bf2eaac5dbd6236175150ce579f46b","Type":"tree"} @@ -29,9 +29,9 @@ {"ID":"c54c4899c4d7dcda8b9e597aebfbaf7d65c9c7a760527d77e7fc9894283d736e","Type":"data"} {"ID":"ca51ecf1633896f852929cb2d56ad1b5bed4ab6055bdcf370ced4011bed164aa","Type":"data"} {"ID":"ce8b656cead478c34060510962daf97cea52abde68bbef7934dd5c5513cf6f3b","Type":"data"} +{"ID":"d820dc900a141e48065df20b0e50e8e7b06c07c95f0abc0809fef315399487e0","Type":"tree"} {"ID":"dafbb65569781083b627de833fb931cf98401299a62d747f03d8fc135ab57279","Type":"data"} {"ID":"e193d395410520580e76a5b89b8d23a1d162c0e28c52cb8194d409a74a120f7d","Type":"data"} -{"ID":"e752efd93f9850ba0cafbbac01bb283c10095ac923cdb8ff027393001123d406","Type":"tree"} +{"ID":"edd9fdd58923bc2d58533d8d256928eb772d72c9017470b38dd93bae092a812b","Type":"tree"} {"ID":"f728e5576d4ab63248c310396d67d9afa3267dd2dea3cfba690dbd04efe181fb","Type":"data"} -{"ID":"f75b6460b68d254f2195b08c606672fb55c05fb7bed7e16699b3231104b673ea","Type":"tree"} {"ID":"fe19f084021bdac5a9a5d270042ff53ef36357dd0743318d0480dee1a43de266","Type":"data"} diff --git a/src/restic/testdata/used_blobs_snapshot1 b/src/restic/testdata/used_blobs_snapshot1 index 3e6b6f395..390c41bcd 100644 --- a/src/restic/testdata/used_blobs_snapshot1 +++ b/src/restic/testdata/used_blobs_snapshot1 @@ -1,14 +1,14 @@ -{"ID":"011a951a9796979c2b515ef4209662013bd1f16a20a1b35d1d950d7408bdc8b4","Type":"tree"} {"ID":"087e8d5f45f93a78e52a938ac0b7864f92f8910091c0da69201a156242df3b78","Type":"data"} {"ID":"0bad18b7f2d82d7c9cf8e405262ad2f3dbe57928aa242c1070b917042a99072d","Type":"data"} {"ID":"0bf505951741c44714527d252313b6959ce4f19d2e5512fca1c1b2da14424da3","Type":"data"} {"ID":"0c82d00e6ee78b48559cda2f9cc909beeb8769183b115dfda0a5767832accc8d","Type":"data"} +{"ID":"1d9d469696d55ea59985da74a7bbecbecd65549758c8800517d652fe306a576b","Type":"tree"} +{"ID":"20cf190a4de4b09d3f26212cf75c4bf5780cbe71a94cac10d60b6196e674557c","Type":"tree"} {"ID":"2941bfd03b8933bb150b085a2252b69675495af64523bf8d38e67429e7cccb45","Type":"data"} {"ID":"3ffcf5128fc404c2a363e3e8a8d4c8a7ae8c36fcacba7fdfe71ec9dabcadd567","Type":"data"} {"ID":"40f5ca234e5eed1dc967c83fa99076ef636619148082f300cf877676728ebf14","Type":"data"} {"ID":"42bc8f509dbd6b9881cab4c1684d5cf74207046336f654db1b884197f15cae7b","Type":"data"} {"ID":"47cf470c1c6de9af00b3b1ee963de8b94f51a2870b3338b3f33cfc565c0f8be4","Type":"data"} -{"ID":"4b2e91022c34c756b7bd8ece046a2bab6f0dcad89f46c52d1f84cd48e8da55df","Type":"tree"} {"ID":"6416bc2321cdeb8758188af2b3925f2c82ffde014bf53b7a69c0f113a5c460fe","Type":"data"} {"ID":"714f9e16404b9ec83de56715e5387b2c4c2ed0af1889166a4e767822f971bf52","Type":"data"} {"ID":"80ba9a145bf46cae605e911c18165c02213e8d11d68dc5b7824f259d17b7b6d0","Type":"data"} @@ -24,11 +24,11 @@ {"ID":"bd4dacd46031b2b837bc9bd06145b0571156fa496408ce728c003ae50b265aaf","Type":"data"} {"ID":"c3596f717c495d20c33561e991d4295550b6d7544687f2363e999bdc0266224d","Type":"data"} {"ID":"c54c4899c4d7dcda8b9e597aebfbaf7d65c9c7a760527d77e7fc9894283d736e","Type":"data"} +{"ID":"c7e9366ab3f61d19c584e5b9ecbb56544404b1ce391817af08ad8a03bf2e6b7d","Type":"tree"} {"ID":"ca51ecf1633896f852929cb2d56ad1b5bed4ab6055bdcf370ced4011bed164aa","Type":"data"} {"ID":"cb8001715217b4f6960aa24c1abb4b60a20c10f23abc1e5f69e0f5436bd788c8","Type":"data"} {"ID":"d39c4c264e01ec47b0386da3775c6b0cc337974627ff55792938cca4895ac6c4","Type":"data"} {"ID":"dafbb65569781083b627de833fb931cf98401299a62d747f03d8fc135ab57279","Type":"data"} {"ID":"e193d395410520580e76a5b89b8d23a1d162c0e28c52cb8194d409a74a120f7d","Type":"data"} -{"ID":"e791912a7fad8954c764fae41d2958d2feeae2278e403429add9119ab43a36f5","Type":"tree"} {"ID":"f728e5576d4ab63248c310396d67d9afa3267dd2dea3cfba690dbd04efe181fb","Type":"data"} {"ID":"fe19f084021bdac5a9a5d270042ff53ef36357dd0743318d0480dee1a43de266","Type":"data"}