{"action":"created","issue":{"url":"https://api.github.com/repos/ipfs/go-blockservice/issues/9","repository_url":"https://api.github.com/repos/ipfs/go-blockservice","labels_url":"https://api.github.com/repos/ipfs/go-blockservice/issues/9/labels{/name}","comments_url":"https://api.github.com/repos/ipfs/go-blockservice/issues/9/comments","events_url":"https://api.github.com/repos/ipfs/go-blockservice/issues/9/events","html_url":"https://github.com/ipfs/go-blockservice/issues/9","id":378277222,"node_id":"MDU6SXNzdWUzNzgyNzcyMjI=","number":9,"title":"AddBlock will check file exist twice for new file","user":{"login":"songjiayang","id":1459834,"node_id":"MDQ6VXNlcjE0NTk4MzQ=","avatar_url":"https://avatars0.githubusercontent.com/u/1459834?v=4","gravatar_id":"","url":"https://api.github.com/users/songjiayang","html_url":"https://github.com/songjiayang","followers_url":"https://api.github.com/users/songjiayang/followers","following_url":"https://api.github.com/users/songjiayang/following{/other_user}","gists_url":"https://api.github.com/users/songjiayang/gists{/gist_id}","starred_url":"https://api.github.com/users/songjiayang/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/songjiayang/subscriptions","organizations_url":"https://api.github.com/users/songjiayang/orgs","repos_url":"https://api.github.com/users/songjiayang/repos","events_url":"https://api.github.com/users/songjiayang/events{/privacy}","received_events_url":"https://api.github.com/users/songjiayang/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-11-07T12:59:26Z","updated_at":"2018-11-07T18:43:28Z","closed_at":null,"author_association":"NONE","body":"1. The BlockService's AddBlock method look like:\r\n```golang\r\n\r\n# go-blockservice/blockservice.go\r\nfunc (s *blockService) AddBlock(o blocks.Block) error {\r\n\tif s.checkFirst {\r\n\t\tif has, err := s.blockstore.Has(c); has || err != nil {\r\n\t\t\treturn err\r\n\t\t}\r\n\t}\r\n\r\n\r\n\tif err := s.blockstore.Put(o); err != nil {\r\n\t\treturn err\r\n\t}\r\n}\r\n```\r\n2. blockstore.Put method look like: \r\n\r\n```golang\r\n# go-ipfs-blockstore/blockstore.go\r\nfunc (bs *blockstore) Put(block blocks.Block) error {\r\n\tk := dshelp.CidToDsKey(block.Cid())\r\n\r\n\t// Has is cheaper than Put, so see if we already have it\r\n\texists, err := bs.datastore.Has(k)\r\n\tif err == nil && exists {\r\n\t\treturn nil // already stored.\r\n\t}\r\n\treturn bs.datastore.Put(k, block.RawData())\r\n}\r\n```\r\n\r\n3. As we can see it check key exist twice, the check method is:\r\n\r\n```golang\r\n# go-ds-flatfs/flatfs.go\r\nfunc (fs *Datastore) Has(key datastore.Key) (exists bool, err error) {\r\n\t_, path := fs.encode(key)\r\n\tswitch _, err := os.Stat(path); {\r\n\tcase err == nil:\r\n\t\treturn true, nil\r\n\tcase os.IsNotExist(err):\r\n\t\treturn false, nil\r\n\tdefault:\r\n\t\treturn false, err\r\n\t}\r\n}\r\n``` \r\n\r\nIt's expensive to check file exist with system call (os.Stat), maybe we can cache the check result.\r\n\r\nis it right? maybe I made a mistake. \r\n\r\n@whyrusleeping look forward to your reply.\r\n"},"comment":{"url":"https://api.github.com/repos/ipfs/go-blockservice/issues/comments/436733235","html_url":"https://github.com/ipfs/go-blockservice/issues/9#issuecomment-436733235","issue_url":"https://api.github.com/repos/ipfs/go-blockservice/issues/9","id":436733235,"node_id":"MDEyOklzc3VlQ29tbWVudDQzNjczMzIzNQ==","user":{"login":"whyrusleeping","id":1243164,"node_id":"MDQ6VXNlcjEyNDMxNjQ=","avatar_url":"https://avatars0.githubusercontent.com/u/1243164?v=4","gravatar_id":"","url":"https://api.github.com/users/whyrusleeping","html_url":"https://github.com/whyrusleeping","followers_url":"https://api.github.com/users/whyrusleeping/followers","following_url":"https://api.github.com/users/whyrusleeping/following{/other_user}","gists_url":"https://api.github.com/users/whyrusleeping/gists{/gist_id}","starred_url":"https://api.github.com/users/whyrusleeping/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/whyrusleeping/subscriptions","organizations_url":"https://api.github.com/users/whyrusleeping/orgs","repos_url":"https://api.github.com/users/whyrusleeping/repos","events_url":"https://api.github.com/users/whyrusleeping/events{/privacy}","received_events_url":"https://api.github.com/users/whyrusleeping/received_events","type":"User","site_admin":false},"created_at":"2018-11-07T18:43:28Z","updated_at":"2018-11-07T18:43:28Z","author_association":"MEMBER","body":"@songjiayang that is correct. We try to skirt that issue by using a 'has-cached' blockstore. So the actual blockstore implementation given to the blockservice should be: https://github.com/ipfs/go-ipfs-blockstore/blob/master/arc_cache.go\r\n\r\nWhere all the has calls get cached in the ARC cache. \r\n\r\nWe should make this more clear, as I think that many users will just construct a simple blockstore, and thus incur the double has checks.\r\n\r\nAlso, I would like to get rid of the blockservice entirely, it feels more and more like a not-super-useful wrapper, and its job could be taken over entirely by something else.\r\n\r\ncc @Stebalien "}}
{"actor":{"display_login":"whyrusleeping"}}