Many comments on your PR. It looks like it's in a pretty reasonble state for doing some testing. Are you able to run rocksdb on top of it yet, or are you just running the unit tests? Please rebase on current ceph/rocksdb master, e.g. git remote add cephrocksdb git@xxxxxxxxxx:ceph/rocksdb git fetch cephrocksdb git rebase HEAD^^ --onto cephrocksdb/master sage On Wed, 8 Jun 2016, Ryne Li wrote: > Hi Sage, > Thank you so much for your advises, especially the IO_CTX pointer one. It is > really helpful. I have revised my code according to your comments. > Generally, I finished all your comments except distinguishing Flush() and > Sync(). Main changes list as follow, > > - change copy of IO_CTX in file handler object to IO_CTX pointer > - remove xattr functions > - change split function to adapt RocksDB's 2-level structure (dir/file) > - add buffer to writable file handler > - correct Makefile > > By the way, I accidentally forcibly push my modification. So the diff > between current version and last version can't reflect my latest > modification. Sorry for the inconvenience. > > Following is the link to latest version of my code: > > https://github.com/ryneli/rocksdb/blob/master/utilities/env_librados.cc > > Yours, > > Ryne > > On Tue, Jun 7, 2016 at 9:19 AM, Ryne Li <rli@xxxxxxxxxx> wrote: > Hi Sage, > Thank you for your comments and advises. It is really helpful. I will > revise my code according to your comments. > > I didn't find omap api at the beginning. So I used these xattr api > instead. I will remove these functions later. > > Useless Makefile changes may be caused my careless commit. Sorry about > that. > > Yours, > > Ryne > > On Tue, Jun 7, 2016 at 8:57 AM, Sage Weil <sweil@xxxxxxxxxx> wrote: > [Copying ceph-devel so that interest parties can take a > look] > > On Tue, 7 Jun 2016, Ryne Li wrote: > > Hi Sage, > > Most code is in following file, > > > > > https://github.com/ryneli/rocksdb/blob/master/utilities/env_librados.cc > > > > Test code is in file, > > > > https://github.com/ryneli/rocksdb/blob/master/utilities/env_librados_tes > t > > .cc > > This is great! I opened a PR against our rocksdb repo > just so I could > comment on the code: > > https://github.com/ceph/rocksdb/pull/8 > > I made a bunch of comments. This looks like a great > start! > > My main question is what the purpose of the xattr > functions is. I don't > see any callers, and I think an omap object for the > directory mapping > should be sufficient. > > It looks like there are a bunch of Makefile changes in > that commit that > probably don't need to be included? > > sage > > > > >
