Level / community

Discussion, support and common information for projects in the community.
MIT License
42 stars 15 forks source link

Add `db.getMany(keys)` across the board #101

Closed vweevers closed 2 years ago

vweevers commented 2 years ago

Background: https://github.com/Level/abstract-leveldown/issues/380 and https://github.com/Level/levelup/issues/491.

MeirionHughes commented 2 years ago

I'll try with the rockdb port to help out if no one was already doing it. I also need to check the typings on DT as they're broken after the 'default' export removals. So I'll start adding getMany at same time.

vweevers commented 2 years ago

Go for it, thanks! Note that on rocksdb we must first port https://github.com/Level/leveldown/pull/783, https://github.com/Level/leveldown/pull/784, https://github.com/Level/leveldown/pull/785 (in that order), and then https://github.com/Level/leveldown/pull/787.

gogoout commented 2 years ago

I'll try with the rockdb port to help out if no one was already doing it. I also need to check the typings on DT as they're broken after the 'default' export removals. So I'll start adding getMany at same time.

Just asking, do you plan to use the rocksdb's native getMany interface to implementing this?

vweevers commented 2 years ago

I would prefer that we don't, because it will increase the code diff between leveldown and rocksdb, making cherry-picking commits from one to the other more work. In addition, the leveldown code is written in anticipation of additional features like iterator.all() which will reuse common functions.

MeirionHughes commented 2 years ago

Just asking, do you plan to use the rocksdb's native getMany interface to implementing this?

That is what I was looking to do...

I would prefer that we don't, because it will increase the code diff between leveldown and rocksdb

... but I guess not. :D

I don't think I know enough to do it tbh: the low-hanging method I was targetting to call:

  virtual std::vector<Status> MultiGet(const ReadOptions& options,
                                       const std::vector<Slice>& keys,
                                       std::vector<std::string>* values) {

has the comment:

Consistent Get of many keys across column families without the need for an explicit snapshot. NOTE: the implementation of this MultiGet API does not have the performance benefits of the void-returning MultiGet functions.

So:

Probably makes sense to just do all cherry picking for now.

gogoout commented 2 years ago

@vweevers @MeirionHughes Just confirming anyone of you are going to implementing this for rocksdb? Just eager to try the new interface :)

sebastianst commented 2 years ago

Types are still missing getMany. Opened an issue in the levelup repo.

Bonaventure00916 commented 2 years ago

Disappointed 😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭