refactor: kv::adapter should use Buffer (write part) #4496
Conversation
Signed-off-by: tison <[email protected]>
Signed-off-by: tison <[email protected]>
core/src/types/buffer.rs
Outdated
| @@ -149,6 +149,12 @@ impl Default for Buffer { | |||
| } | |||
| } | |||
|
|
|||
| impl AsRef<[u8]> for Buffer { | |||
There was a problem hiding this comment.
Buffer could have multiple bytes. It's better not to implement this to avoid Buffer been used wrongly.
There was a problem hiding this comment.
Then we call .chunk everywhere?
There was a problem hiding this comment.
Then we call
.chunkeverywhere?
Yes, if the consumer accepts bytes::Buf. In most cases, such as in this PR, we should collect them into a single Bytes to allow writing them in one request (for example, in redis set call).
We can optimize this by introducing multi_write for the KV adapter, similar to what we do with oio::Write.
There was a problem hiding this comment.
I suggest you push a new commit onto this branch. I can't get it quite clear.
And we may wait a bit for #4497 to get the collect update.
There was a problem hiding this comment.
I suggest you push a new commit onto this branch. I can't get it quite clear.
DONE.
core/src/raw/adapters/kv/backend.rs
Outdated
| }; | ||
|
|
||
| self.kv.set(&self.path, &buf).await | ||
| let buf = self.buffer.collect(); |
There was a problem hiding this comment.
I wonder if we can avoid clone here but set self.buffer = QueueBuf::default() with mem::replace. And then call into_buffer as introduced in #4497.
Signed-off-by: Xuanwo <[email protected]>
This closes #4479