{"action":"created","issue":{"url":"https://api.github.com/repos/dgraph-io/badger/issues/341","repository_url":"https://api.github.com/repos/dgraph-io/badger","labels_url":"https://api.github.com/repos/dgraph-io/badger/issues/341/labels{/name}","comments_url":"https://api.github.com/repos/dgraph-io/badger/issues/341/comments","events_url":"https://api.github.com/repos/dgraph-io/badger/issues/341/events","html_url":"https://github.com/dgraph-io/badger/pull/341","id":279643090,"number":341,"title":"Make mmap-ing of value log optional.","user":{"login":"deepakjois","id":5342,"avatar_url":"https://avatars2.githubusercontent.com/u/5342?v=4","gravatar_id":"","url":"https://api.github.com/users/deepakjois","html_url":"https://github.com/deepakjois","followers_url":"https://api.github.com/users/deepakjois/followers","following_url":"https://api.github.com/users/deepakjois/following{/other_user}","gists_url":"https://api.github.com/users/deepakjois/gists{/gist_id}","starred_url":"https://api.github.com/users/deepakjois/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/deepakjois/subscriptions","organizations_url":"https://api.github.com/users/deepakjois/orgs","repos_url":"https://api.github.com/users/deepakjois/repos","events_url":"https://api.github.com/users/deepakjois/events{/privacy}","received_events_url":"https://api.github.com/users/deepakjois/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"manishrjain","id":1782324,"avatar_url":"https://avatars1.githubusercontent.com/u/1782324?v=4","gravatar_id":"","url":"https://api.github.com/users/manishrjain","html_url":"https://github.com/manishrjain","followers_url":"https://api.github.com/users/manishrjain/followers","following_url":"https://api.github.com/users/manishrjain/following{/other_user}","gists_url":"https://api.github.com/users/manishrjain/gists{/gist_id}","starred_url":"https://api.github.com/users/manishrjain/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/manishrjain/subscriptions","organizations_url":"https://api.github.com/users/manishrjain/orgs","repos_url":"https://api.github.com/users/manishrjain/repos","events_url":"https://api.github.com/users/manishrjain/events{/privacy}","received_events_url":"https://api.github.com/users/manishrjain/received_events","type":"User","site_admin":false},"assignees":[{"login":"manishrjain","id":1782324,"avatar_url":"https://avatars1.githubusercontent.com/u/1782324?v=4","gravatar_id":"","url":"https://api.github.com/users/manishrjain","html_url":"https://github.com/manishrjain","followers_url":"https://api.github.com/users/manishrjain/followers","following_url":"https://api.github.com/users/manishrjain/following{/other_user}","gists_url":"https://api.github.com/users/manishrjain/gists{/gist_id}","starred_url":"https://api.github.com/users/manishrjain/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/manishrjain/subscriptions","organizations_url":"https://api.github.com/users/manishrjain/orgs","repos_url":"https://api.github.com/users/manishrjain/repos","events_url":"https://api.github.com/users/manishrjain/events{/privacy}","received_events_url":"https://api.github.com/users/manishrjain/received_events","type":"User","site_admin":false}],"milestone":null,"comments":2,"created_at":"2017-12-06T06:32:53Z","updated_at":"2017-12-11T05:35:49Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/dgraph-io/badger/pulls/341","html_url":"https://github.com/dgraph-io/badger/pull/341","diff_url":"https://github.com/dgraph-io/badger/pull/341.diff","patch_url":"https://github.com/dgraph-io/badger/pull/341.patch"},"body":"We introduce a new option ValueLogLoadingMode, which controls\r\nwhether the value log is read via mmap, or by standard File I/O.\r\n\r\nMethods that fall in the path to reading the value from log now\r\ntake an additional argument for the slice which would be used to\r\nread the data, if the ValueLoadLoadingMode is set to FileIO.\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/dgraph-io/badger/341)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/dgraph-io/badger/issues/comments/350626827","html_url":"https://github.com/dgraph-io/badger/pull/341#issuecomment-350626827","issue_url":"https://api.github.com/repos/dgraph-io/badger/issues/341","id":350626827,"user":{"login":"deepakjois","id":5342,"avatar_url":"https://avatars2.githubusercontent.com/u/5342?v=4","gravatar_id":"","url":"https://api.github.com/users/deepakjois","html_url":"https://github.com/deepakjois","followers_url":"https://api.github.com/users/deepakjois/followers","following_url":"https://api.github.com/users/deepakjois/following{/other_user}","gists_url":"https://api.github.com/users/deepakjois/gists{/gist_id}","starred_url":"https://api.github.com/users/deepakjois/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/deepakjois/subscriptions","organizations_url":"https://api.github.com/users/deepakjois/orgs","repos_url":"https://api.github.com/users/deepakjois/repos","events_url":"https://api.github.com/users/deepakjois/events{/privacy}","received_events_url":"https://api.github.com/users/deepakjois/received_events","type":"User","site_admin":false},"created_at":"2017-12-11T05:35:48Z","updated_at":"2017-12-11T05:35:48Z","author_association":"MEMBER","body":"\n\n\n\nReview status: 5 of 9 files reviewed at latest revision, 5 unresolved discussions.\n\n---\n\n*[value.go, line 125 at r1](https://reviewable.io:443/reviews/dgraph-io/badger/341#-L01ucMR92OSDlBAJUK-:-L030fjukJf10MU36LV9:bz191y0) ([raw file](https://github.com/dgraph-io/badger/blob/8ba769be7ad9692bac1654b4ec91bddc06617374/value.go#L125)):*\n`n` and `offset` are not comparable. Do you mean `n < len(buf)`? If so that is already covered.\n\n> ReadAt always returns a non-nil error when n < len(b).\n\n---\n\n*[value.go, line 143 at r1](https://reviewable.io:443/reviews/dgraph-io/badger/341#-L-zTtOP91wnxKH4nOR9:-L02zCAI-iffNZ5qv82z:b-886gsx) ([raw file](https://github.com/dgraph-io/badger/blob/8ba769be7ad9692bac1654b4ec91bddc06617374/value.go#L143)):*\n<details><summary><i>Previously, manishrjain (Manish R Jain) wrote…</i></summary><blockquote>\n\nLooks like a typo.\n</blockquote></details>\n\nFixed\n\n---\n\n*[value.go, line 159 at r1](https://reviewable.io:443/reviews/dgraph-io/badger/341#-L-zTzfrgeiCb6fy6VPA:-L031UsgjrN7A6duJUtQ:bnkfa0r) ([raw file](https://github.com/dgraph-io/badger/blob/8ba769be7ad9692bac1654b4ec91bddc06617374/value.go#L159)):*\n<details><summary><i>Previously, manishrjain (Manish R Jain) wrote…</i></summary><blockquote>\n\nThis change of behavior looks buggy.\n</blockquote></details>\n\nI guess it is a typo. I intended to have `defer lf.lock.Unlock()` before doing `lf.munmap()`.\n\nFixed now.\n\n---\n\n*[value.go, line 814 at r1](https://reviewable.io:443/reviews/dgraph-io/badger/341#-L-zUtoY7BLwqlS1TaS-:-L033W-jdxkjcQ3bIQdg:bh79iux) ([raw file](https://github.com/dgraph-io/badger/blob/8ba769be7ad9692bac1654b4ec91bddc06617374/value.go#L814)):*\n<details><summary><i>Previously, manishrjain (Manish R Jain) wrote…</i></summary><blockquote>\n\nIf the file is not mmaped, we don't need to hold a lock onto it, because we're copying the value directly into slice anyway.\n</blockquote></details>\n\nGood point.\n\nI have now changed the behavior to immediately unlock if we are using File I/O.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/dgraph-io/badger/341)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"deepakjois"}}