You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "zhixinwen (via GitHub)" <gi...@apache.org> on 2025/07/31 22:21:45 UTC
[PR] feat(replication): make replication delay configurable [kvrocks]
zhixinwen opened a new pull request, #3087:
URL: https://github.com/apache/kvrocks/pull/3087
(no comment)
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(replication): make replication delay configurable [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #3087:
URL: https://github.com/apache/kvrocks/pull/3087#discussion_r2247273389
##########
src/cluster/replication.h:
##########
@@ -87,8 +93,9 @@ class FeedSlaveThread {
redis::Request req_;
std::atomic<rocksdb::SequenceNumber> ack_seq_ = 0;
- static const size_t kMaxDelayUpdates = 16;
- static const size_t kMaxDelayBytes = 16 * 1024;
+ // Configurable delay limits
+ size_t max_delay_bytes_ = 16 * 1024;
+ size_t max_delay_updates_ = 16;
Review Comment:
```suggestion
size_t max_delay_bytes_;
size_t max_delay_updates_;
```
Default value seems not so useful here.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(replication): make replication delay configurable [kvrocks]
Posted by "zhixinwen (via GitHub)" <gi...@apache.org>.
zhixinwen commented on code in PR #3087:
URL: https://github.com/apache/kvrocks/pull/3087#discussion_r2248914709
##########
src/cluster/replication.h:
##########
@@ -64,8 +65,13 @@ using FetchFileCallback = std::function<void(const std::string &, uint32_t)>;
class FeedSlaveThread {
public:
- explicit FeedSlaveThread(Server *srv, redis::Connection *conn, rocksdb::SequenceNumber next_repl_seq)
- : srv_(srv), conn_(conn), next_repl_seq_(next_repl_seq), req_(srv) {}
+ FeedSlaveThread(Server *srv, redis::Connection *conn, rocksdb::SequenceNumber next_repl_seq)
+ : srv_(srv),
+ conn_(conn),
+ next_repl_seq_(next_repl_seq),
+ req_(srv),
+ max_delay_bytes_(srv->GetConfig()->max_replication_delay_bytes),
+ max_delay_updates_(srv->GetConfig()->max_replication_delay_updates) {}
Review Comment:
ok, I see you are referring to the lint error. I will google it, thx.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(replication): make replication delay configurable [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice merged PR #3087:
URL: https://github.com/apache/kvrocks/pull/3087
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(replication): make replication delay configurable [kvrocks]
Posted by "zhixinwen (via GitHub)" <gi...@apache.org>.
zhixinwen commented on code in PR #3087:
URL: https://github.com/apache/kvrocks/pull/3087#discussion_r2248866628
##########
src/cluster/replication.h:
##########
@@ -64,8 +65,13 @@ using FetchFileCallback = std::function<void(const std::string &, uint32_t)>;
class FeedSlaveThread {
public:
- explicit FeedSlaveThread(Server *srv, redis::Connection *conn, rocksdb::SequenceNumber next_repl_seq)
- : srv_(srv), conn_(conn), next_repl_seq_(next_repl_seq), req_(srv) {}
+ FeedSlaveThread(Server *srv, redis::Connection *conn, rocksdb::SequenceNumber next_repl_seq)
+ : srv_(srv),
+ conn_(conn),
+ next_repl_seq_(next_repl_seq),
+ req_(srv),
+ max_delay_bytes_(srv->GetConfig()->max_replication_delay_bytes),
+ max_delay_updates_(srv->GetConfig()->max_replication_delay_updates) {}
Review Comment:
could you explain what is incomplete type here?
Sorry not familiar with C++
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(replication): make replication delay configurable [kvrocks]
Posted by "zhixinwen (via GitHub)" <gi...@apache.org>.
zhixinwen commented on code in PR #3087:
URL: https://github.com/apache/kvrocks/pull/3087#discussion_r2248914709
##########
src/cluster/replication.h:
##########
@@ -64,8 +65,13 @@ using FetchFileCallback = std::function<void(const std::string &, uint32_t)>;
class FeedSlaveThread {
public:
- explicit FeedSlaveThread(Server *srv, redis::Connection *conn, rocksdb::SequenceNumber next_repl_seq)
- : srv_(srv), conn_(conn), next_repl_seq_(next_repl_seq), req_(srv) {}
+ FeedSlaveThread(Server *srv, redis::Connection *conn, rocksdb::SequenceNumber next_repl_seq)
+ : srv_(srv),
+ conn_(conn),
+ next_repl_seq_(next_repl_seq),
+ req_(srv),
+ max_delay_bytes_(srv->GetConfig()->max_replication_delay_bytes),
+ max_delay_updates_(srv->GetConfig()->max_replication_delay_updates) {}
Review Comment:
ok, I saw you are referring to the lint error. I will google it, thx.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(replication): make replication delay configurable [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #3087:
URL: https://github.com/apache/kvrocks/pull/3087#discussion_r2247272200
##########
src/cluster/replication.h:
##########
@@ -64,8 +65,13 @@ using FetchFileCallback = std::function<void(const std::string &, uint32_t)>;
class FeedSlaveThread {
public:
- explicit FeedSlaveThread(Server *srv, redis::Connection *conn, rocksdb::SequenceNumber next_repl_seq)
- : srv_(srv), conn_(conn), next_repl_seq_(next_repl_seq), req_(srv) {}
+ FeedSlaveThread(Server *srv, redis::Connection *conn, rocksdb::SequenceNumber next_repl_seq)
+ : srv_(srv),
+ conn_(conn),
+ next_repl_seq_(next_repl_seq),
+ req_(srv),
+ max_delay_bytes_(srv->GetConfig()->max_replication_delay_bytes),
+ max_delay_updates_(srv->GetConfig()->max_replication_delay_updates) {}
Review Comment:
Need to move it to `replication.cc` to avoid incomplete type.
##########
src/cluster/replication.h:
##########
@@ -36,6 +36,7 @@
#include "io_util.h"
#include "rocksdb/write_batch.h"
#include "server/redis_connection.h"
+#include "server/server.h"
Review Comment:
Remove this.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(replication): make replication delay configurable [kvrocks]
Posted by "zhixinwen (via GitHub)" <gi...@apache.org>.
zhixinwen commented on code in PR #3087:
URL: https://github.com/apache/kvrocks/pull/3087#discussion_r2248914709
##########
src/cluster/replication.h:
##########
@@ -64,8 +65,13 @@ using FetchFileCallback = std::function<void(const std::string &, uint32_t)>;
class FeedSlaveThread {
public:
- explicit FeedSlaveThread(Server *srv, redis::Connection *conn, rocksdb::SequenceNumber next_repl_seq)
- : srv_(srv), conn_(conn), next_repl_seq_(next_repl_seq), req_(srv) {}
+ FeedSlaveThread(Server *srv, redis::Connection *conn, rocksdb::SequenceNumber next_repl_seq)
+ : srv_(srv),
+ conn_(conn),
+ next_repl_seq_(next_repl_seq),
+ req_(srv),
+ max_delay_bytes_(srv->GetConfig()->max_replication_delay_bytes),
+ max_delay_updates_(srv->GetConfig()->max_replication_delay_updates) {}
Review Comment:
ok, I saw you are referring to the lint error. I will google it.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org