feat(server family): impl ROLE command #660
Conversation
Signed-off-by: adi_holden <adi@dragonflydb.io>
src/server/dflycmd.h
Outdated
| struct ReplicaData { | ||
| ReplicaData(std::string address, std::string port, SyncState sync_state) | ||
| : address(address), port(port) { | ||
| switch (sync_state) { |
There was a problem hiding this comment.
Pls move the impl to cc. I would even keep SyncState as is and render its string inside servet_family
src/server/dflycmd.h
Outdated
| Context cntx; | ||
|
|
||
| std::vector<FlowInfo> flows; | ||
| ::boost::fibers::mutex mu; // See top of header for locking levels. | ||
| }; | ||
| struct ReplicaData { |
There was a problem hiding this comment.
Since we already have ReplicaInfo, ReplicaData is a bit confusing. Maybe ReplicaRoleInfo?
src/facade/dragonfly_connection.cc
Outdated
| std::string Connection::RemoteEndpointIp() const { | ||
| LinuxSocketBase* lsb = static_cast<LinuxSocketBase*>(socket_.get()); | ||
| auto re = lsb->RemoteEndpoint(); | ||
| // TODO: roman port is this the right property of re? I get wrong values |
There was a problem hiding this comment.
I did not understand the question. It's a remote port of the client connection. I.e it's never 6380. What redis shows here?
There was a problem hiding this comment.
Ohh you are right I got confused. So I am not sure what is the meaning of port in this flow. We are opening several connections in the sync
There was a problem hiding this comment.
True. Lets send an empty string for now and see later what it breaks :)
Signed-off-by: adi_holden <adi@dragonflydb.io>
Signed-off-by: adi_holden <adi@dragonflydb.io>
|
@romange can you please review again |
src/server/dflycmd.cc
Outdated
| std::vector<ReplicaRoleInfo> vec; | ||
| unique_lock lk(mu_); | ||
| for (const auto& info : replica_infos_) { | ||
| vec.emplace_back(info.second->address, info.second->state); |
There was a problem hiding this comment.
The ReplicaInfo fields are not guarded by the global mutex itself. While address is nowhere accessed, the state is on all operations.
Reading from the state (and even reading an old value) has no consequences at all here, but doing this is technically undefined behavior, so we should probably make this field atomic
Signed-off-by: adi_holden <adi@dragonflydb.io>
Signed-off-by: adi_holden <adi@dragonflydb.io>
| @@ -403,7 +423,8 @@ void DflyCmd::FullSyncFb(FlowInfo* flow, Context* cntx) { | |||
| } | |||
| } | |||
|
|
|||
| uint32_t DflyCmd::CreateSyncSession() { | |||
| uint32_t DflyCmd::CreateSyncSession(ConnectionContext* cntx) { | |||
| ; | |||
Signed-off-by: adi_holden adi@dragonflydb.io