mapleFU commented on code in PR #2689:
URL: https://github.com/apache/kvrocks/pull/2689#discussion_r1878098076
##########
src/cluster/replication.cc:
##########
@@ -576,31 +578,35 @@ ReplicationThread::CBState
ReplicationThread::incrementBatchLoopCB(bufferevent *
}
case Incr_batch_data:
// Read bulk data (batch data)
- if (incr_bulk_len_ + 2 <= evbuffer_get_length(input)) { // We got
enough data
- bulk_data = reinterpret_cast<char *>(evbuffer_pullup(input,
static_cast<ssize_t>(incr_bulk_len_ + 2)));
- std::string bulk_string = std::string(bulk_data, incr_bulk_len_);
- // master would send the ping heartbeat packet to check whether the
slave was alive or not,
- // don't write ping to db here.
- if (bulk_string != "ping") {
- auto s = storage_->ReplicaApplyWriteBatch(std::string(bulk_data,
incr_bulk_len_));
- if (!s.IsOK()) {
- LOG(ERROR) << "[replication] CRITICAL - Failed to write batch to
local, " << s.Msg() << ". batch: 0x"
- << util::StringToHex(bulk_string);
- return CBState::RESTART;
- }
+ if (incr_bulk_len_ + 2 > evbuffer_get_length(input)) { // If data not
enough
+ return CBState::AGAIN;
+ }
- s = parseWriteBatch(bulk_string);
- if (!s.IsOK()) {
- LOG(ERROR) << "[replication] CRITICAL - failed to parse write
batch 0x" << util::StringToHex(bulk_string)
- << ": " << s.Msg();
- return CBState::RESTART;
- }
- }
- evbuffer_drain(input, incr_bulk_len_ + 2);
- incr_state_ = Incr_batch_size;
- } else {
+ char* bulk_data = reinterpret_cast<char *>(evbuffer_pullup(input,
static_cast<ssize_t>(incr_bulk_len_ + 2)));
+ std::string bulk_string = std::string(bulk_data, incr_bulk_len_);
+ evbuffer_drain(input, incr_bulk_len_ + 2);
Review Comment:
Not related to this issue directly
I'm not familiar with `evbuffer` api, but would
```c++
std::string bulk_string(0, incr_bulk_len_);
evbuffer_remove(input, bulk_string.data(), incr_bulk_len_ + 2);
```
Being ok since it avoid adjust the `input` internal? (The bad things is that
std::string would zero-initialize itself, which introducing a round of copying
😅) @git-hulk @PragmaTwice
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
