From 517864adaa244d7477769bec526177c417b2e0f5 Mon Sep 17 00:00:00 2001 From: Uddeshya Singh Date: Mon, 12 Jun 2023 16:37:16 +0530 Subject: [PATCH 1/3] Don't allow the instance replication of itself and it's own replicas (#1488) Co-authored-by: git-hulk Co-authored-by: Twice --- src/commands/cmd_server.cc | 25 +++++++++++++++- src/common/io_util.cc | 27 +++++++++++++++++ src/common/io_util.h | 1 + src/server/server.cc | 13 +++++++++ src/server/server.h | 1 + .../replication/replication_test.go | 29 +++++++++++++++++++ 6 files changed, 95 insertions(+), 1 deletion(-) diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc index 4002aa0057d..cc0b2f2286f 100644 --- a/src/commands/cmd_server.cc +++ b/src/commands/cmd_server.cc @@ -20,6 +20,7 @@ #include "commander.h" #include "commands/scan_base.h" +#include "common/io_util.h" #include "config/config.h" #include "error_constants.h" #include "server/redis_connection.h" @@ -867,6 +868,24 @@ class CommandFlushBackup : public Commander { class CommandSlaveOf : public Commander { public: + static Status IsTryingToReplicateItself(Server *svr, const std::string &host, uint32_t port) { + auto ip_addresses = util::LookupHostByName(host); + if (!ip_addresses) { + return {Status::NotOK, "Can not resolve hostname: " + host}; + } + for (auto &ip : *ip_addresses) { + if (util::MatchListeningIP(svr->GetConfig()->binds, ip) && port == svr->GetConfig()->port) { + return {Status::NotOK, "can't replicate itself"}; + } + for (std::pair &host_port_pair : svr->GetSlaveHostAndPort()) { + if (host_port_pair.first == ip && host_port_pair.second == port) { + return {Status::NotOK, "can't replicate your own replicas"}; + } + } + } + return Status::OK(); + } + Status Parse(const std::vector &args) override { host_ = args[1]; const auto &port = args[2]; @@ -914,7 +933,11 @@ class CommandSlaveOf : public Commander { return Status::OK(); } - auto s = svr->AddMaster(host_, port_, false); + auto s = IsTryingToReplicateItself(svr, host_, port_); + if (!s.IsOK()) { + return {Status::RedisExecErr, s.Msg()}; + } + s = svr->AddMaster(host_, port_, false); if (s.IsOK()) { *output = redis::SimpleString("OK"); LOG(WARNING) << "SLAVE OF " << host_ << ":" << port_ << " enabled (user request from '" << conn->GetAddr() diff --git a/src/common/io_util.cc b/src/common/io_util.cc index a56854d677d..cb30a48047b 100644 --- a/src/common/io_util.cc +++ b/src/common/io_util.cc @@ -99,6 +99,33 @@ Status SockSetTcpKeepalive(int fd, int interval) { return Status::OK(); } +// Lookup IP addresses by hostname +StatusOr> LookupHostByName(const std::string &host) { + addrinfo hints = {}, *servinfo = nullptr; + + hints.ai_family = AF_UNSPEC; + hints.ai_socktype = SOCK_STREAM; + + if (int rv = getaddrinfo(host.c_str(), nullptr, &hints, &servinfo); rv != 0) { + return {Status::NotOK, gai_strerror(rv)}; + } + + auto exit = MakeScopeExit([servinfo] { freeaddrinfo(servinfo); }); + + std::vector ips; + for (auto p = servinfo; p != nullptr; p = p->ai_next) { + char ip[INET6_ADDRSTRLEN] = {}; + if (p->ai_family == AF_INET) { + inet_ntop(p->ai_family, &((struct sockaddr_in *)p->ai_addr)->sin_addr, ip, sizeof(ip)); + } else { + inet_ntop(p->ai_family, &((struct sockaddr_in6 *)p->ai_addr)->sin6_addr, ip, sizeof(ip)); + } + ips.emplace_back(ip); + } + + return ips; +} + StatusOr SockConnect(const std::string &host, uint32_t port, int conn_timeout, int timeout) { addrinfo hints = {}, *servinfo = nullptr; diff --git a/src/common/io_util.h b/src/common/io_util.h index e0d4ade1978..8391d2cc129 100644 --- a/src/common/io_util.h +++ b/src/common/io_util.h @@ -27,6 +27,7 @@ namespace util { sockaddr_in NewSockaddrInet(const std::string &host, uint32_t port); +StatusOr> LookupHostByName(const std::string &host); StatusOr SockConnect(const std::string &host, uint32_t port, int conn_timeout = 0, int timeout = 0); Status SockSetTcpNoDelay(int fd, int val); Status SockSetTcpKeepalive(int fd, int interval); diff --git a/src/server/server.cc b/src/server/server.cc index 21d299a0319..6be0aae761b 100644 --- a/src/server/server.cc +++ b/src/server/server.cc @@ -1751,3 +1751,16 @@ void Server::ResetWatchedKeys(redis::Connection *conn) { watched_key_size_ = watched_key_map_.size(); } } + +std::list> Server::GetSlaveHostAndPort() { + std::list> result; + slave_threads_mu_.lock(); + for (const auto &slave : slave_threads_) { + if (slave->IsStopped()) continue; + std::pair host_port_pair = {slave->GetConn()->GetAnnounceIP(), + slave->GetConn()->GetListeningPort()}; + result.emplace_back(host_port_pair); + } + slave_threads_mu_.unlock(); + return result; +} diff --git a/src/server/server.h b/src/server/server.h index 79f7d2280c5..a34d8cdb32c 100644 --- a/src/server/server.h +++ b/src/server/server.h @@ -229,6 +229,7 @@ class Server { void WatchKey(redis::Connection *conn, const std::vector &keys); static bool IsWatchedKeysModified(redis::Connection *conn); void ResetWatchedKeys(redis::Connection *conn); + std::list> GetSlaveHostAndPort(); #ifdef ENABLE_OPENSSL UniqueSSLContext ssl_ctx; diff --git a/tests/gocase/integration/replication/replication_test.go b/tests/gocase/integration/replication/replication_test.go index 0ade5936065..c38e1fee93f 100644 --- a/tests/gocase/integration/replication/replication_test.go +++ b/tests/gocase/integration/replication/replication_test.go @@ -422,3 +422,32 @@ func TestReplicationAnnounceIP(t *testing.T) { require.Equal(t, "1234", slave0port) }) } + +func TestShouldNotReplicate(t *testing.T) { + master := util.StartServer(t, map[string]string{}) + defer master.Close() + masterClient := master.NewClient() + defer func() { require.NoError(t, masterClient.Close()) }() + + ctx := context.Background() + + slave := util.StartServer(t, map[string]string{}) + defer slave.Close() + slaveClient := slave.NewClient() + defer func() { require.NoError(t, slaveClient.Close()) }() + + t.Run("Setting server as replica of itself should throw error", func(t *testing.T) { + err := slaveClient.SlaveOf(ctx, slave.Host(), fmt.Sprintf("%d", slave.Port())).Err() + require.Equal(t, "ERR can't replicate itself", err.Error()) + require.Equal(t, "master", util.FindInfoEntry(slaveClient, "role")) + }) + + t.Run("Master should not be able to replicate slave", func(t *testing.T) { + util.SlaveOf(t, slaveClient, master) + util.WaitForSync(t, slaveClient) + require.Equal(t, "slave", util.FindInfoEntry(slaveClient, "role")) + err := masterClient.SlaveOf(ctx, slave.Host(), fmt.Sprintf("%d", slave.Port())).Err() + require.EqualErrorf(t, err, "ERR can't replicate your own replicas", err.Error()) + require.Equal(t, "master", util.FindInfoEntry(masterClient, "role")) + }) +} From ad704f57311a9ca734171f0582fcfd67ca091baa Mon Sep 17 00:00:00 2001 From: clundro Date: Tue, 13 Jun 2023 12:48:24 +0800 Subject: [PATCH 2/3] Add support of the new command SINTERCARD(Redis 7) (#1444) --- src/commands/cmd_set.cc | 69 ++++++++++++++++++++++++++ src/commands/error_constants.h | 1 + src/types/redis_set.cc | 46 +++++++++++++++++ src/types/redis_set.h | 1 + tests/cppunit/types/set_test.cc | 45 ++++++++++++++++- tests/gocase/unit/type/set/set_test.go | 69 ++++++++++++++++++++++++++ 6 files changed, 230 insertions(+), 1 deletion(-) diff --git a/src/commands/cmd_set.cc b/src/commands/cmd_set.cc index d6a094305aa..c066e372c6d 100644 --- a/src/commands/cmd_set.cc +++ b/src/commands/cmd_set.cc @@ -18,6 +18,8 @@ * */ +#include + #include "commander.h" #include "commands/scan_base.h" #include "error_constants.h" @@ -292,6 +294,72 @@ class CommandSInter : public Commander { } }; +/* + * description: + * syntax: `SINTERCARD numkeys key [key ...] [LIMIT limit]` + * + * limit: the valid limit is an non-negative integer. + */ +class CommandSInterCard : public Commander { + public: + Status Parse(const std::vector &args) override { + auto parse_numkey = ParseInt(args[1], 10); + if (!parse_numkey) { + return {Status::RedisParseErr, errValueNotInteger}; + } + + if (*parse_numkey <= 0) { + return {Status::RedisParseErr, errValueMustBePositive}; + } + numkeys_ = *parse_numkey; + + // command: for example, SINTERCARD 2 key1 key2 LIMIT 1 + auto arg_sz = args.size(); + if (arg_sz == numkeys_ + 4 && util::ToLower(args[numkeys_ + 2]) == "limit") { + auto parse_limit = ParseInt(args[numkeys_ + 3], 10); + if (!parse_limit) { + return {Status::RedisParseErr, errValueNotInteger}; + } + if (*parse_limit < 0) { + return {Status::RedisParseErr, errLimitIsNegative}; + } + limit_ = *parse_limit; + return Commander::Parse(args); + } + + if (arg_sz != numkeys_ + 2) { + return {Status::RedisParseErr, errWrongNumOfArguments}; + } + return Commander::Parse(args); + } + + Status Execute(Server *svr, Connection *conn, std::string *output) override { + std::vector keys; + for (size_t i = 2; i < numkeys_ + 2; i++) { + keys.emplace_back(args_[i]); + } + + redis::Set set_db(svr->storage, conn->GetNamespace()); + uint64_t ret = 0; + auto s = set_db.InterCard(keys, limit_, &ret); + if (!s.ok()) { + return {Status::RedisExecErr, s.ToString()}; + } + + *output = redis::Integer(ret); + return Status::OK(); + } + + static CommandKeyRange Range(const std::vector &args) { + int num_key = *ParseInt(args[1], 10); + return {2, 1 + num_key, 1}; + } + + private: + uint64_t numkeys_ = 0; + uint64_t limit_ = 0; +}; + class CommandSDiffStore : public Commander { public: Status Execute(Server *svr, Connection *conn, std::string *output) override { @@ -380,6 +448,7 @@ REDIS_REGISTER_COMMANDS(MakeCmdAttr("sadd", -3, "write", 1, 1, 1), MakeCmdAttr("sdiff", -2, "read-only", 1, -1, 1), MakeCmdAttr("sunion", -2, "read-only", 1, -1, 1), MakeCmdAttr("sinter", -2, "read-only", 1, -1, 1), + MakeCmdAttr("sintercard", -3, "read-only", CommandSInterCard::Range), MakeCmdAttr("sdiffstore", -3, "write", 1, -1, 1), MakeCmdAttr("sunionstore", -3, "write", 1, -1, 1), MakeCmdAttr("sinterstore", -3, "write", 1, -1, 1), diff --git a/src/commands/error_constants.h b/src/commands/error_constants.h index bfba200a58e..94858289c06 100644 --- a/src/commands/error_constants.h +++ b/src/commands/error_constants.h @@ -32,6 +32,7 @@ inline constexpr const char *errNoSuchKey = "no such key"; inline constexpr const char *errUnbalancedStreamList = "Unbalanced XREAD list of streams: for each stream key an ID or '$' must be specified."; inline constexpr const char *errTimeoutIsNegative = "timeout is negative"; +inline constexpr const char *errLimitIsNegative = "LIMIT can't be negative"; inline constexpr const char *errLimitOptionNotAllowed = "syntax error, LIMIT cannot be used without the special ~ option"; inline constexpr const char *errZSetLTGTNX = "GT, LT, and/or NX options at the same time are not compatible"; diff --git a/src/types/redis_set.cc b/src/types/redis_set.cc index de5f1e8d33d..0a26c9f9161 100644 --- a/src/types/redis_set.cc +++ b/src/types/redis_set.cc @@ -358,6 +358,52 @@ rocksdb::Status Set::Inter(const std::vector &keys, std::vector &keys, uint64_t limit, uint64_t *cardinality) { + *cardinality = 0; + + std::map member_counters; + std::vector target_members; + + auto s = Members(keys[0], &target_members); + if (!s.ok() || target_members.empty()) return s; + for (const auto &member : target_members) { + member_counters[member] = 1; + } + if (limit == 0) { + limit = target_members.size(); + } + + size_t keys_size = keys.size(); + if (keys_size == 1) { + *cardinality = std::min(static_cast(target_members.size()), limit); + return rocksdb::Status::OK(); + } + + bool limit_reached = false; + for (size_t i = 1; i < keys_size; i++) { + s = Members(keys[i], &target_members); + if (!s.ok() || target_members.empty()) { + return s; + } + + for (const auto &member : target_members) { + auto iter = member_counters.find(member); + if (iter == member_counters.end()) continue; + if (++iter->second == keys_size) { + *cardinality += 1; + if (--limit == 0) { + limit_reached = true; + break; + } + } + } + + if (limit_reached) break; + } + + return rocksdb::Status::OK(); +} + rocksdb::Status Set::DiffStore(const Slice &dst, const std::vector &keys, uint64_t *saved_cnt) { *saved_cnt = 0; std::vector members; diff --git a/src/types/redis_set.h b/src/types/redis_set.h index 28ab85de1da..739c3077c35 100644 --- a/src/types/redis_set.h +++ b/src/types/redis_set.h @@ -43,6 +43,7 @@ class Set : public SubKeyScanner { rocksdb::Status Diff(const std::vector &keys, std::vector *members); rocksdb::Status Union(const std::vector &keys, std::vector *members); rocksdb::Status Inter(const std::vector &keys, std::vector *members); + rocksdb::Status InterCard(const std::vector &keys, uint64_t limit, uint64_t *cardinality); rocksdb::Status Overwrite(Slice user_key, const std::vector &members); rocksdb::Status DiffStore(const Slice &dst, const std::vector &keys, uint64_t *saved_cnt); rocksdb::Status UnionStore(const Slice &dst, const std::vector &keys, uint64_t *save_cnt); diff --git a/tests/cppunit/types/set_test.cc b/tests/cppunit/types/set_test.cc index 26dbb4385ed..c6d78b75db7 100644 --- a/tests/cppunit/types/set_test.cc +++ b/tests/cppunit/types/set_test.cc @@ -178,19 +178,62 @@ TEST_F(RedisSetTest, Union) { TEST_F(RedisSetTest, Inter) { uint64_t ret = 0; - std::string k1 = "key1", k2 = "key2", k3 = "key3"; + std::string k1 = "key1", k2 = "key2", k3 = "key3", k4 = "key4", k5 = "key5"; rocksdb::Status s = set_->Add(k1, {"a", "b", "c", "d"}, &ret); EXPECT_EQ(ret, 4); set_->Add(k2, {"c"}, &ret); EXPECT_EQ(ret, 1); set_->Add(k3, {"a", "c", "e"}, &ret); EXPECT_EQ(ret, 3); + set_->Add(k5, {"a"}, &ret); + EXPECT_EQ(ret, 1); std::vector members; set_->Inter({k1, k2, k3}, &members); EXPECT_EQ(1, members.size()); + members.clear(); + set_->Inter({k1, k2, k4}, &members); + EXPECT_EQ(0, members.size()); + set_->Inter({k1, k4, k5}, &members); + EXPECT_EQ(0, members.size()); + set_->Del(k1); + set_->Del(k2); + set_->Del(k3); + set_->Del(k4); + set_->Del(k5); +} + +TEST_F(RedisSetTest, InterCard) { + uint64_t ret = 0; + std::string k1 = "key1", k2 = "key2", k3 = "key3", k4 = "key4"; + rocksdb::Status s = set_->Add(k1, {"a", "b", "c", "d"}, &ret); + EXPECT_EQ(ret, 4); + set_->Add(k2, {"c", "d", "e"}, &ret); + EXPECT_EQ(ret, 3); + set_->Add(k3, {"e", "f"}, &ret); + EXPECT_EQ(ret, 2); + set_->InterCard({k1, k2}, 0, &ret); + EXPECT_EQ(ret, 2); + set_->InterCard({k1, k2}, 1, &ret); + EXPECT_EQ(ret, 1); + set_->InterCard({k1, k2}, 3, &ret); + EXPECT_EQ(ret, 2); + set_->InterCard({k2, k3}, 1, &ret); + EXPECT_EQ(ret, 1); + set_->InterCard({k1, k3}, 5, &ret); + EXPECT_EQ(ret, 0); + set_->InterCard({k1, k4}, 5, &ret); + EXPECT_EQ(ret, 0); + set_->InterCard({k1}, 0, &ret); + EXPECT_EQ(ret, 4); + for (uint32_t i = 1; i < 20; i++) { + set_->InterCard({k1}, i, &ret); + uint64_t val = (i >= 4) ? 4 : i; + EXPECT_EQ(ret, val); + } set_->Del(k1); set_->Del(k2); set_->Del(k3); + set_->Del(k4); } TEST_F(RedisSetTest, Overwrite) { diff --git a/tests/gocase/unit/type/set/set_test.go b/tests/gocase/unit/type/set/set_test.go index b7fa08d03ee..61ecc2a1756 100644 --- a/tests/gocase/unit/type/set/set_test.go +++ b/tests/gocase/unit/type/set/set_test.go @@ -229,6 +229,34 @@ func TestSet(t *testing.T) { } }) + t.Run("SINTERCARD with a single set - "+dstype, func(t *testing.T) { + cmd1 := rdb.SInter(ctx, "set1") + require.NoError(t, cmd1.Err()) + for j := 1; j < 20; j++ { + cmd2 := rdb.SInterCard(ctx, int64(j), "set1") + require.NoError(t, cmd2.Err()) + if j > len(cmd1.Val()) { + require.EqualValues(t, cmd2.Val(), len(cmd1.Val())) + } else { + require.EqualValues(t, cmd2.Val(), j) + } + } + }) + + t.Run("SINTERCARD with two sets - "+dstype, func(t *testing.T) { + cmd1 := rdb.SInter(ctx, "set1", "set2") + require.NoError(t, cmd1.Err()) + for j := 1; j < 20; j++ { + cmd2 := rdb.SInterCard(ctx, int64(j), "set1", "set2") + require.NoError(t, cmd2.Err()) + if j > len(cmd1.Val()) { + require.EqualValues(t, cmd2.Val(), len(cmd1.Val())) + } else { + require.EqualValues(t, cmd2.Val(), j) + } + } + }) + t.Run("SINTERSTORE with two sets - "+dstype, func(t *testing.T) { require.NoError(t, rdb.SInterStore(ctx, "setres", "set1", "set2").Err()) cmd := rdb.SMembers(ctx, "setres") @@ -282,6 +310,20 @@ func TestSet(t *testing.T) { } }) + t.Run("SINTERCARD with three sets - "+dstype, func(t *testing.T) { + cmd1 := rdb.SInter(ctx, "set1", "set2", "set3") + require.NoError(t, cmd1.Err()) + for j := 1; j < 20; j++ { + cmd2 := rdb.SInterCard(ctx, int64(j), "set1", "set2", "set3") + require.NoError(t, cmd2.Err()) + if j > len(cmd1.Val()) { + require.EqualValues(t, cmd2.Val(), len(cmd1.Val())) + } else { + require.EqualValues(t, cmd2.Val(), j) + } + } + }) + t.Run("SINTERSTORE with three sets - "+dstype, func(t *testing.T) { require.NoError(t, rdb.SInterStore(ctx, "setres", "set1", "set2", "set3").Err()) cmd := rdb.SMembers(ctx, "setres") @@ -399,6 +441,33 @@ func TestSet(t *testing.T) { require.EqualValues(t, []string{}, rdb.SInter(ctx, "set1", "set2", "key3").Val()) }) + t.Run("SINTERCARD with wrong args", func(t *testing.T) { + CreateSet(t, rdb, ctx, "set1", []interface{}{"foo", "2"}) + CreateSet(t, rdb, ctx, "set2", []interface{}{"a", "b", "2"}) + + // numkey should be a positive integer. + require.ErrorContains(t, rdb.Do(ctx, "sintercard", "3.5", "set1").Err(), "is not an integer") + require.ErrorContains(t, rdb.Do(ctx, "sintercard", "-1", "set1").Err(), "must be positive") + require.ErrorContains(t, rdb.Do(ctx, "sintercard", "0", "set1").Err(), "must be positive") + + require.ErrorContains(t, rdb.Do(ctx, "sintercard", "1", "set1", "set2").Err(), "wrong number of arguments") + require.ErrorContains(t, rdb.Do(ctx, "sintercard", "1", "set1", "set2", "LIMIT", "2").Err(), "wrong number of arguments") + + // The limit argument should be a positive integer. + require.ErrorContains(t, rdb.Do(ctx, "sintercard", "2", "set1", "set2", "LIMIT", "-1").Err(), "can't be negative") + require.ErrorContains(t, rdb.Do(ctx, "sintercard", "2", "set1", "set2", "LIMIT", "1.5").Err(), "is not an integer") + }) + + t.Run("SINTERCARD should return zero with empty sets", func(t *testing.T) { + require.NoError(t, rdb.Del(ctx, "set1", "set2", "set3").Err()) + require.NoError(t, rdb.SAdd(ctx, "set1", "a", "b", "c").Err()) + require.NoError(t, rdb.SAdd(ctx, "set2", "b", "c", "d").Err()) + for j := 1; j < 20; j++ { + require.EqualValues(t, 0, rdb.SInterCard(ctx, int64(j), "set1", "set2", "key3").Val()) + } + require.EqualValues(t, 0, rdb.SInterCard(ctx, 0, "set3").Val()) + }) + t.Run("SINTER with same integer elements but different encoding", func(t *testing.T) { require.NoError(t, rdb.Del(ctx, "set1", "set2").Err()) require.NoError(t, rdb.SAdd(ctx, "set1", 1, 2, 3).Err()) From e89af5289d5c3d12c47628394c72f0d67e652800 Mon Sep 17 00:00:00 2001 From: hulk Date: Tue, 13 Jun 2023 16:21:10 +0800 Subject: [PATCH 3/3] Fix data race when joining the task runner (#1493) --- src/common/task_runner.cc | 3 ++- src/server/server.cc | 6 +++--- tests/cppunit/task_runner_test.cc | 2 ++ 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/common/task_runner.cc b/src/common/task_runner.cc index 318d5800bd6..810a66c580c 100644 --- a/src/common/task_runner.cc +++ b/src/common/task_runner.cc @@ -45,7 +45,8 @@ Status TaskRunner::Join() { for (auto &thread : threads_) { if (auto s = util::ThreadJoin(thread); !s) { - return s.Prefixed("Task thread operation failed"); + LOG(WARNING) << "Failed to join thread: " << s.Msg(); + continue; } } diff --git a/src/server/server.cc b/src/server/server.cc index 6be0aae761b..a9a117d9a30 100644 --- a/src/server/server.cc +++ b/src/server/server.cc @@ -230,15 +230,15 @@ void Server::Join() { worker->Join(); } - if (auto s = task_runner_.Join(); !s) { - LOG(WARNING) << s.Msg(); - } if (auto s = util::ThreadJoin(cron_thread_); !s) { LOG(WARNING) << "Cron thread operation failed: " << s.Msg(); } if (auto s = util::ThreadJoin(compaction_checker_thread_); !s) { LOG(WARNING) << "Compaction checker thread operation failed: " << s.Msg(); } + if (auto s = task_runner_.Join(); !s) { + LOG(WARNING) << s.Msg(); + } } Status Server::AddMaster(const std::string &host, uint32_t port, bool force_reconnect) { diff --git a/tests/cppunit/task_runner_test.cc b/tests/cppunit/task_runner_test.cc index 8407adbfa23..2d4ab30809b 100644 --- a/tests/cppunit/task_runner_test.cc +++ b/tests/cppunit/task_runner_test.cc @@ -103,4 +103,6 @@ TEST(TaskRunner, PublishAfterStart) { std::this_thread::sleep_for(0.1s); ASSERT_EQ(counter, 20); + tr.Cancel(); + _ = tr.Join(); }