From 4ea825077b8c17b204a488040b9eb5afdf53754f Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Mon, 7 Feb 2022 09:03:34 -0700 Subject: [PATCH 01/24] Added public interfaces --- fdbrpc/FlowTransport.h | 1 + fdbrpc/fdbrpc.h | 23 +++++++++++++++-------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/fdbrpc/FlowTransport.h b/fdbrpc/FlowTransport.h index 24daae400a..ab132d78f4 100644 --- a/fdbrpc/FlowTransport.h +++ b/fdbrpc/FlowTransport.h @@ -129,6 +129,7 @@ class NetworkMessageReceiver { public: virtual void receive(ArenaObjectReader&) = 0; virtual bool isStream() const { return false; } + virtual bool isPublic() const = 0; virtual PeerCompatibilityPolicy peerCompatibilityPolicy() const { return { RequirePeer::Exactly, g_network->protocolVersion() }; } diff --git a/fdbrpc/fdbrpc.h b/fdbrpc/fdbrpc.h index 70e199289b..c7c5b21480 100644 --- a/fdbrpc/fdbrpc.h +++ b/fdbrpc/fdbrpc.h @@ -110,6 +110,8 @@ struct NetSAV final : SAV, FlowReceiver, FastAllocated> { SAV::sendAndDelPromiseRef(message.get().asUnderlyingType()); } } + + bool isPublic() const override { return true; } }; template @@ -290,6 +292,8 @@ struct AcknowledgementReceiver final : FlowReceiver, FastAllocated message; reader.deserialize(message); @@ -336,6 +340,8 @@ struct NetNotifiedQueueWithAcknowledgements final : NotifiedQueue, operation_obsolete()); } + bool isPublic() const override { return true; } + void destroy() override { delete this; } void receive(ArenaObjectReader& reader) override { this->addPromiseRef(); @@ -602,10 +608,10 @@ struct serializable_traits> : std::true_type { } }; -template -struct NetNotifiedQueue final : NotifiedQueue, FlowReceiver, FastAllocated> { - using FastAllocated>::operator new; - using FastAllocated>::operator delete; +template +struct NetNotifiedQueue final : NotifiedQueue, FlowReceiver, FastAllocated> { + using FastAllocated>::operator new; + using FastAllocated>::operator delete; NetNotifiedQueue(int futures, int promises) : NotifiedQueue(futures, promises) {} NetNotifiedQueue(int futures, int promises, const Endpoint& remoteEndpoint) @@ -620,9 +626,10 @@ struct NetNotifiedQueue final : NotifiedQueue, FlowReceiver, FastAllocateddelPromiseRef(); } bool isStream() const override { return true; } + bool isPublic() const override { return IsPublic; } }; -template +template class RequestStream { public: // stream.send( request ) @@ -778,13 +785,13 @@ public: return getReplyUnlessFailedFor(ReplyPromise(), sustainedFailureDuration, sustainedFailureSlope); } - explicit RequestStream(const Endpoint& endpoint) : queue(new NetNotifiedQueue(0, 1, endpoint)) {} + explicit RequestStream(const Endpoint& endpoint) : queue(new NetNotifiedQueue(0, 1, endpoint)) {} FutureStream getFuture() const { queue->addFutureRef(); return FutureStream(queue); } - RequestStream() : queue(new NetNotifiedQueue(0, 1)) {} + RequestStream() : queue(new NetNotifiedQueue(0, 1)) {} explicit RequestStream(PeerCompatibilityPolicy policy) : RequestStream() { queue->setPeerCompatibilityPolicy(policy); } @@ -828,7 +835,7 @@ public: } private: - NetNotifiedQueue* queue; + NetNotifiedQueue* queue; }; template From c7899b9d3965629ef7fddcc0a5586b05f03b481b Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Wed, 16 Feb 2022 18:12:03 +0100 Subject: [PATCH 02/24] Implemented public endpoints, started allow list --- fdbclient/CommitProxyInterface.h | 6 +- fdbclient/CoordinationInterface.h | 4 +- fdbclient/GrvProxyInterface.h | 2 +- fdbclient/StorageServerInterface.h | 24 +++---- fdbrpc/FlowTransport.actor.cpp | 60 +++++++++++++++-- fdbrpc/TenantInfo.h | 73 +++++++++++++++++++++ fdbserver/fdbserver.actor.cpp | 102 ++++++++++++++++++++++++++++- flow/ObjectSerializer.h | 39 ++++++++++- flow/error_definitions.h | 4 ++ tests/fast/CycleTest.toml | 32 --------- 10 files changed, 289 insertions(+), 57 deletions(-) create mode 100644 fdbrpc/TenantInfo.h diff --git a/fdbclient/CommitProxyInterface.h b/fdbclient/CommitProxyInterface.h index c46bdcb709..102b5a2088 100644 --- a/fdbclient/CommitProxyInterface.h +++ b/fdbclient/CommitProxyInterface.h @@ -42,13 +42,13 @@ struct CommitProxyInterface { Optional processId; bool provisional; - RequestStream commit; - RequestStream + RequestStream commit; + RequestStream getConsistentReadVersion; // Returns a version which (1) is committed, and (2) is >= the latest version reported // committed (by a commit response) when this request was sent // (at some point between when this request is sent and when its response is // received, the latest version reported committed) - RequestStream getKeyServersLocations; + RequestStream getKeyServersLocations; RequestStream getStorageServerRejoinInfo; RequestStream> waitFailure; diff --git a/fdbclient/CoordinationInterface.h b/fdbclient/CoordinationInterface.h index 9cf938931b..936ec82ccf 100644 --- a/fdbclient/CoordinationInterface.h +++ b/fdbclient/CoordinationInterface.h @@ -32,8 +32,8 @@ const int MAX_CLUSTER_FILE_BYTES = 60000; struct ClientLeaderRegInterface { - RequestStream getLeader; - RequestStream openDatabase; + RequestStream getLeader; + RequestStream openDatabase; RequestStream checkDescriptorMutable; ClientLeaderRegInterface() {} diff --git a/fdbclient/GrvProxyInterface.h b/fdbclient/GrvProxyInterface.h index d4b3b78bcb..a555f2efd5 100644 --- a/fdbclient/GrvProxyInterface.h +++ b/fdbclient/GrvProxyInterface.h @@ -36,7 +36,7 @@ struct GrvProxyInterface { Optional processId; bool provisional; - RequestStream + RequestStream getConsistentReadVersion; // Returns a version which (1) is committed, and (2) is >= the latest version reported // committed (by a commit response) when this request was sent // (at some point between when this request is sent and when its response is received, the latest version reported diff --git a/fdbclient/StorageServerInterface.h b/fdbclient/StorageServerInterface.h index 9e40f95e1f..3da2ad402b 100644 --- a/fdbclient/StorageServerInterface.h +++ b/fdbclient/StorageServerInterface.h @@ -60,30 +60,30 @@ struct StorageServerInterface { UID uniqueID; Optional tssPairID; - RequestStream getValue; - RequestStream getKey; + RequestStream getValue; + RequestStream getKey; // Throws a wrong_shard_server if the keys in the request or result depend on data outside this server OR if a large // selector offset prevents all data from being read in one range read - RequestStream getKeyValues; - RequestStream getKeyValuesAndFlatMap; + RequestStream getKeyValues; + RequestStream getKeyValuesAndFlatMap; - RequestStream getShardState; + RequestStream getShardState; RequestStream waitMetrics; RequestStream splitMetrics; RequestStream getStorageMetrics; - RequestStream> waitFailure; + RequestStream, true> waitFailure; RequestStream getQueuingMetrics; RequestStream> getKeyValueStoreType; - RequestStream watchValue; + RequestStream watchValue; RequestStream getReadHotRanges; RequestStream getRangeSplitPoints; - RequestStream getKeyValuesStream; - RequestStream changeFeedStream; - RequestStream overlappingChangeFeeds; - RequestStream changeFeedPop; - RequestStream changeFeedVersionUpdate; + RequestStream getKeyValuesStream; + RequestStream changeFeedStream; + RequestStream overlappingChangeFeeds; + RequestStream changeFeedPop; + RequestStream changeFeedVersionUpdate; explicit StorageServerInterface(UID uid) : uniqueID(uid) {} StorageServerInterface() : uniqueID(deterministicRandom()->randomUniqueID()) {} diff --git a/fdbrpc/FlowTransport.actor.cpp b/fdbrpc/FlowTransport.actor.cpp index 26b6e89b87..851e3c084f 100644 --- a/fdbrpc/FlowTransport.actor.cpp +++ b/fdbrpc/FlowTransport.actor.cpp @@ -27,6 +27,7 @@ #include #endif +#include "fdbrpc/TenantInfo.h" #include "fdbrpc/fdbrpc.h" #include "fdbrpc/FailureMonitor.h" #include "fdbrpc/HealthMonitor.h" @@ -204,6 +205,7 @@ struct EndpointNotFoundReceiver final : NetworkMessageReceiver { Endpoint e = FlowTransport::transport().loadedEndpoint(token); IFailureMonitor::failureMonitor().endpointNotFound(e); } + bool isPublic() const override { return true; } }; struct PingRequest { @@ -228,6 +230,29 @@ struct PingReceiver final : NetworkMessageReceiver { PeerCompatibilityPolicy peerCompatibilityPolicy() const override { return PeerCompatibilityPolicy{ RequirePeer::AtLeast, ProtocolVersion::withStableInterfaces() }; } + bool isPublic() const override { return true; } +}; + +struct TenantAuthorizer final : NetworkMessageReceiver { + void receive(ArenaObjectReader& reader) override { + AuthorizationRequest req; + try { + reader.deserialize(req); + // TODO: verify that token is valid + AuthorizedTenants& auth = reader.variable("AuthorizedTenants"); + for (auto const& t : req.tenants) { + auth.authorizedTenants.insert(TenantInfoRef(auth.arena, t)); + } + req.reply.send(Void()); + } catch (Error& e) { + if (e.code() == error_code_permission_denied) { + req.reply.sendError(e); + } else { + throw; + } + } + } + bool isPublic() const override { return true; } }; class TransportData { @@ -918,6 +943,8 @@ ACTOR static void deliver(TransportData* self, Endpoint destination, TaskPriority priority, ArenaReader reader, + NetworkAddress peerAddress, + AuthorizedTenants authorizedTenants, bool inReadSocket) { // We want to run the task at the right priority. If the priority is higher than the current priority (which is // ReadSocket) we can just upgrade. Otherwise we'll context switch so that we don't block other tasks that might run @@ -932,6 +959,11 @@ ACTOR static void deliver(TransportData* self, auto receiver = self->endpoints.get(destination.token); if (receiver) { + if (!authorizedTenants.trusted && !receiver->isPublic()) { + TraceEvent(SevWarnAlways, "AttemptedRPCToPrivatePrevented") + .detail("From", peerAddress); + throw connection_failed(); + } if (!checkCompatible(receiver->peerCompatibilityPolicy(), reader.protocolVersion())) { return; } @@ -940,6 +972,9 @@ ACTOR static void deliver(TransportData* self, StringRef data = reader.arenaReadAll(); ASSERT(data.size() > 8); ArenaObjectReader objReader(reader.arena(), reader.arenaReadAll(), AssumeVersion(reader.protocolVersion())); + bool didInsert = objReader.variable("AuthorizedTenants", &authorizedTenants); + didInsert = didInsert && objReader.variable("PeerAddress", &peerAddress); + ASSERT(didInsert); // check that we could set both context variables receiver->receive(objReader); g_currentDeliveryPeerAddress = { NetworkAddress() }; } catch (Error& e) { @@ -977,6 +1012,7 @@ static void scanPackets(TransportData* transport, const uint8_t* e, Arena& arena, NetworkAddress const& peerAddress, + AuthorizedTenants authorizedTenants, ProtocolVersion peerProtocolVersion) { // Find each complete packet in the given byte range and queue a ready task to deliver it. // Remove the complete packets from the range by increasing unprocessed_begin. @@ -1090,7 +1126,13 @@ static void scanPackets(TransportData* transport, // we have many messages to UnknownEndpoint we want to optimize earlier. As deliver is an actor it // will allocate some state on the heap and this prevents it from doing that. if (priority != TaskPriority::UnknownEndpoint || (token.first() & TOKEN_STREAM_FLAG) != 0) { - deliver(transport, Endpoint({ peerAddress }, token), priority, std::move(reader), true); + deliver(transport, + Endpoint({ peerAddress }, token), + priority, + std::move(reader), + peerAddress, + authorizedTenants, + true); } unprocessed_begin = p = p + packetLen; @@ -1136,8 +1178,11 @@ ACTOR static Future connectionReader(TransportData* transport, state bool incompatibleProtocolVersionNewer = false; state NetworkAddress peerAddress; state ProtocolVersion peerProtocolVersion; + state AuthorizedTenants authorizedTenants; peerAddress = conn->getPeerAddress(); + // TODO: check whether peers ip is in trusted range + authorizedTenants.trusted = true; if (!peer) { ASSERT(!peerAddress.isPublic()); } @@ -1286,7 +1331,7 @@ ACTOR static Future connectionReader(TransportData* transport, if (!expectConnectPacket) { if (compatible || peerProtocolVersion.hasStableInterfaces()) { scanPackets( - transport, unprocessed_begin, unprocessed_end, arena, peerAddress, peerProtocolVersion); + transport, unprocessed_begin, unprocessed_end, arena, peerAddress, authorizedTenants, peerProtocolVersion); } else { unprocessed_begin = unprocessed_end; peer->resetPing.trigger(); @@ -1554,8 +1599,15 @@ static void sendLocal(TransportData* self, ISerializeSource const& what, const E ASSERT(copy.size() > 0); TaskPriority priority = self->endpoints.getPriority(destination.token); if (priority != TaskPriority::UnknownEndpoint || (destination.token.first() & TOKEN_STREAM_FLAG) != 0) { - deliver( - self, destination, priority, ArenaReader(copy.arena(), copy, AssumeVersion(currentProtocolVersion)), false); + AuthorizedTenants authorizedTenants; + authorizedTenants.trusted = true; + deliver(self, + destination, + priority, + ArenaReader(copy.arena(), copy, AssumeVersion(currentProtocolVersion)), + NetworkAddress(), + authorizedTenants, + false); } } diff --git a/fdbrpc/TenantInfo.h b/fdbrpc/TenantInfo.h new file mode 100644 index 0000000000..ad350f5b39 --- /dev/null +++ b/fdbrpc/TenantInfo.h @@ -0,0 +1,73 @@ +/* + * TenantInfo.h + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2022 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once +#ifndef FDBRPC_TENANTINFO_H_ +#define FDBRPC_TENANTINFO_H_ +#include "flow/Arena.h" +#include "fdbrpc/fdbrpc.h" +#include + +struct TenantInfoRef { + TenantInfoRef() {} + TenantInfoRef(Arena& p, StringRef toCopy) : tenantName(StringRef(p, toCopy)) {} + TenantInfoRef(Arena& p, TenantInfoRef toCopy) + : tenantName(toCopy.tenantName.present() ? Optional(StringRef(p, toCopy.tenantName.get())) + : Optional()) {} + // Empty tenant name means that the peer is trusted + Optional tenantName; + + bool operator<(TenantInfoRef const& other) const { + if (!other.tenantName.present()) { + return false; + } + if (!tenantName.present()) { + return true; + } + return tenantName.get() < other.tenantName.get(); + } + + template + void serialize(Ar& ar) { + serializer(ar, tenantName); + } +}; + +struct AuthorizedTenants { + Arena arena; + std::set authorizedTenants; + bool trusted = false; +}; + +// TODO: receive and validate token instead +struct AuthorizationRequest { + constexpr static FileIdentifier file_identifier = 11499331; + + Arena arena; + VectorRef tenants; + ReplyPromise reply; + + template + void serialize(Ar& ar) { + serializer(ar, tenants, reply, arena); + } +}; + +#endif // FDBRPC_TENANTINFO_H_ diff --git a/fdbserver/fdbserver.actor.cpp b/fdbserver/fdbserver.actor.cpp index 8bcd61c943..4f5d7a26a9 100644 --- a/fdbserver/fdbserver.actor.cpp +++ b/fdbserver/fdbserver.actor.cpp @@ -101,6 +101,7 @@ enum { OPT_TRACECLOCK, OPT_NUMTESTERS, OPT_DEVHELP, OPT_ROLLSIZE, OPT_MAXLOGS, OPT_MAXLOGSSIZE, OPT_KNOB, OPT_UNITTESTPARAM, OPT_TESTSERVERS, OPT_TEST_ON_SERVERS, OPT_METRICSCONNFILE, OPT_METRICSPREFIX, OPT_LOGGROUP, OPT_LOCALITY, OPT_IO_TRUST_SECONDS, OPT_IO_TRUST_WARN_ONLY, OPT_FILESYSTEM, OPT_PROFILER_RSS_SIZE, OPT_KVFILE, OPT_TRACE_FORMAT, OPT_WHITELIST_BINPATH, OPT_BLOB_CREDENTIAL_FILE, OPT_CONFIG_PATH, OPT_USE_TEST_CONFIG_DB, OPT_FAULT_INJECTION, OPT_PROFILER, OPT_PRINT_SIMTIME, + OPT_IP_TRUSTED_MASK, }; CSimpleOpt::SOption g_rgOptions[] = { @@ -188,7 +189,8 @@ CSimpleOpt::SOption g_rgOptions[] = { { OPT_FAULT_INJECTION, "-fi", SO_REQ_SEP }, { OPT_FAULT_INJECTION, "--fault-injection", SO_REQ_SEP }, { OPT_PROFILER, "--profiler-", SO_REQ_SEP}, - { OPT_PRINT_SIMTIME, "--print-sim-time", SO_NONE }, + { OPT_PRINT_SIMTIME, "--print-sim-time", SO_NONE }, + { OPT_IP_TRUSTED_MASK, "--trusted-subnet-", SO_REQ_SEP }, #ifndef TLS_DISABLED TLS_OPTION_FLAGS @@ -294,7 +296,7 @@ UID getSharedMemoryMachineId() { std::string sharedMemoryIdentifier = "fdbserver_shared_memory_id"; loop { try { - // "0" is the default parameter "addr" + // "0" is the default netPrefix "addr" boost::interprocess::managed_shared_memory segment( boost::interprocess::open_or_create, sharedMemoryIdentifier.c_str(), 1000, 0, p.permission); machineId = segment.find_or_construct("machineId")(deterministicRandom()->randomUniqueID()); @@ -1666,7 +1668,103 @@ private: }; } // namespace +#include + +struct AuthAllowedSubnet { + IPAddress baseAddress; + IPAddress addressMask; + + AuthAllowedSubnet(IPAddress const& baseAddress, IPAddress const& addressMask) + : baseAddress(baseAddress), addressMask(addressMask) { + ASSERT(baseAddress.isV4() == addressMask.isV4()); + } + + static AuthAllowedSubnet fromString(std::string_view addressString) { + auto pos = addressString.find('/'); + if (pos == std::string_view::npos) { + fmt::print("ERROR: {} is not a valid (use Network-Prefix/hostcount syntax)\n"); + throw invalid_option(); + } + auto address = addressString.substr(0, pos); + auto hostCount = std::stoi(std::string(addressString.substr(pos + 1))); + auto addr = boost::asio::ip::make_address(address); + if (addr.is_v4()) { + auto bM = createBitMask(addr.to_v4().to_bytes(), hostCount); + // we typically would expect a base address has been passed, but to be safe we still + // will make the last bits 0 + auto mask = boost::asio::ip::address_v4(bM).to_uint(); + auto baseAddress = addr.to_v4().to_uint() & mask; + return AuthAllowedSubnet(IPAddress(baseAddress), IPAddress(mask)); + } else { + auto mask = createBitMask(addr.to_v6().to_bytes(), hostCount); + auto baseAddress = addr.to_v6().to_bytes(); + for (int i = 0; i < mask.size(); ++i) { + baseAddress[i] &= mask[i]; + } + return AuthAllowedSubnet(IPAddress(baseAddress), IPAddress(mask)); + } + } + + template + static auto createBitMask(std::array const& addr, int hostCount) -> std::array { + std::array res; + res.fill((unsigned char)0xff); + for (auto idx = (hostCount / 8) - 1; idx < res.size(); ++idx) { + if (hostCount > 0) { + // 2^(hostCount % 8) - 1 sets the last (hostCount % 8) number of bits to 1 + // everything else will be zero. For example: 2^3 - 1 == 7 == 0b111 + unsigned char bitmask = (1 << (hostCount % 8)) - ((unsigned char)1); + res[idx] ^= bitmask; + } else { + res[idx] = (unsigned char)0; + } + hostCount = 0; + } + return res; + } + + bool operator() (IPAddress const& address) const { + if (addressMask.isV4() != address.isV4()) { + return false; + } + if (addressMask.isV4()) { + return (addressMask.toV4() & address.toV4()) == baseAddress.toV4(); + } else { + auto res = address.toV6(); + auto const& mask = addressMask.toV6(); + for (int i = 0; i < res.size(); ++i) { + res[i] &= mask[i]; + } + return res == baseAddress.toV6(); + } + } +}; + +template +void printIP(std::array const& addr) { + for (auto c : addr) { + fmt::print(" {:02x}", int(c)); + } +} + +void printIP(std::string_view txt, IPAddress const& address) { + fmt::print("{}:", txt); + if (address.isV4()) { + printIP(boost::asio::ip::address_v4(address.toV4()).to_bytes()); + } else { + printIP(address.toV6()); + } + fmt::print("\n"); +} + int main(int argc, char* argv[]) { + //auto allowed = AuthAllowedSubnet::fromString(argv[1]); + //printIP("Base Address", allowed.baseAddress); + //printIP("Address Mask", allowed.addressMask); + //for (int idx = 1; idx < argc; ++idx) { + // auto addr = IPAddress::parse(argv[idx]); + //} + //return 0; // TODO: Remove later, this is just to force the statics to be initialized // otherwise the unit test won't run #ifdef ENABLE_SAMPLING diff --git a/flow/ObjectSerializer.h b/flow/ObjectSerializer.h index efb5dc0fd7..45f4fac6f3 100644 --- a/flow/ObjectSerializer.h +++ b/flow/ObjectSerializer.h @@ -24,9 +24,13 @@ #include "flow/flat_buffers.h" #include "flow/ProtocolVersion.h" +#include + template struct LoadContext { Ar* ar; + std::unordered_map variables; + LoadContext(Ar* ar) : ar(ar) {} Arena& arena() { return ar->arena(); } @@ -47,6 +51,23 @@ struct LoadContext { void addArena(Arena& arena) { arena = ar->arena(); } LoadContext& context() { return *this; } + + template + bool variable(std::string_view name, T* val) { + auto p = variables.insert(std::make_pair(name, val)); + return p.second; + } + + template + T& variable(std::string_view name) { + auto res = variables.at(name); + return *reinterpret_cast(res); + } + + template + T const& variable(std::string_view name) const { + return const_cast*>(this)->variable(name); + } }; template @@ -69,15 +90,16 @@ template class _ObjectReader { protected: ProtocolVersion mProtocolVersion; + LoadContext context; public: + _ObjectReader() : context(static_cast(this)) {} ProtocolVersion protocolVersion() const { return mProtocolVersion; } void setProtocolVersion(ProtocolVersion v) { mProtocolVersion = v; } template void deserialize(FileIdentifier file_identifier, Items&... items) { const uint8_t* data = static_cast(this)->data(); - LoadContext context(static_cast(this)); if (read_file_identifier(data) != file_identifier) { // Some file identifiers are changed in 7.0, so file identifier mismatches // are expected during a downgrade from 7.0 to 6.3 @@ -100,6 +122,21 @@ public: void deserialize(Item& item) { deserialize(FileIdentifierFor::value, item); } + + template + bool variable(std::string_view name, T* val) { + return context.template variable(name, val); + } + + template + T& variable(std::string_view name) { + return context.template variable(name); + } + + template + T const& variable(std::string_view name) const { + return context.template variable(name); + } }; class ObjectReader : public _ObjectReader { diff --git a/flow/error_definitions.h b/flow/error_definitions.h index c25a386fdb..5cd99f1740 100755 --- a/flow/error_definitions.h +++ b/flow/error_definitions.h @@ -273,6 +273,10 @@ ERROR( snap_invalid_uid_string, 2509, "The given uid string is not a 32-length h // 4xxx Internal errors (those that should be generated only by bugs) are decimal 4xxx ERROR( unknown_error, 4000, "An unknown error occurred" ) // C++ exception not of type Error ERROR( internal_error, 4100, "An internal error occurred" ) + +// 5xxx Authorization and authentication error codes +ERROR( permission_denied, 5000, "Client is not allowed to access endpoint" ) +ERROR( unauthorized_attempt, 5001, "A untrusted client tried to send a message to a private endpoint" ) // clang-format on #undef ERROR diff --git a/tests/fast/CycleTest.toml b/tests/fast/CycleTest.toml index a97dfd9969..ad61effc4f 100644 --- a/tests/fast/CycleTest.toml +++ b/tests/fast/CycleTest.toml @@ -6,35 +6,3 @@ testTitle = 'Clogged' transactionsPerSecond = 2500.0 testDuration = 10.0 expectedRate = 0 - - [[test.workload]] - testName = 'RandomClogging' - testDuration = 10.0 - - [[test.workload]] - testName = 'Rollback' - meanDelay = 10.0 - testDuration = 10.0 - - [[test.workload]] - testName = 'Attrition' - machinesToKill = 10 - machinesToLeave = 3 - reboot = true - testDuration = 10.0 - - [[test.workload]] - testName = 'Attrition' - machinesToKill = 10 - machinesToLeave = 3 - reboot = true - testDuration = 10.0 - -[[test]] -testTitle = 'Unclogged' - - [[test.workload]] - testName = 'Cycle' - transactionsPerSecond = 250.0 - testDuration = 10.0 - expectedRate = 0.80 From 45f912368c04feeab8c31ca60c42afbc0e9e86a1 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Fri, 18 Feb 2022 16:48:44 +0100 Subject: [PATCH 03/24] Allow List and first test --- CMakeLists.txt | 2 +- fdbclient/BackupAgent.actor.h | 2 +- fdbclient/BackupAgentBase.actor.cpp | 6 +- fdbclient/CommitProxyInterface.h | 4 +- fdbclient/NativeAPI.actor.cpp | 12 +- fdbclient/StorageServerInterface.h | 22 ++-- fdbrpc/CMakeLists.txt | 1 + fdbrpc/FlowTransport.actor.cpp | 3 +- fdbrpc/FlowTransport.h | 4 +- fdbrpc/IPAllowList.cpp | 165 ++++++++++++++++++++++++++ fdbrpc/IPAllowList.h | 101 ++++++++++++++++ fdbrpc/LoadBalance.actor.h | 40 +++---- fdbrpc/fdbrpc.h | 22 ++-- fdbrpc/genericactors.actor.h | 8 +- fdbserver/ApplyMetadataMutation.cpp | 2 +- fdbserver/ClusterController.actor.cpp | 6 +- fdbserver/GrvProxyServer.actor.cpp | 4 +- fdbserver/ProxyCommitData.actor.h | 11 +- fdbserver/fdbserver.actor.cpp | 111 +++-------------- fdbserver/worker.actor.cpp | 4 +- flow/CMakeLists.txt | 1 + 21 files changed, 361 insertions(+), 170 deletions(-) create mode 100644 fdbrpc/IPAllowList.cpp create mode 100644 fdbrpc/IPAllowList.h diff --git a/CMakeLists.txt b/CMakeLists.txt index e07a9c9ac3..5d43cdc201 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -167,6 +167,7 @@ endif() include(CompileBoost) include(GetMsgpack) +add_subdirectory(contrib) add_subdirectory(flow) add_subdirectory(fdbrpc) add_subdirectory(fdbclient) @@ -178,7 +179,6 @@ else() add_subdirectory(fdbservice) endif() add_subdirectory(fdbbackup) -add_subdirectory(contrib) add_subdirectory(tests) add_subdirectory(flowbench EXCLUDE_FROM_ALL) if(WITH_PYTHON AND WITH_C_BINDING) diff --git a/fdbclient/BackupAgent.actor.h b/fdbclient/BackupAgent.actor.h index 5669f5d9d7..1f18b10a4c 100644 --- a/fdbclient/BackupAgent.actor.h +++ b/fdbclient/BackupAgent.actor.h @@ -561,7 +561,7 @@ ACTOR Future applyMutations(Database cx, Key removePrefix, Version beginVersion, Version* endVersion, - RequestStream commit, + RequestStream commit, NotifiedVersion* committedVersion, Reference> keyVersion); ACTOR Future cleanupBackup(Database cx, DeleteData deleteData); diff --git a/fdbclient/BackupAgentBase.actor.cpp b/fdbclient/BackupAgentBase.actor.cpp index 74f40743d2..75445d52f3 100644 --- a/fdbclient/BackupAgentBase.actor.cpp +++ b/fdbclient/BackupAgentBase.actor.cpp @@ -598,7 +598,7 @@ ACTOR Future dumpData(Database cx, Key uid, Key addPrefix, Key removePrefix, - RequestStream commit, + RequestStream commit, NotifiedVersion* committedVersion, Optional endVersion, Key rangeBegin, @@ -675,7 +675,7 @@ ACTOR Future dumpData(Database cx, ACTOR Future coalesceKeyVersionCache(Key uid, Version endVersion, Reference> keyVersion, - RequestStream commit, + RequestStream commit, NotifiedVersion* committedVersion, PromiseStream> addActor, FlowLock* commitLock) { @@ -725,7 +725,7 @@ ACTOR Future applyMutations(Database cx, Key removePrefix, Version beginVersion, Version* endVersion, - RequestStream commit, + RequestStream commit, NotifiedVersion* committedVersion, Reference> keyVersion) { state FlowLock commitLock(CLIENT_KNOBS->BACKUP_LOCK_BYTES); diff --git a/fdbclient/CommitProxyInterface.h b/fdbclient/CommitProxyInterface.h index 102b5a2088..af8f733ec3 100644 --- a/fdbclient/CommitProxyInterface.h +++ b/fdbclient/CommitProxyInterface.h @@ -71,9 +71,9 @@ struct CommitProxyInterface { serializer(ar, processId, provisional, commit); if (Archive::isDeserializing) { getConsistentReadVersion = - RequestStream(commit.getEndpoint().getAdjustedEndpoint(1)); + RequestStream(commit.getEndpoint().getAdjustedEndpoint(1)); getKeyServersLocations = - RequestStream(commit.getEndpoint().getAdjustedEndpoint(2)); + RequestStream(commit.getEndpoint().getAdjustedEndpoint(2)); getStorageServerRejoinInfo = RequestStream(commit.getEndpoint().getAdjustedEndpoint(3)); waitFailure = RequestStream>(commit.getEndpoint().getAdjustedEndpoint(4)); diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp index 79165dbf4c..ca5436e72c 100644 --- a/fdbclient/NativeAPI.actor.cpp +++ b/fdbclient/NativeAPI.actor.cpp @@ -100,11 +100,11 @@ namespace { TransactionLineageCollector transactionLineageCollector; NameLineageCollector nameLineageCollector; -template +template Future loadBalance( DatabaseContext* ctx, const Reference alternatives, - RequestStream Interface::*channel, + RequestStream Interface::*channel, const Request& request = Request(), TaskPriority taskID = TaskPriority::DefaultPromiseEndpoint, AtMostOnce atMostOnce = @@ -2223,7 +2223,7 @@ void stopNetwork() { if (!g_network) throw network_not_setup(); - TraceEvent("ClientStopNetwork"); + TraceEvent("ClientStopNetwork").log(); g_network->stop(); closeTraceFile(); } @@ -3176,7 +3176,7 @@ void transformRangeLimits(GetRangeLimits limits, Reverse reverse, GetKeyValuesFa } template -RequestStream StorageServerInterface::*getRangeRequestStream() { +RequestStream StorageServerInterface::*getRangeRequestStream() { if constexpr (std::is_same::value) { return &StorageServerInterface::getKeyValues; } else if (std::is_same::value) { @@ -3908,9 +3908,9 @@ static Future tssStreamComparison(Request request, // Currently only used for GetKeyValuesStream but could easily be plugged for other stream types // User of the stream has to forward the SS's responses to the returned promise stream, if it is set -template +template Optional> -maybeDuplicateTSSStreamFragment(Request& req, QueueModel* model, RequestStream const* ssStream) { +maybeDuplicateTSSStreamFragment(Request& req, QueueModel* model, RequestStream const* ssStream) { if (model) { Optional tssData = model->getTssData(ssStream->getEndpoint().token.first()); diff --git a/fdbclient/StorageServerInterface.h b/fdbclient/StorageServerInterface.h index 3da2ad402b..9636367ec9 100644 --- a/fdbclient/StorageServerInterface.h +++ b/fdbclient/StorageServerInterface.h @@ -68,11 +68,11 @@ struct StorageServerInterface { RequestStream getKeyValues; RequestStream getKeyValuesAndFlatMap; - RequestStream getShardState; + RequestStream getShardState; RequestStream waitMetrics; RequestStream splitMetrics; RequestStream getStorageMetrics; - RequestStream, true> waitFailure; + RequestStream> waitFailure; RequestStream getQueuingMetrics; RequestStream> getKeyValueStoreType; @@ -106,8 +106,8 @@ struct StorageServerInterface { serializer(ar, uniqueID, locality, getValue); } if (Ar::isDeserializing) { - getKey = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(1)); - getKeyValues = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(2)); + getKey = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(1)); + getKeyValues = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(2)); getShardState = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(3)); waitMetrics = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(4)); @@ -119,22 +119,22 @@ struct StorageServerInterface { RequestStream(getValue.getEndpoint().getAdjustedEndpoint(8)); getKeyValueStoreType = RequestStream>(getValue.getEndpoint().getAdjustedEndpoint(9)); - watchValue = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(10)); + watchValue = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(10)); getReadHotRanges = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(11)); getRangeSplitPoints = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(12)); getKeyValuesStream = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(13)); + RequestStream(getValue.getEndpoint().getAdjustedEndpoint(13)); getKeyValuesAndFlatMap = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(14)); + RequestStream(getValue.getEndpoint().getAdjustedEndpoint(14)); changeFeedStream = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(15)); + RequestStream(getValue.getEndpoint().getAdjustedEndpoint(15)); overlappingChangeFeeds = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(16)); + RequestStream(getValue.getEndpoint().getAdjustedEndpoint(16)); changeFeedPop = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(17)); - changeFeedVersionUpdate = RequestStream( + RequestStream(getValue.getEndpoint().getAdjustedEndpoint(17)); + changeFeedVersionUpdate = RequestStream( getValue.getEndpoint().getAdjustedEndpoint(18)); } } else { diff --git a/fdbrpc/CMakeLists.txt b/fdbrpc/CMakeLists.txt index 046ba4ff46..00c149aec5 100644 --- a/fdbrpc/CMakeLists.txt +++ b/fdbrpc/CMakeLists.txt @@ -15,6 +15,7 @@ set(FDBRPC_SRCS genericactors.actor.cpp HealthMonitor.actor.cpp IAsyncFile.actor.cpp + IPAllowList.cpp LoadBalance.actor.cpp LoadBalance.actor.h Locality.cpp diff --git a/fdbrpc/FlowTransport.actor.cpp b/fdbrpc/FlowTransport.actor.cpp index 851e3c084f..a9dc7d8c1f 100644 --- a/fdbrpc/FlowTransport.actor.cpp +++ b/fdbrpc/FlowTransport.actor.cpp @@ -32,6 +32,7 @@ #include "fdbrpc/FailureMonitor.h" #include "fdbrpc/HealthMonitor.h" #include "fdbrpc/genericactors.actor.h" +#include "fdbrpc/IPAllowList.h" #include "fdbrpc/simulator.h" #include "flow/ActorCollection.h" #include "flow/Error.h" @@ -1807,7 +1808,7 @@ bool FlowTransport::incompatibleOutgoingConnectionsPresent() { return self->numIncompatibleConnections > 0; } -void FlowTransport::createInstance(bool isClient, uint64_t transportId, int maxWellKnownEndpoints) { +void FlowTransport::createInstance(bool isClient, uint64_t transportId, int maxWellKnownEndpoints, IPAllowList const* allowList) { g_network->setGlobal(INetwork::enFlowTransport, (flowGlobalType) new FlowTransport(transportId, maxWellKnownEndpoints)); g_network->setGlobal(INetwork::enNetworkAddressFunc, (flowGlobalType)&FlowTransport::getGlobalLocalAddress); diff --git a/fdbrpc/FlowTransport.h b/fdbrpc/FlowTransport.h index ab132d78f4..9ff39a5596 100644 --- a/fdbrpc/FlowTransport.h +++ b/fdbrpc/FlowTransport.h @@ -182,6 +182,8 @@ struct Peer : public ReferenceCounted { void onIncomingConnection(Reference self, Reference conn, Future reader); }; +class IPAllowList; + class FlowTransport { public: FlowTransport(uint64_t transportId, int maxWellKnownEndpoints); @@ -189,7 +191,7 @@ public: // Creates a new FlowTransport and makes FlowTransport::transport() return it. This uses g_network->global() // variables, so it will be private to a simulation. - static void createInstance(bool isClient, uint64_t transportId, int maxWellKnownEndpoints); + static void createInstance(bool isClient, uint64_t transportId, int maxWellKnownEndpoints, IPAllowList const* allowList = nullptr); static bool isClient() { return g_network->global(INetwork::enClientFailureMonitor) != nullptr; } diff --git a/fdbrpc/IPAllowList.cpp b/fdbrpc/IPAllowList.cpp new file mode 100644 index 0000000000..416db4a5e7 --- /dev/null +++ b/fdbrpc/IPAllowList.cpp @@ -0,0 +1,165 @@ +/* + * IPAllowList.h + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2022 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "fdbrpc/IPAllowList.h" +#include "flow/UnitTest.h" + +#include +#include + +namespace { + +template +std::string binRep(std::array const& addr) { + return fmt::format("{:02x}", fmt::join(addr, ":")); +} + +template +void printIP(std::array const& addr) { + fmt::print(" {}", binRep(addr)); +} + +} // namespace + +AuthAllowedSubnet AuthAllowedSubnet::fromString(std::string_view addressString) { + auto pos = addressString.find('/'); + if (pos == std::string_view::npos) { + fmt::print("ERROR: {} is not a valid (use Network-Prefix/hostcount syntax)\n"); + throw invalid_option(); + } + auto address = addressString.substr(0, pos); + auto hostCount = std::stoi(std::string(addressString.substr(pos + 1))); + auto addr = boost::asio::ip::make_address(address); + if (addr.is_v4()) { + auto bM = createBitMask(addr.to_v4().to_bytes(), hostCount); + // we typically would expect a base address has been passed, but to be safe we still + // will make the last bits 0 + auto mask = boost::asio::ip::address_v4(bM).to_uint(); + auto baseAddress = addr.to_v4().to_uint() & mask; + fmt::print("For address {}:", addressString); + printIP("Base Address", IPAddress(baseAddress)); + printIP("Mask:", IPAddress(mask)); + return AuthAllowedSubnet(IPAddress(baseAddress), IPAddress(mask)); + } else { + auto mask = createBitMask(addr.to_v6().to_bytes(), hostCount); + auto baseAddress = addr.to_v6().to_bytes(); + for (int i = 0; i < mask.size(); ++i) { + baseAddress[i] &= mask[i]; + } + return AuthAllowedSubnet(IPAddress(baseAddress), IPAddress(mask)); + } +} + +void AuthAllowedSubnet::printIP(std::string_view txt, IPAddress const& address) { + fmt::print("{}:", txt); + if (address.isV4()) { + ::printIP(boost::asio::ip::address_v4(address.toV4()).to_bytes()); + } else { + ::printIP(address.toV6()); + } + fmt::print("\n"); +} + +// helpers for testing +namespace { +using boost::asio::ip::address_v4; +using boost::asio::ip::address_v6; + +void traceAddress(TraceEvent& evt, const char* name, IPAddress address) { + evt.detail(name, address); + std::string bin; + if (address.isV4()) { + boost::asio::ip::address_v4 a(address.toV4()); + bin = binRep(a.to_bytes()); + } else { + bin = binRep(address.toV6()); + } + evt.detail(fmt::format("{}Binary", name).c_str(), bin); +} + +void subnetAssert(IPAllowList const& allowList, IPAddress addr, bool expectAllowed) { + if (allowList(addr) == expectAllowed) { + return; + } + TraceEvent evt(SevError, expectAllowed ? "ExpectedAddressToBeTrusted" : "ExpectedAddressToBeUntrusted"); + traceAddress(evt, "Address", addr); + auto const& subnets = allowList.subnets(); + for (int i = 0; i < subnets.size(); ++i) { + traceAddress(evt, fmt::format("SubnetBase{}", i).c_str(), subnets[i].baseAddress); + traceAddress(evt, fmt::format("SubnetMask{}", i).c_str(), subnets[i].addressMask); + } +} + +IPAddress parseAddr(std::string const& str) { + auto res = IPAddress::parse(str); + ASSERT(res.present()); + return res.get(); +} + +struct SubNetTest { + AuthAllowedSubnet subnet; + SubNetTest(AuthAllowedSubnet&& subnet) + : subnet(subnet) + { + } + template + static SubNetTest randomSubNetImpl() { + constexpr int width = V4 ? 4 : 16; + std::array binAddr; + unsigned char rnd[4]; + for (int i = 0; i < binAddr.size(); ++i) { + if (i % 4 == 0) { + auto tmp = deterministicRandom()->randomUInt32(); + ::memcpy(rnd, &tmp, 4); + } + binAddr[i] = rnd[i % 4]; + } + auto hostCount = deterministicRandom()->randomInt(1, width); + std::string address; + if constexpr (V4) { + address_v4 a(binAddr); + address = a.to_string(); + } else { + address_v6 a(binAddr); + address = a.to_string(); + } + return SubNetTest(AuthAllowedSubnet::fromString(fmt::format("{}/{}", address, hostCount))); + } + static SubNetTest randomSubNet() { + if (deterministicRandom()->coinflip()) { + return randomSubNetImpl(); + } else { + return randomSubNetImpl(); + } + } +}; + +} // namespace + +TEST_CASE("/fdbrpc/allow_list") { + IPAllowList allowList; + allowList.addTrustedSubnet("1.0.0.0/8"); + allowList.addTrustedSubnet("2.0.0.0/4"); + ::subnetAssert(allowList, parseAddr("1.0.1.1"), true); + ::subnetAssert(allowList, parseAddr("1.1.2.2"), true); + ::subnetAssert(allowList, parseAddr("2.2.1.1"), true); + ::subnetAssert(allowList, parseAddr("128.0.1.1"), false); + return Void(); +} diff --git a/fdbrpc/IPAllowList.h b/fdbrpc/IPAllowList.h new file mode 100644 index 0000000000..5cf2bcc0e0 --- /dev/null +++ b/fdbrpc/IPAllowList.h @@ -0,0 +1,101 @@ +/* + * IPAllowList.h + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2022 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once +#ifndef FDBRPC_IP_ALLOW_LIST_H +#define FDBRPC_IP_ALLOW_LIST_H + +#include "flow/network.h" +#include "flow/Arena.h" + +struct AuthAllowedSubnet { + IPAddress baseAddress; + IPAddress addressMask; + + AuthAllowedSubnet(IPAddress const& baseAddress, IPAddress const& addressMask) + : baseAddress(baseAddress), addressMask(addressMask) { + ASSERT(baseAddress.isV4() == addressMask.isV4()); + } + + static AuthAllowedSubnet fromString(std::string_view addressString); + + template + static auto createBitMask(std::array const& addr, int hostCount) -> std::array { + std::array res; + res.fill((unsigned char)0xff); + int idx = hostCount / 8; + if (hostCount % 8 > 0) { + // 2^(hostCount % 8) - 1 sets the last (hostCount % 8) number of bits to 1 + // everything else will be zero. For example: 2^3 - 1 == 7 == 0b111 + unsigned char bitmask = (1 << (hostCount % 8)) - ((unsigned char)1); + res[idx] ^= bitmask; + ++idx; + } + for (; idx < res.size(); ++idx) { + res[idx] = (unsigned char)0; + } + return res; + } + + bool operator() (IPAddress const& address) const { + if (addressMask.isV4() != address.isV4()) { + return false; + } + if (addressMask.isV4()) { + return (addressMask.toV4() & address.toV4()) == baseAddress.toV4(); + } else { + auto res = address.toV6(); + auto const& mask = addressMask.toV6(); + for (int i = 0; i < res.size(); ++i) { + res[i] &= mask[i]; + } + return res == baseAddress.toV6(); + } + } + + // some useful helper functions if we need to debug ip masks etc + static void printIP(std::string_view txt, IPAddress const& address); +}; + +class IPAllowList { + std::vector subnetList; +public: + void addTrustedSubnet(std::string_view str) { + subnetList.push_back(AuthAllowedSubnet::fromString(str)); + } + + std::vector const& subnets() const { + return subnetList; + } + + bool operator() (IPAddress address) const { + if (subnetList.empty()) { + return true; + } + for (auto const& subnet : subnetList) { + if (subnet(address)) { + return true; + } + } + return false; + } +}; + +#endif // FDBRPC_IP_ALLOW_LIST_H diff --git a/fdbrpc/LoadBalance.actor.h b/fdbrpc/LoadBalance.actor.h index 838a1762b8..d150fded11 100644 --- a/fdbrpc/LoadBalance.actor.h +++ b/fdbrpc/LoadBalance.actor.h @@ -78,14 +78,14 @@ struct LoadBalancedReply { Optional getLoadBalancedReply(const LoadBalancedReply* reply); Optional getLoadBalancedReply(const void*); -ACTOR template +ACTOR template Future tssComparison(Req req, Future> fSource, Future> fTss, TSSEndpointData tssData, uint64_t srcEndpointId, Reference> ssTeam, - RequestStream Interface::*channel) { + RequestStream Interface::*channel) { state double startTime = now(); state Future>> fTssWithTimeout = timeout(fTss, FLOW_KNOBS->LOAD_BALANCE_TSS_TIMEOUT); state int finished = 0; @@ -157,7 +157,7 @@ Future tssComparison(Req req, state std::vector>> restOfTeamFutures; restOfTeamFutures.reserve(ssTeam->size() - 1); for (int i = 0; i < ssTeam->size(); i++) { - RequestStream const* si = &ssTeam->get(i, channel); + RequestStream const* si = &ssTeam->get(i, channel); if (si->getEndpoint().token.first() != srcEndpointId) { // don't re-request to SS we already have a response from resetReply(req); @@ -242,7 +242,7 @@ FDB_DECLARE_BOOLEAN_PARAM(AtMostOnce); FDB_DECLARE_BOOLEAN_PARAM(TriedAllOptions); // Stores state for a request made by the load balancer -template +template struct RequestData : NonCopyable { typedef ErrorOr Reply; @@ -257,12 +257,12 @@ struct RequestData : NonCopyable { // This is true once setupRequest is called, even though at that point the response is Never(). bool isValid() { return response.isValid(); } - static void maybeDuplicateTSSRequest(RequestStream const* stream, + static void maybeDuplicateTSSRequest(RequestStream const* stream, Request& request, QueueModel* model, Future ssResponse, Reference> alternatives, - RequestStream Interface::*channel) { + RequestStream Interface::*channel) { if (model) { // Send parallel request to TSS pair, if it exists Optional tssData = model->getTssData(stream->getEndpoint().token.first()); @@ -271,7 +271,7 @@ struct RequestData : NonCopyable { TEST(true); // duplicating request to TSS resetReply(request); // FIXME: optimize to avoid creating new netNotifiedQueue for each message - RequestStream tssRequestStream(tssData.get().endpoint); + RequestStream tssRequestStream(tssData.get().endpoint); Future> fTssResult = tssRequestStream.tryGetReply(request); model->addActor.send(tssComparison(request, ssResponse, @@ -288,11 +288,11 @@ struct RequestData : NonCopyable { void startRequest( double backoff, TriedAllOptions triedAllOptions, - RequestStream const* stream, + RequestStream const* stream, Request& request, QueueModel* model, Reference> alternatives, // alternatives and channel passed through for TSS check - RequestStream Interface::*channel) { + RequestStream Interface::*channel) { modelHolder = Reference(); requestStarted = false; @@ -438,18 +438,18 @@ struct RequestData : NonCopyable { // list of servers. // When model is set, load balance among alternatives in the same DC aims to balance request queue length on these // interfaces. If too many interfaces in the same DC are bad, try remote interfaces. -ACTOR template +ACTOR template Future loadBalance( Reference> alternatives, - RequestStream Interface::*channel, + RequestStream Interface::*channel, Request request = Request(), TaskPriority taskID = TaskPriority::DefaultPromiseEndpoint, AtMostOnce atMostOnce = AtMostOnce::False, // if true, throws request_maybe_delivered() instead of retrying automatically QueueModel* model = nullptr) { - state RequestData firstRequestData; - state RequestData secondRequestData; + state RequestData firstRequestData; + state RequestData secondRequestData; state Optional firstRequestEndpoint; state Future secondDelay = Never(); @@ -488,7 +488,7 @@ Future loadBalance( break; } - RequestStream const* thisStream = &alternatives->get(i, channel); + RequestStream const* thisStream = &alternatives->get(i, channel); if (!IFailureMonitor::failureMonitor().getState(thisStream->getEndpoint()).failed) { auto& qd = model->getMeasurement(thisStream->getEndpoint().token.first()); if (now() > qd.failedUntil) { @@ -527,7 +527,7 @@ Future loadBalance( // go through all the remote servers again, since we may have // skipped it. for (int i = alternatives->countBest(); i < alternatives->size(); i++) { - RequestStream const* thisStream = &alternatives->get(i, channel); + RequestStream const* thisStream = &alternatives->get(i, channel); if (!IFailureMonitor::failureMonitor().getState(thisStream->getEndpoint()).failed) { auto& qd = model->getMeasurement(thisStream->getEndpoint().token.first()); if (now() > qd.failedUntil) { @@ -574,7 +574,7 @@ Future loadBalance( if (ev.isEnabled()) { ev.log(); for (int alternativeNum = 0; alternativeNum < alternatives->size(); alternativeNum++) { - RequestStream const* thisStream = &alternatives->get(alternativeNum, channel); + RequestStream const* thisStream = &alternatives->get(alternativeNum, channel); TraceEvent(SevWarn, "LoadBalanceTooLongEndpoint") .detail("Addr", thisStream->getEndpoint().getPrimaryAddress()) .detail("Token", thisStream->getEndpoint().token) @@ -586,7 +586,7 @@ Future loadBalance( // Find an alternative, if any, that is not failed, starting with // nextAlt. This logic matters only if model == nullptr. Otherwise, the // bestAlt and nextAlt have been decided. - state RequestStream const* stream = nullptr; + state RequestStream const* stream = nullptr; for (int alternativeNum = 0; alternativeNum < alternatives->size(); alternativeNum++) { int useAlt = nextAlt; if (nextAlt == startAlt) @@ -724,9 +724,9 @@ Optional getBasicLoadBalancedReply(const BasicLoadBalanc Optional getBasicLoadBalancedReply(const void*); // A simpler version of LoadBalance that does not send second requests where the list of servers are always fresh -ACTOR template +ACTOR template Future basicLoadBalance(Reference> alternatives, - RequestStream Interface::*channel, + RequestStream Interface::*channel, Request request = Request(), TaskPriority taskID = TaskPriority::DefaultPromiseEndpoint, AtMostOnce atMostOnce = AtMostOnce::False) { @@ -749,7 +749,7 @@ Future basicLoadBalance(Reference> al state int useAlt; loop { // Find an alternative, if any, that is not failed, starting with nextAlt - state RequestStream const* stream = nullptr; + state RequestStream const* stream = nullptr; for (int alternativeNum = 0; alternativeNum < alternatives->size(); alternativeNum++) { useAlt = nextAlt; if (nextAlt == startAlt) diff --git a/fdbrpc/fdbrpc.h b/fdbrpc/fdbrpc.h index c7c5b21480..d78816910a 100644 --- a/fdbrpc/fdbrpc.h +++ b/fdbrpc/fdbrpc.h @@ -825,8 +825,8 @@ public: queue->makeWellKnownEndpoint(Endpoint::Token(-1, wlTokenID), taskID); } - bool operator==(const RequestStream& rhs) const { return queue == rhs.queue; } - bool operator!=(const RequestStream& rhs) const { return !(*this == rhs); } + bool operator==(const RequestStream& rhs) const { return queue == rhs.queue; } + bool operator!=(const RequestStream& rhs) const { return !(*this == rhs); } bool isEmpty() const { return !queue->isReady(); } uint32_t size() const { return queue->size(); } @@ -838,29 +838,29 @@ private: NetNotifiedQueue* queue; }; -template -void save(Ar& ar, const RequestStream& value) { +template +void save(Ar& ar, const RequestStream& value) { auto const& ep = value.getEndpoint(); ar << ep; UNSTOPPABLE_ASSERT( ep.getPrimaryAddress().isValid()); // No serializing PromiseStreams on a client with no public address } -template -void load(Ar& ar, RequestStream& value) { +template +void load(Ar& ar, RequestStream& value) { Endpoint endpoint; ar >> endpoint; - value = RequestStream(endpoint); + value = RequestStream(endpoint); } -template -struct serializable_traits> : std::true_type { +template +struct serializable_traits> : std::true_type { template - static void serialize(Archiver& ar, RequestStream& stream) { + static void serialize(Archiver& ar, RequestStream& stream) { if constexpr (Archiver::isDeserializing) { Endpoint endpoint; serializer(ar, endpoint); - stream = RequestStream(endpoint); + stream = RequestStream(endpoint); } else { const auto& ep = stream.getEndpoint(); serializer(ar, ep); diff --git a/fdbrpc/genericactors.actor.h b/fdbrpc/genericactors.actor.h index 46a79d29cf..da476bf339 100644 --- a/fdbrpc/genericactors.actor.h +++ b/fdbrpc/genericactors.actor.h @@ -30,8 +30,8 @@ #include "fdbrpc/fdbrpc.h" #include "flow/actorcompiler.h" // This must be the last #include. -ACTOR template -Future retryBrokenPromise(RequestStream to, Req request) { +ACTOR template +Future retryBrokenPromise(RequestStream to, Req request) { // Like to.getReply(request), except that a broken_promise exception results in retrying request immediately. // Suitable for use with well known endpoints, which are likely to return to existence after the other process // restarts. Not normally useful for ordinary endpoints, which conventionally are permanently destroyed after @@ -50,8 +50,8 @@ Future retryBrokenPromise(RequestStream to, Req request) { } } -ACTOR template -Future retryBrokenPromise(RequestStream to, Req request, TaskPriority taskID) { +ACTOR template +Future retryBrokenPromise(RequestStream to, Req request, TaskPriority taskID) { // Like to.getReply(request), except that a broken_promise exception results in retrying request immediately. // Suitable for use with well known endpoints, which are likely to return to existence after the other process // restarts. Not normally useful for ordinary endpoints, which conventionally are permanently destroyed after diff --git a/fdbserver/ApplyMetadataMutation.cpp b/fdbserver/ApplyMetadataMutation.cpp index 47871915f9..96ca476648 100644 --- a/fdbserver/ApplyMetadataMutation.cpp +++ b/fdbserver/ApplyMetadataMutation.cpp @@ -107,7 +107,7 @@ private: KeyRangeMap* keyInfo = nullptr; KeyRangeMap* cacheInfo = nullptr; std::map* uid_applyMutationsData = nullptr; - RequestStream commit = RequestStream(); + RequestStream commit = RequestStream(); Database cx = Database(); NotifiedVersion* commitVersion = nullptr; std::map>* storageCache = nullptr; diff --git a/fdbserver/ClusterController.actor.cpp b/fdbserver/ClusterController.actor.cpp index 40456dba38..eacef60897 100644 --- a/fdbserver/ClusterController.actor.cpp +++ b/fdbserver/ClusterController.actor.cpp @@ -2919,7 +2919,7 @@ TEST_CASE("/fdbserver/clustercontroller/shouldTriggerRecoveryDueToDegradedServer testDbInfo.logSystemConfig.tLogs.push_back(remoteTLogSet); GrvProxyInterface proxyInterf; - proxyInterf.getConsistentReadVersion = RequestStream(Endpoint({ proxy }, testUID)); + proxyInterf.getConsistentReadVersion = RequestStream(Endpoint({ proxy }, testUID)); testDbInfo.client.grvProxies.push_back(proxyInterf); ResolverInterface resolverInterf; @@ -3028,11 +3028,11 @@ TEST_CASE("/fdbserver/clustercontroller/shouldTriggerFailoverDueToDegradedServer testDbInfo.logSystemConfig.tLogs.push_back(remoteTLogSet); GrvProxyInterface grvProxyInterf; - grvProxyInterf.getConsistentReadVersion = RequestStream(Endpoint({ proxy }, testUID)); + grvProxyInterf.getConsistentReadVersion = RequestStream(Endpoint({ proxy }, testUID)); testDbInfo.client.grvProxies.push_back(grvProxyInterf); CommitProxyInterface commitProxyInterf; - commitProxyInterf.commit = RequestStream(Endpoint({ proxy2 }, testUID)); + commitProxyInterf.commit = RequestStream(Endpoint({ proxy2 }, testUID)); testDbInfo.client.commitProxies.push_back(commitProxyInterf); ResolverInterface resolverInterf; diff --git a/fdbserver/GrvProxyServer.actor.cpp b/fdbserver/GrvProxyServer.actor.cpp index 5f427dd0b0..3ae922a980 100644 --- a/fdbserver/GrvProxyServer.actor.cpp +++ b/fdbserver/GrvProxyServer.actor.cpp @@ -229,7 +229,7 @@ struct GrvProxyData { GrvProxyStats stats; MasterInterface master; - RequestStream getConsistentReadVersion; + RequestStream getConsistentReadVersion; Reference logSystem; Database cx; @@ -261,7 +261,7 @@ struct GrvProxyData { GrvProxyData(UID dbgid, MasterInterface master, - RequestStream getConsistentReadVersion, + RequestStream getConsistentReadVersion, Reference const> db) : dbgid(dbgid), stats(dbgid), master(master), getConsistentReadVersion(getConsistentReadVersion), cx(openDBOnServer(db, TaskPriority::DefaultEndpoint, LockAware::True)), db(db), lastStartCommit(0), diff --git a/fdbserver/ProxyCommitData.actor.h b/fdbserver/ProxyCommitData.actor.h index d1f9ebccf4..715aa6ddcc 100644 --- a/fdbserver/ProxyCommitData.actor.h +++ b/fdbserver/ProxyCommitData.actor.h @@ -28,6 +28,9 @@ #include "fdbclient/FDBTypes.h" #include "fdbrpc/Stats.h" #include "fdbserver/Knobs.h" +#include "fdbserver/LogSystem.h" +#include "fdbserver/MasterInterface.h" +#include "fdbserver/ResolverInterface.h" #include "fdbserver/LogSystemDiskQueueAdapter.h" #include "flow/IRandom.h" @@ -189,8 +192,8 @@ struct ProxyCommitData { NotifiedVersion latestLocalCommitBatchResolving; NotifiedVersion latestLocalCommitBatchLogging; - RequestStream getConsistentReadVersion; - RequestStream commit; + RequestStream getConsistentReadVersion; + RequestStream commit; Database cx; Reference const> db; EventMetricHandle singleKeyMutationEvent; @@ -267,9 +270,9 @@ struct ProxyCommitData { ProxyCommitData(UID dbgid, MasterInterface master, - RequestStream getConsistentReadVersion, + RequestStream getConsistentReadVersion, Version recoveryTransactionVersion, - RequestStream commit, + RequestStream commit, Reference const> db, bool firstProxy) : dbgid(dbgid), commitBatchesMemBytesCount(0), diff --git a/fdbserver/fdbserver.actor.cpp b/fdbserver/fdbserver.actor.cpp index 4f5d7a26a9..2c2915f3f4 100644 --- a/fdbserver/fdbserver.actor.cpp +++ b/fdbserver/fdbserver.actor.cpp @@ -35,6 +35,8 @@ #include #include +#include + #include "fdbclient/ActorLineageProfiler.h" #include "fdbclient/ClusterConnectionFile.h" #include "fdbclient/IKnobCollection.h" @@ -45,6 +47,7 @@ #include "fdbclient/WellKnownEndpoints.h" #include "fdbclient/SimpleIni.h" #include "fdbrpc/AsyncFileCached.actor.h" +#include "fdbrpc/IPAllowList.h" #include "fdbrpc/Net2FileSystem.h" #include "fdbrpc/PerfMetric.h" #include "fdbrpc/simulator.h" @@ -1017,6 +1020,7 @@ struct CLIOptions { std::map profilerConfig; bool printSimTime = false; + IPAllowList allowList; static CLIOptions parseArgs(int argc, char* argv[]) { CLIOptions opts; @@ -1120,6 +1124,15 @@ private: localities.set(key, Standalone(std::string(args.OptionArg()))); break; } + case OPT_IP_TRUSTED_MASK: { + Optional subnetKey = extractPrefixedArgument("--trusted-subnet", args.OptionSyntax()); + if (!subnetKey.present()) { + fprintf(stderr, "ERROR: unable to parse locality key '%s'\n", args.OptionSyntax()); + flushAndExit(FDB_EXIT_ERROR); + } + allowList.addTrustedSubnet(args.OptionArg()); + break; + } case OPT_VERSION: printVersion(); flushAndExit(FDB_EXIT_SUCCESS); @@ -1668,103 +1681,7 @@ private: }; } // namespace -#include - -struct AuthAllowedSubnet { - IPAddress baseAddress; - IPAddress addressMask; - - AuthAllowedSubnet(IPAddress const& baseAddress, IPAddress const& addressMask) - : baseAddress(baseAddress), addressMask(addressMask) { - ASSERT(baseAddress.isV4() == addressMask.isV4()); - } - - static AuthAllowedSubnet fromString(std::string_view addressString) { - auto pos = addressString.find('/'); - if (pos == std::string_view::npos) { - fmt::print("ERROR: {} is not a valid (use Network-Prefix/hostcount syntax)\n"); - throw invalid_option(); - } - auto address = addressString.substr(0, pos); - auto hostCount = std::stoi(std::string(addressString.substr(pos + 1))); - auto addr = boost::asio::ip::make_address(address); - if (addr.is_v4()) { - auto bM = createBitMask(addr.to_v4().to_bytes(), hostCount); - // we typically would expect a base address has been passed, but to be safe we still - // will make the last bits 0 - auto mask = boost::asio::ip::address_v4(bM).to_uint(); - auto baseAddress = addr.to_v4().to_uint() & mask; - return AuthAllowedSubnet(IPAddress(baseAddress), IPAddress(mask)); - } else { - auto mask = createBitMask(addr.to_v6().to_bytes(), hostCount); - auto baseAddress = addr.to_v6().to_bytes(); - for (int i = 0; i < mask.size(); ++i) { - baseAddress[i] &= mask[i]; - } - return AuthAllowedSubnet(IPAddress(baseAddress), IPAddress(mask)); - } - } - - template - static auto createBitMask(std::array const& addr, int hostCount) -> std::array { - std::array res; - res.fill((unsigned char)0xff); - for (auto idx = (hostCount / 8) - 1; idx < res.size(); ++idx) { - if (hostCount > 0) { - // 2^(hostCount % 8) - 1 sets the last (hostCount % 8) number of bits to 1 - // everything else will be zero. For example: 2^3 - 1 == 7 == 0b111 - unsigned char bitmask = (1 << (hostCount % 8)) - ((unsigned char)1); - res[idx] ^= bitmask; - } else { - res[idx] = (unsigned char)0; - } - hostCount = 0; - } - return res; - } - - bool operator() (IPAddress const& address) const { - if (addressMask.isV4() != address.isV4()) { - return false; - } - if (addressMask.isV4()) { - return (addressMask.toV4() & address.toV4()) == baseAddress.toV4(); - } else { - auto res = address.toV6(); - auto const& mask = addressMask.toV6(); - for (int i = 0; i < res.size(); ++i) { - res[i] &= mask[i]; - } - return res == baseAddress.toV6(); - } - } -}; - -template -void printIP(std::array const& addr) { - for (auto c : addr) { - fmt::print(" {:02x}", int(c)); - } -} - -void printIP(std::string_view txt, IPAddress const& address) { - fmt::print("{}:", txt); - if (address.isV4()) { - printIP(boost::asio::ip::address_v4(address.toV4()).to_bytes()); - } else { - printIP(address.toV6()); - } - fmt::print("\n"); -} - int main(int argc, char* argv[]) { - //auto allowed = AuthAllowedSubnet::fromString(argv[1]); - //printIP("Base Address", allowed.baseAddress); - //printIP("Address Mask", allowed.addressMask); - //for (int idx = 1; idx < argc; ++idx) { - // auto addr = IPAddress::parse(argv[idx]); - //} - //return 0; // TODO: Remove later, this is just to force the statics to be initialized // otherwise the unit test won't run #ifdef ENABLE_SAMPLING @@ -1892,7 +1809,7 @@ int main(int argc, char* argv[]) { } else { g_network = newNet2(opts.tlsConfig, opts.useThreadPool, true); g_network->addStopCallback(Net2FileSystem::stop); - FlowTransport::createInstance(false, 1, WLTOKEN_RESERVED_COUNT); + FlowTransport::createInstance(false, 1, WLTOKEN_RESERVED_COUNT, &opts.allowList); const bool expectsPublicAddress = (role == ServerRole::FDBD || role == ServerRole::NetworkTestServer || role == ServerRole::Restore); diff --git a/fdbserver/worker.actor.cpp b/fdbserver/worker.actor.cpp index 9c19e13512..e2715f15b1 100644 --- a/fdbserver/worker.actor.cpp +++ b/fdbserver/worker.actor.cpp @@ -749,14 +749,14 @@ TEST_CASE("/fdbserver/worker/addressInDbAndPrimaryDc") { NetworkAddress grvProxyAddress(IPAddress(0x26262626), 1); GrvProxyInterface grvProxyInterf; grvProxyInterf.getConsistentReadVersion = - RequestStream(Endpoint({ grvProxyAddress }, UID(1, 2))); + RequestStream(Endpoint({ grvProxyAddress }, UID(1, 2))); testDbInfo.client.grvProxies.push_back(grvProxyInterf); ASSERT(addressInDbAndPrimaryDc(grvProxyAddress, makeReference>(testDbInfo))); NetworkAddress commitProxyAddress(IPAddress(0x37373737), 1); CommitProxyInterface commitProxyInterf; commitProxyInterf.commit = - RequestStream(Endpoint({ commitProxyAddress }, UID(1, 2))); + RequestStream(Endpoint({ commitProxyAddress }, UID(1, 2))); testDbInfo.client.commitProxies.push_back(commitProxyInterf); ASSERT(addressInDbAndPrimaryDc(commitProxyAddress, makeReference>(testDbInfo))); diff --git a/flow/CMakeLists.txt b/flow/CMakeLists.txt index a64b028974..990632f290 100644 --- a/flow/CMakeLists.txt +++ b/flow/CMakeLists.txt @@ -134,6 +134,7 @@ target_link_libraries(flow PUBLIC fmt::fmt) add_flow_target(STATIC_LIBRARY NAME flow_sampling SRCS ${FLOW_SRCS}) target_link_libraries(flow_sampling PRIVATE stacktrace) +target_link_libraries(flow_sampling PUBLIC fmt::fmt) target_compile_definitions(flow_sampling PRIVATE -DENABLE_SAMPLING) if(WIN32) add_dependencies(flow_sampling_actors flow_actors) From dc973fb67ed4a2a4c1c2cc1102f13c8d4cfd9b32 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Fri, 18 Feb 2022 16:48:44 +0100 Subject: [PATCH 04/24] Allow List and first test --- CMakeLists.txt | 2 +- fdbclient/BackupAgent.actor.h | 2 +- fdbclient/BackupAgentBase.actor.cpp | 6 +- fdbclient/CommitProxyInterface.h | 4 +- fdbclient/NativeAPI.actor.cpp | 12 +- fdbclient/StorageServerInterface.h | 22 +- fdbrpc/CMakeLists.txt | 1 + fdbrpc/FlowTransport.actor.cpp | 3 +- fdbrpc/FlowTransport.h | 4 +- fdbrpc/IPAllowList.cpp | 290 ++++++++++++++++++++++++++ fdbrpc/IPAllowList.h | 109 ++++++++++ fdbrpc/LoadBalance.actor.h | 40 ++-- fdbrpc/fdbrpc.h | 22 +- fdbrpc/genericactors.actor.h | 8 +- fdbserver/ApplyMetadataMutation.cpp | 2 +- fdbserver/ClusterController.actor.cpp | 6 +- fdbserver/GrvProxyServer.actor.cpp | 4 +- fdbserver/ProxyCommitData.actor.h | 11 +- fdbserver/fdbserver.actor.cpp | 111 ++-------- fdbserver/worker.actor.cpp | 4 +- flow/CMakeLists.txt | 1 + 21 files changed, 494 insertions(+), 170 deletions(-) create mode 100644 fdbrpc/IPAllowList.cpp create mode 100644 fdbrpc/IPAllowList.h diff --git a/CMakeLists.txt b/CMakeLists.txt index e07a9c9ac3..5d43cdc201 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -167,6 +167,7 @@ endif() include(CompileBoost) include(GetMsgpack) +add_subdirectory(contrib) add_subdirectory(flow) add_subdirectory(fdbrpc) add_subdirectory(fdbclient) @@ -178,7 +179,6 @@ else() add_subdirectory(fdbservice) endif() add_subdirectory(fdbbackup) -add_subdirectory(contrib) add_subdirectory(tests) add_subdirectory(flowbench EXCLUDE_FROM_ALL) if(WITH_PYTHON AND WITH_C_BINDING) diff --git a/fdbclient/BackupAgent.actor.h b/fdbclient/BackupAgent.actor.h index 5669f5d9d7..1f18b10a4c 100644 --- a/fdbclient/BackupAgent.actor.h +++ b/fdbclient/BackupAgent.actor.h @@ -561,7 +561,7 @@ ACTOR Future applyMutations(Database cx, Key removePrefix, Version beginVersion, Version* endVersion, - RequestStream commit, + RequestStream commit, NotifiedVersion* committedVersion, Reference> keyVersion); ACTOR Future cleanupBackup(Database cx, DeleteData deleteData); diff --git a/fdbclient/BackupAgentBase.actor.cpp b/fdbclient/BackupAgentBase.actor.cpp index 74f40743d2..75445d52f3 100644 --- a/fdbclient/BackupAgentBase.actor.cpp +++ b/fdbclient/BackupAgentBase.actor.cpp @@ -598,7 +598,7 @@ ACTOR Future dumpData(Database cx, Key uid, Key addPrefix, Key removePrefix, - RequestStream commit, + RequestStream commit, NotifiedVersion* committedVersion, Optional endVersion, Key rangeBegin, @@ -675,7 +675,7 @@ ACTOR Future dumpData(Database cx, ACTOR Future coalesceKeyVersionCache(Key uid, Version endVersion, Reference> keyVersion, - RequestStream commit, + RequestStream commit, NotifiedVersion* committedVersion, PromiseStream> addActor, FlowLock* commitLock) { @@ -725,7 +725,7 @@ ACTOR Future applyMutations(Database cx, Key removePrefix, Version beginVersion, Version* endVersion, - RequestStream commit, + RequestStream commit, NotifiedVersion* committedVersion, Reference> keyVersion) { state FlowLock commitLock(CLIENT_KNOBS->BACKUP_LOCK_BYTES); diff --git a/fdbclient/CommitProxyInterface.h b/fdbclient/CommitProxyInterface.h index 102b5a2088..af8f733ec3 100644 --- a/fdbclient/CommitProxyInterface.h +++ b/fdbclient/CommitProxyInterface.h @@ -71,9 +71,9 @@ struct CommitProxyInterface { serializer(ar, processId, provisional, commit); if (Archive::isDeserializing) { getConsistentReadVersion = - RequestStream(commit.getEndpoint().getAdjustedEndpoint(1)); + RequestStream(commit.getEndpoint().getAdjustedEndpoint(1)); getKeyServersLocations = - RequestStream(commit.getEndpoint().getAdjustedEndpoint(2)); + RequestStream(commit.getEndpoint().getAdjustedEndpoint(2)); getStorageServerRejoinInfo = RequestStream(commit.getEndpoint().getAdjustedEndpoint(3)); waitFailure = RequestStream>(commit.getEndpoint().getAdjustedEndpoint(4)); diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp index 79165dbf4c..ca5436e72c 100644 --- a/fdbclient/NativeAPI.actor.cpp +++ b/fdbclient/NativeAPI.actor.cpp @@ -100,11 +100,11 @@ namespace { TransactionLineageCollector transactionLineageCollector; NameLineageCollector nameLineageCollector; -template +template Future loadBalance( DatabaseContext* ctx, const Reference alternatives, - RequestStream Interface::*channel, + RequestStream Interface::*channel, const Request& request = Request(), TaskPriority taskID = TaskPriority::DefaultPromiseEndpoint, AtMostOnce atMostOnce = @@ -2223,7 +2223,7 @@ void stopNetwork() { if (!g_network) throw network_not_setup(); - TraceEvent("ClientStopNetwork"); + TraceEvent("ClientStopNetwork").log(); g_network->stop(); closeTraceFile(); } @@ -3176,7 +3176,7 @@ void transformRangeLimits(GetRangeLimits limits, Reverse reverse, GetKeyValuesFa } template -RequestStream StorageServerInterface::*getRangeRequestStream() { +RequestStream StorageServerInterface::*getRangeRequestStream() { if constexpr (std::is_same::value) { return &StorageServerInterface::getKeyValues; } else if (std::is_same::value) { @@ -3908,9 +3908,9 @@ static Future tssStreamComparison(Request request, // Currently only used for GetKeyValuesStream but could easily be plugged for other stream types // User of the stream has to forward the SS's responses to the returned promise stream, if it is set -template +template Optional> -maybeDuplicateTSSStreamFragment(Request& req, QueueModel* model, RequestStream const* ssStream) { +maybeDuplicateTSSStreamFragment(Request& req, QueueModel* model, RequestStream const* ssStream) { if (model) { Optional tssData = model->getTssData(ssStream->getEndpoint().token.first()); diff --git a/fdbclient/StorageServerInterface.h b/fdbclient/StorageServerInterface.h index 3da2ad402b..9636367ec9 100644 --- a/fdbclient/StorageServerInterface.h +++ b/fdbclient/StorageServerInterface.h @@ -68,11 +68,11 @@ struct StorageServerInterface { RequestStream getKeyValues; RequestStream getKeyValuesAndFlatMap; - RequestStream getShardState; + RequestStream getShardState; RequestStream waitMetrics; RequestStream splitMetrics; RequestStream getStorageMetrics; - RequestStream, true> waitFailure; + RequestStream> waitFailure; RequestStream getQueuingMetrics; RequestStream> getKeyValueStoreType; @@ -106,8 +106,8 @@ struct StorageServerInterface { serializer(ar, uniqueID, locality, getValue); } if (Ar::isDeserializing) { - getKey = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(1)); - getKeyValues = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(2)); + getKey = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(1)); + getKeyValues = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(2)); getShardState = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(3)); waitMetrics = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(4)); @@ -119,22 +119,22 @@ struct StorageServerInterface { RequestStream(getValue.getEndpoint().getAdjustedEndpoint(8)); getKeyValueStoreType = RequestStream>(getValue.getEndpoint().getAdjustedEndpoint(9)); - watchValue = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(10)); + watchValue = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(10)); getReadHotRanges = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(11)); getRangeSplitPoints = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(12)); getKeyValuesStream = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(13)); + RequestStream(getValue.getEndpoint().getAdjustedEndpoint(13)); getKeyValuesAndFlatMap = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(14)); + RequestStream(getValue.getEndpoint().getAdjustedEndpoint(14)); changeFeedStream = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(15)); + RequestStream(getValue.getEndpoint().getAdjustedEndpoint(15)); overlappingChangeFeeds = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(16)); + RequestStream(getValue.getEndpoint().getAdjustedEndpoint(16)); changeFeedPop = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(17)); - changeFeedVersionUpdate = RequestStream( + RequestStream(getValue.getEndpoint().getAdjustedEndpoint(17)); + changeFeedVersionUpdate = RequestStream( getValue.getEndpoint().getAdjustedEndpoint(18)); } } else { diff --git a/fdbrpc/CMakeLists.txt b/fdbrpc/CMakeLists.txt index 046ba4ff46..00c149aec5 100644 --- a/fdbrpc/CMakeLists.txt +++ b/fdbrpc/CMakeLists.txt @@ -15,6 +15,7 @@ set(FDBRPC_SRCS genericactors.actor.cpp HealthMonitor.actor.cpp IAsyncFile.actor.cpp + IPAllowList.cpp LoadBalance.actor.cpp LoadBalance.actor.h Locality.cpp diff --git a/fdbrpc/FlowTransport.actor.cpp b/fdbrpc/FlowTransport.actor.cpp index 851e3c084f..a9dc7d8c1f 100644 --- a/fdbrpc/FlowTransport.actor.cpp +++ b/fdbrpc/FlowTransport.actor.cpp @@ -32,6 +32,7 @@ #include "fdbrpc/FailureMonitor.h" #include "fdbrpc/HealthMonitor.h" #include "fdbrpc/genericactors.actor.h" +#include "fdbrpc/IPAllowList.h" #include "fdbrpc/simulator.h" #include "flow/ActorCollection.h" #include "flow/Error.h" @@ -1807,7 +1808,7 @@ bool FlowTransport::incompatibleOutgoingConnectionsPresent() { return self->numIncompatibleConnections > 0; } -void FlowTransport::createInstance(bool isClient, uint64_t transportId, int maxWellKnownEndpoints) { +void FlowTransport::createInstance(bool isClient, uint64_t transportId, int maxWellKnownEndpoints, IPAllowList const* allowList) { g_network->setGlobal(INetwork::enFlowTransport, (flowGlobalType) new FlowTransport(transportId, maxWellKnownEndpoints)); g_network->setGlobal(INetwork::enNetworkAddressFunc, (flowGlobalType)&FlowTransport::getGlobalLocalAddress); diff --git a/fdbrpc/FlowTransport.h b/fdbrpc/FlowTransport.h index ab132d78f4..9ff39a5596 100644 --- a/fdbrpc/FlowTransport.h +++ b/fdbrpc/FlowTransport.h @@ -182,6 +182,8 @@ struct Peer : public ReferenceCounted { void onIncomingConnection(Reference self, Reference conn, Future reader); }; +class IPAllowList; + class FlowTransport { public: FlowTransport(uint64_t transportId, int maxWellKnownEndpoints); @@ -189,7 +191,7 @@ public: // Creates a new FlowTransport and makes FlowTransport::transport() return it. This uses g_network->global() // variables, so it will be private to a simulation. - static void createInstance(bool isClient, uint64_t transportId, int maxWellKnownEndpoints); + static void createInstance(bool isClient, uint64_t transportId, int maxWellKnownEndpoints, IPAllowList const* allowList = nullptr); static bool isClient() { return g_network->global(INetwork::enClientFailureMonitor) != nullptr; } diff --git a/fdbrpc/IPAllowList.cpp b/fdbrpc/IPAllowList.cpp new file mode 100644 index 0000000000..dbe7cfa012 --- /dev/null +++ b/fdbrpc/IPAllowList.cpp @@ -0,0 +1,290 @@ +/* + * IPAllowList.h + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2022 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "fdbrpc/IPAllowList.h" +#include "flow/UnitTest.h" + +#include +#include + +namespace { + +template +std::string binRep(std::array const& addr) { + return fmt::format("{:02x}", fmt::join(addr, ":")); +} + +template +void printIP(std::array const& addr) { + fmt::print(" {}", binRep(addr)); +} + +template +int hostCountImpl(std::array const& addr) { + int count = 0; + for (int i = 0; i < addr.size() && addr[i] != 0xff; ++i) { + std::bitset<8> b(addr[i]); + count += 8 - b.count(); + } + return count; +} + +} // namespace + +IPAddress AuthAllowedSubnet::netmask() const { + if (addressMask.isV4()) { + uint32_t res = 0xffffffff ^ addressMask.toV4(); + return IPAddress(res); + } else { + std::array res; + res.fill(0xff); + auto mask = addressMask.toV6(); + for (int i = 0; i < mask.size(); ++i) { + res[i] ^= mask[i]; + } + return IPAddress(res); + } +} + + +int AuthAllowedSubnet::hostCount() const { + if (addressMask.isV4()) { + boost::asio::ip::address_v4 addr(addressMask.toV4()); + return hostCountImpl(addr.to_bytes()); + } else { + return hostCountImpl(addressMask.toV6()); + } +} + +AuthAllowedSubnet AuthAllowedSubnet::fromString(std::string_view addressString) { + auto pos = addressString.find('/'); + if (pos == std::string_view::npos) { + fmt::print("ERROR: {} is not a valid (use Network-Prefix/hostcount syntax)\n"); + throw invalid_option(); + } + auto address = addressString.substr(0, pos); + auto hostCount = std::stoi(std::string(addressString.substr(pos + 1))); + auto addr = boost::asio::ip::make_address(address); + if (addr.is_v4()) { + auto bM = createBitMask(addr.to_v4().to_bytes(), hostCount); + // we typically would expect a base address has been passed, but to be safe we still + // will make the last bits 0 + auto mask = boost::asio::ip::address_v4(bM).to_uint(); + auto baseAddress = addr.to_v4().to_uint() & mask; + fmt::print("For address {}:", addressString); + printIP("Base Address", IPAddress(baseAddress)); + printIP("Mask:", IPAddress(mask)); + return AuthAllowedSubnet(IPAddress(baseAddress), IPAddress(mask)); + } else { + auto mask = createBitMask(addr.to_v6().to_bytes(), hostCount); + auto baseAddress = addr.to_v6().to_bytes(); + for (int i = 0; i < mask.size(); ++i) { + baseAddress[i] &= mask[i]; + } + return AuthAllowedSubnet(IPAddress(baseAddress), IPAddress(mask)); + } +} + +void AuthAllowedSubnet::printIP(std::string_view txt, IPAddress const& address) { + fmt::print("{}:", txt); + if (address.isV4()) { + ::printIP(boost::asio::ip::address_v4(address.toV4()).to_bytes()); + } else { + ::printIP(address.toV6()); + } + fmt::print("\n"); +} + +// helpers for testing +namespace { +using boost::asio::ip::address_v4; +using boost::asio::ip::address_v6; + +void traceAddress(TraceEvent& evt, const char* name, IPAddress address) { + evt.detail(name, address); + std::string bin; + if (address.isV4()) { + boost::asio::ip::address_v4 a(address.toV4()); + bin = binRep(a.to_bytes()); + } else { + bin = binRep(address.toV6()); + } + evt.detail(fmt::format("{}Binary", name).c_str(), bin); +} + +void subnetAssert(IPAllowList const& allowList, IPAddress addr, bool expectAllowed) { + if (allowList(addr) == expectAllowed) { + return; + } + TraceEvent evt(SevError, expectAllowed ? "ExpectedAddressToBeTrusted" : "ExpectedAddressToBeUntrusted"); + traceAddress(evt, "Address", addr); + auto const& subnets = allowList.subnets(); + for (int i = 0; i < subnets.size(); ++i) { + traceAddress(evt, fmt::format("SubnetBase{}", i).c_str(), subnets[i].baseAddress); + traceAddress(evt, fmt::format("SubnetMask{}", i).c_str(), subnets[i].addressMask); + } +} + +IPAddress parseAddr(std::string const& str) { + auto res = IPAddress::parse(str); + ASSERT(res.present()); + return res.get(); +} + +struct SubNetTest { + AuthAllowedSubnet subnet; + SubNetTest(AuthAllowedSubnet&& subnet) + : subnet(subnet) + { + } + template + static SubNetTest randomSubNetImpl() { + constexpr int width = V4 ? 4 : 16; + std::array binAddr; + unsigned char rnd[4]; + for (int i = 0; i < binAddr.size(); ++i) { + if (i % 4 == 0) { + auto tmp = deterministicRandom()->randomUInt32(); + ::memcpy(rnd, &tmp, 4); + } + binAddr[i] = rnd[i % 4]; + } + auto hostCount = deterministicRandom()->randomInt(1, width); + std::string address; + if constexpr (V4) { + address_v4 a(binAddr); + address = a.to_string(); + } else { + address_v6 a(binAddr); + address = a.to_string(); + } + return SubNetTest(AuthAllowedSubnet::fromString(fmt::format("{}/{}", address, hostCount))); + } + static SubNetTest randomSubNet() { + if (deterministicRandom()->coinflip()) { + return randomSubNetImpl(); + } else { + return randomSubNetImpl(); + } + } + + template + IPAddress intArrayToAddress(uint32_t* arr) { + if constexpr (V4) { + return IPAddress(arr[0]); + } else { + std::array res; + memcpy(res.data(), arr, 4); + return IPAddress(res); + } + } + + template + I transformIntToSubnet(I val, I subnetMask, I baseAddress) { + return (val & subnetMask) ^ baseAddress; + } + + template + IPAddress randomAddress(bool inSubnet) { + ASSERT(V4 == subnet.baseAddress.isV4() || !inSubnet); + constexpr int width = V4 ? 4 : 16; + for (;;) { + uint32_t rnd[width / 4]; + for (int i = 0; i < width / 4; ++i) { + rnd[i] = deterministicRandom()->randomUInt32(); + } + auto res = intArrayToAddress(rnd); + if (V4 != subnet.baseAddress.isV4()) { + return res; + } + if (!inSubnet) { + if (!subnet(res)) { + return res; + } else { + continue; + } + } + // first we make sure the address is in the subnet + if constexpr (V4) { + auto a = res.toV4(); + auto base = subnet.baseAddress.toV4(); + auto netmask = subnet.netmask().toV4(); + auto validAddress = transformIntToSubnet(a, netmask, base); + res = IPAddress(validAddress); + } else { + auto a = res.toV6(); + auto base = subnet.baseAddress.toV6(); + auto netmask = subnet.netmask().toV6(); + for (int i = 0; i < a.size(); ++i) { + a[i] = transformIntToSubnet(a[i], netmask[i], base[i]); + } + res = IPAddress(a); + } + return res; + } + } + + IPAddress randomAddress(bool inSubnet) { + if (!inSubnet && deterministicRandom()->random01() < 0.1) { + // return an address of a different type + if (subnet.baseAddress.isV4()) { + return randomAddress(false); + } else { + return randomAddress(false); + } + } + if (subnet.addressMask.isV4()) { + return randomAddress(inSubnet); + } else { + return randomAddress(inSubnet); + } + } +}; + +} // namespace + +TEST_CASE("/fdbrpc/allow_list") { + IPAllowList allowList; + allowList.addTrustedSubnet("1.0.0.0/8"); + allowList.addTrustedSubnet("2.0.0.0/4"); + ::subnetAssert(allowList, parseAddr("1.0.1.1"), true); + ::subnetAssert(allowList, parseAddr("1.1.2.2"), true); + ::subnetAssert(allowList, parseAddr("2.2.1.1"), true); + ::subnetAssert(allowList, parseAddr("128.0.1.1"), false); + allowList = IPAllowList(); + allowList.addTrustedSubnet("0.0.0.0/2"); + ::subnetAssert(allowList, parseAddr("1.0.1.1"), true); + ::subnetAssert(allowList, parseAddr("1.1.2.2"), true); + ::subnetAssert(allowList, parseAddr("2.2.1.1"), true); + ::subnetAssert(allowList, parseAddr("5.2.1.1"), true); + ::subnetAssert(allowList, parseAddr("128.0.1.1"), false); + ::subnetAssert(allowList, parseAddr("192.168.3.1"), false); + for (int i = 0; i < 100; ++i) { + SubNetTest subnetTest(SubNetTest::randomSubNet()); + allowList = IPAllowList(); + allowList.addTrustedSubnet(subnetTest.subnet); + for (int j = 0; j < 10; ++j) { + bool inSubnet = deterministicRandom()->random01() < 0.7; + auto addr = subnetTest.randomAddress(inSubnet); + ::subnetAssert(allowList, addr, inSubnet); + } + } + return Void(); +} diff --git a/fdbrpc/IPAllowList.h b/fdbrpc/IPAllowList.h new file mode 100644 index 0000000000..9860029e4a --- /dev/null +++ b/fdbrpc/IPAllowList.h @@ -0,0 +1,109 @@ +/* + * IPAllowList.h + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2022 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once +#ifndef FDBRPC_IP_ALLOW_LIST_H +#define FDBRPC_IP_ALLOW_LIST_H + +#include "flow/network.h" +#include "flow/Arena.h" + +struct AuthAllowedSubnet { + IPAddress baseAddress; + IPAddress addressMask; + + AuthAllowedSubnet(IPAddress const& baseAddress, IPAddress const& addressMask) + : baseAddress(baseAddress), addressMask(addressMask) { + ASSERT(baseAddress.isV4() == addressMask.isV4()); + } + + static AuthAllowedSubnet fromString(std::string_view addressString); + + template + static auto createBitMask(std::array const& addr, int hostCount) -> std::array { + std::array res; + res.fill((unsigned char)0xff); + int idx = hostCount / 8; + if (hostCount % 8 > 0) { + // 2^(hostCount % 8) - 1 sets the last (hostCount % 8) number of bits to 1 + // everything else will be zero. For example: 2^3 - 1 == 7 == 0b111 + unsigned char bitmask = (1 << (hostCount % 8)) - ((unsigned char)1); + res[idx] ^= bitmask; + ++idx; + } + for (; idx < res.size(); ++idx) { + res[idx] = (unsigned char)0; + } + return res; + } + + bool operator() (IPAddress const& address) const { + if (addressMask.isV4() != address.isV4()) { + return false; + } + if (addressMask.isV4()) { + return (addressMask.toV4() & address.toV4()) == baseAddress.toV4(); + } else { + auto res = address.toV6(); + auto const& mask = addressMask.toV6(); + for (int i = 0; i < res.size(); ++i) { + res[i] &= mask[i]; + } + return res == baseAddress.toV6(); + } + } + + IPAddress netmask() const; + + int hostCount() const; + + // some useful helper functions if we need to debug ip masks etc + static void printIP(std::string_view txt, IPAddress const& address); +}; + +class IPAllowList { + std::vector subnetList; +public: + void addTrustedSubnet(std::string_view str) { + subnetList.push_back(AuthAllowedSubnet::fromString(str)); + } + + void addTrustedSubnet(AuthAllowedSubnet const& subnet) { + subnetList.push_back(subnet); + } + + std::vector const& subnets() const { + return subnetList; + } + + bool operator() (IPAddress address) const { + if (subnetList.empty()) { + return true; + } + for (auto const& subnet : subnetList) { + if (subnet(address)) { + return true; + } + } + return false; + } +}; + +#endif // FDBRPC_IP_ALLOW_LIST_H diff --git a/fdbrpc/LoadBalance.actor.h b/fdbrpc/LoadBalance.actor.h index 838a1762b8..d150fded11 100644 --- a/fdbrpc/LoadBalance.actor.h +++ b/fdbrpc/LoadBalance.actor.h @@ -78,14 +78,14 @@ struct LoadBalancedReply { Optional getLoadBalancedReply(const LoadBalancedReply* reply); Optional getLoadBalancedReply(const void*); -ACTOR template +ACTOR template Future tssComparison(Req req, Future> fSource, Future> fTss, TSSEndpointData tssData, uint64_t srcEndpointId, Reference> ssTeam, - RequestStream Interface::*channel) { + RequestStream Interface::*channel) { state double startTime = now(); state Future>> fTssWithTimeout = timeout(fTss, FLOW_KNOBS->LOAD_BALANCE_TSS_TIMEOUT); state int finished = 0; @@ -157,7 +157,7 @@ Future tssComparison(Req req, state std::vector>> restOfTeamFutures; restOfTeamFutures.reserve(ssTeam->size() - 1); for (int i = 0; i < ssTeam->size(); i++) { - RequestStream const* si = &ssTeam->get(i, channel); + RequestStream const* si = &ssTeam->get(i, channel); if (si->getEndpoint().token.first() != srcEndpointId) { // don't re-request to SS we already have a response from resetReply(req); @@ -242,7 +242,7 @@ FDB_DECLARE_BOOLEAN_PARAM(AtMostOnce); FDB_DECLARE_BOOLEAN_PARAM(TriedAllOptions); // Stores state for a request made by the load balancer -template +template struct RequestData : NonCopyable { typedef ErrorOr Reply; @@ -257,12 +257,12 @@ struct RequestData : NonCopyable { // This is true once setupRequest is called, even though at that point the response is Never(). bool isValid() { return response.isValid(); } - static void maybeDuplicateTSSRequest(RequestStream const* stream, + static void maybeDuplicateTSSRequest(RequestStream const* stream, Request& request, QueueModel* model, Future ssResponse, Reference> alternatives, - RequestStream Interface::*channel) { + RequestStream Interface::*channel) { if (model) { // Send parallel request to TSS pair, if it exists Optional tssData = model->getTssData(stream->getEndpoint().token.first()); @@ -271,7 +271,7 @@ struct RequestData : NonCopyable { TEST(true); // duplicating request to TSS resetReply(request); // FIXME: optimize to avoid creating new netNotifiedQueue for each message - RequestStream tssRequestStream(tssData.get().endpoint); + RequestStream tssRequestStream(tssData.get().endpoint); Future> fTssResult = tssRequestStream.tryGetReply(request); model->addActor.send(tssComparison(request, ssResponse, @@ -288,11 +288,11 @@ struct RequestData : NonCopyable { void startRequest( double backoff, TriedAllOptions triedAllOptions, - RequestStream const* stream, + RequestStream const* stream, Request& request, QueueModel* model, Reference> alternatives, // alternatives and channel passed through for TSS check - RequestStream Interface::*channel) { + RequestStream Interface::*channel) { modelHolder = Reference(); requestStarted = false; @@ -438,18 +438,18 @@ struct RequestData : NonCopyable { // list of servers. // When model is set, load balance among alternatives in the same DC aims to balance request queue length on these // interfaces. If too many interfaces in the same DC are bad, try remote interfaces. -ACTOR template +ACTOR template Future loadBalance( Reference> alternatives, - RequestStream Interface::*channel, + RequestStream Interface::*channel, Request request = Request(), TaskPriority taskID = TaskPriority::DefaultPromiseEndpoint, AtMostOnce atMostOnce = AtMostOnce::False, // if true, throws request_maybe_delivered() instead of retrying automatically QueueModel* model = nullptr) { - state RequestData firstRequestData; - state RequestData secondRequestData; + state RequestData firstRequestData; + state RequestData secondRequestData; state Optional firstRequestEndpoint; state Future secondDelay = Never(); @@ -488,7 +488,7 @@ Future loadBalance( break; } - RequestStream const* thisStream = &alternatives->get(i, channel); + RequestStream const* thisStream = &alternatives->get(i, channel); if (!IFailureMonitor::failureMonitor().getState(thisStream->getEndpoint()).failed) { auto& qd = model->getMeasurement(thisStream->getEndpoint().token.first()); if (now() > qd.failedUntil) { @@ -527,7 +527,7 @@ Future loadBalance( // go through all the remote servers again, since we may have // skipped it. for (int i = alternatives->countBest(); i < alternatives->size(); i++) { - RequestStream const* thisStream = &alternatives->get(i, channel); + RequestStream const* thisStream = &alternatives->get(i, channel); if (!IFailureMonitor::failureMonitor().getState(thisStream->getEndpoint()).failed) { auto& qd = model->getMeasurement(thisStream->getEndpoint().token.first()); if (now() > qd.failedUntil) { @@ -574,7 +574,7 @@ Future loadBalance( if (ev.isEnabled()) { ev.log(); for (int alternativeNum = 0; alternativeNum < alternatives->size(); alternativeNum++) { - RequestStream const* thisStream = &alternatives->get(alternativeNum, channel); + RequestStream const* thisStream = &alternatives->get(alternativeNum, channel); TraceEvent(SevWarn, "LoadBalanceTooLongEndpoint") .detail("Addr", thisStream->getEndpoint().getPrimaryAddress()) .detail("Token", thisStream->getEndpoint().token) @@ -586,7 +586,7 @@ Future loadBalance( // Find an alternative, if any, that is not failed, starting with // nextAlt. This logic matters only if model == nullptr. Otherwise, the // bestAlt and nextAlt have been decided. - state RequestStream const* stream = nullptr; + state RequestStream const* stream = nullptr; for (int alternativeNum = 0; alternativeNum < alternatives->size(); alternativeNum++) { int useAlt = nextAlt; if (nextAlt == startAlt) @@ -724,9 +724,9 @@ Optional getBasicLoadBalancedReply(const BasicLoadBalanc Optional getBasicLoadBalancedReply(const void*); // A simpler version of LoadBalance that does not send second requests where the list of servers are always fresh -ACTOR template +ACTOR template Future basicLoadBalance(Reference> alternatives, - RequestStream Interface::*channel, + RequestStream Interface::*channel, Request request = Request(), TaskPriority taskID = TaskPriority::DefaultPromiseEndpoint, AtMostOnce atMostOnce = AtMostOnce::False) { @@ -749,7 +749,7 @@ Future basicLoadBalance(Reference> al state int useAlt; loop { // Find an alternative, if any, that is not failed, starting with nextAlt - state RequestStream const* stream = nullptr; + state RequestStream const* stream = nullptr; for (int alternativeNum = 0; alternativeNum < alternatives->size(); alternativeNum++) { useAlt = nextAlt; if (nextAlt == startAlt) diff --git a/fdbrpc/fdbrpc.h b/fdbrpc/fdbrpc.h index c7c5b21480..d78816910a 100644 --- a/fdbrpc/fdbrpc.h +++ b/fdbrpc/fdbrpc.h @@ -825,8 +825,8 @@ public: queue->makeWellKnownEndpoint(Endpoint::Token(-1, wlTokenID), taskID); } - bool operator==(const RequestStream& rhs) const { return queue == rhs.queue; } - bool operator!=(const RequestStream& rhs) const { return !(*this == rhs); } + bool operator==(const RequestStream& rhs) const { return queue == rhs.queue; } + bool operator!=(const RequestStream& rhs) const { return !(*this == rhs); } bool isEmpty() const { return !queue->isReady(); } uint32_t size() const { return queue->size(); } @@ -838,29 +838,29 @@ private: NetNotifiedQueue* queue; }; -template -void save(Ar& ar, const RequestStream& value) { +template +void save(Ar& ar, const RequestStream& value) { auto const& ep = value.getEndpoint(); ar << ep; UNSTOPPABLE_ASSERT( ep.getPrimaryAddress().isValid()); // No serializing PromiseStreams on a client with no public address } -template -void load(Ar& ar, RequestStream& value) { +template +void load(Ar& ar, RequestStream& value) { Endpoint endpoint; ar >> endpoint; - value = RequestStream(endpoint); + value = RequestStream(endpoint); } -template -struct serializable_traits> : std::true_type { +template +struct serializable_traits> : std::true_type { template - static void serialize(Archiver& ar, RequestStream& stream) { + static void serialize(Archiver& ar, RequestStream& stream) { if constexpr (Archiver::isDeserializing) { Endpoint endpoint; serializer(ar, endpoint); - stream = RequestStream(endpoint); + stream = RequestStream(endpoint); } else { const auto& ep = stream.getEndpoint(); serializer(ar, ep); diff --git a/fdbrpc/genericactors.actor.h b/fdbrpc/genericactors.actor.h index 46a79d29cf..da476bf339 100644 --- a/fdbrpc/genericactors.actor.h +++ b/fdbrpc/genericactors.actor.h @@ -30,8 +30,8 @@ #include "fdbrpc/fdbrpc.h" #include "flow/actorcompiler.h" // This must be the last #include. -ACTOR template -Future retryBrokenPromise(RequestStream to, Req request) { +ACTOR template +Future retryBrokenPromise(RequestStream to, Req request) { // Like to.getReply(request), except that a broken_promise exception results in retrying request immediately. // Suitable for use with well known endpoints, which are likely to return to existence after the other process // restarts. Not normally useful for ordinary endpoints, which conventionally are permanently destroyed after @@ -50,8 +50,8 @@ Future retryBrokenPromise(RequestStream to, Req request) { } } -ACTOR template -Future retryBrokenPromise(RequestStream to, Req request, TaskPriority taskID) { +ACTOR template +Future retryBrokenPromise(RequestStream to, Req request, TaskPriority taskID) { // Like to.getReply(request), except that a broken_promise exception results in retrying request immediately. // Suitable for use with well known endpoints, which are likely to return to existence after the other process // restarts. Not normally useful for ordinary endpoints, which conventionally are permanently destroyed after diff --git a/fdbserver/ApplyMetadataMutation.cpp b/fdbserver/ApplyMetadataMutation.cpp index 47871915f9..96ca476648 100644 --- a/fdbserver/ApplyMetadataMutation.cpp +++ b/fdbserver/ApplyMetadataMutation.cpp @@ -107,7 +107,7 @@ private: KeyRangeMap* keyInfo = nullptr; KeyRangeMap* cacheInfo = nullptr; std::map* uid_applyMutationsData = nullptr; - RequestStream commit = RequestStream(); + RequestStream commit = RequestStream(); Database cx = Database(); NotifiedVersion* commitVersion = nullptr; std::map>* storageCache = nullptr; diff --git a/fdbserver/ClusterController.actor.cpp b/fdbserver/ClusterController.actor.cpp index 40456dba38..eacef60897 100644 --- a/fdbserver/ClusterController.actor.cpp +++ b/fdbserver/ClusterController.actor.cpp @@ -2919,7 +2919,7 @@ TEST_CASE("/fdbserver/clustercontroller/shouldTriggerRecoveryDueToDegradedServer testDbInfo.logSystemConfig.tLogs.push_back(remoteTLogSet); GrvProxyInterface proxyInterf; - proxyInterf.getConsistentReadVersion = RequestStream(Endpoint({ proxy }, testUID)); + proxyInterf.getConsistentReadVersion = RequestStream(Endpoint({ proxy }, testUID)); testDbInfo.client.grvProxies.push_back(proxyInterf); ResolverInterface resolverInterf; @@ -3028,11 +3028,11 @@ TEST_CASE("/fdbserver/clustercontroller/shouldTriggerFailoverDueToDegradedServer testDbInfo.logSystemConfig.tLogs.push_back(remoteTLogSet); GrvProxyInterface grvProxyInterf; - grvProxyInterf.getConsistentReadVersion = RequestStream(Endpoint({ proxy }, testUID)); + grvProxyInterf.getConsistentReadVersion = RequestStream(Endpoint({ proxy }, testUID)); testDbInfo.client.grvProxies.push_back(grvProxyInterf); CommitProxyInterface commitProxyInterf; - commitProxyInterf.commit = RequestStream(Endpoint({ proxy2 }, testUID)); + commitProxyInterf.commit = RequestStream(Endpoint({ proxy2 }, testUID)); testDbInfo.client.commitProxies.push_back(commitProxyInterf); ResolverInterface resolverInterf; diff --git a/fdbserver/GrvProxyServer.actor.cpp b/fdbserver/GrvProxyServer.actor.cpp index 5f427dd0b0..3ae922a980 100644 --- a/fdbserver/GrvProxyServer.actor.cpp +++ b/fdbserver/GrvProxyServer.actor.cpp @@ -229,7 +229,7 @@ struct GrvProxyData { GrvProxyStats stats; MasterInterface master; - RequestStream getConsistentReadVersion; + RequestStream getConsistentReadVersion; Reference logSystem; Database cx; @@ -261,7 +261,7 @@ struct GrvProxyData { GrvProxyData(UID dbgid, MasterInterface master, - RequestStream getConsistentReadVersion, + RequestStream getConsistentReadVersion, Reference const> db) : dbgid(dbgid), stats(dbgid), master(master), getConsistentReadVersion(getConsistentReadVersion), cx(openDBOnServer(db, TaskPriority::DefaultEndpoint, LockAware::True)), db(db), lastStartCommit(0), diff --git a/fdbserver/ProxyCommitData.actor.h b/fdbserver/ProxyCommitData.actor.h index d1f9ebccf4..715aa6ddcc 100644 --- a/fdbserver/ProxyCommitData.actor.h +++ b/fdbserver/ProxyCommitData.actor.h @@ -28,6 +28,9 @@ #include "fdbclient/FDBTypes.h" #include "fdbrpc/Stats.h" #include "fdbserver/Knobs.h" +#include "fdbserver/LogSystem.h" +#include "fdbserver/MasterInterface.h" +#include "fdbserver/ResolverInterface.h" #include "fdbserver/LogSystemDiskQueueAdapter.h" #include "flow/IRandom.h" @@ -189,8 +192,8 @@ struct ProxyCommitData { NotifiedVersion latestLocalCommitBatchResolving; NotifiedVersion latestLocalCommitBatchLogging; - RequestStream getConsistentReadVersion; - RequestStream commit; + RequestStream getConsistentReadVersion; + RequestStream commit; Database cx; Reference const> db; EventMetricHandle singleKeyMutationEvent; @@ -267,9 +270,9 @@ struct ProxyCommitData { ProxyCommitData(UID dbgid, MasterInterface master, - RequestStream getConsistentReadVersion, + RequestStream getConsistentReadVersion, Version recoveryTransactionVersion, - RequestStream commit, + RequestStream commit, Reference const> db, bool firstProxy) : dbgid(dbgid), commitBatchesMemBytesCount(0), diff --git a/fdbserver/fdbserver.actor.cpp b/fdbserver/fdbserver.actor.cpp index 4f5d7a26a9..2c2915f3f4 100644 --- a/fdbserver/fdbserver.actor.cpp +++ b/fdbserver/fdbserver.actor.cpp @@ -35,6 +35,8 @@ #include #include +#include + #include "fdbclient/ActorLineageProfiler.h" #include "fdbclient/ClusterConnectionFile.h" #include "fdbclient/IKnobCollection.h" @@ -45,6 +47,7 @@ #include "fdbclient/WellKnownEndpoints.h" #include "fdbclient/SimpleIni.h" #include "fdbrpc/AsyncFileCached.actor.h" +#include "fdbrpc/IPAllowList.h" #include "fdbrpc/Net2FileSystem.h" #include "fdbrpc/PerfMetric.h" #include "fdbrpc/simulator.h" @@ -1017,6 +1020,7 @@ struct CLIOptions { std::map profilerConfig; bool printSimTime = false; + IPAllowList allowList; static CLIOptions parseArgs(int argc, char* argv[]) { CLIOptions opts; @@ -1120,6 +1124,15 @@ private: localities.set(key, Standalone(std::string(args.OptionArg()))); break; } + case OPT_IP_TRUSTED_MASK: { + Optional subnetKey = extractPrefixedArgument("--trusted-subnet", args.OptionSyntax()); + if (!subnetKey.present()) { + fprintf(stderr, "ERROR: unable to parse locality key '%s'\n", args.OptionSyntax()); + flushAndExit(FDB_EXIT_ERROR); + } + allowList.addTrustedSubnet(args.OptionArg()); + break; + } case OPT_VERSION: printVersion(); flushAndExit(FDB_EXIT_SUCCESS); @@ -1668,103 +1681,7 @@ private: }; } // namespace -#include - -struct AuthAllowedSubnet { - IPAddress baseAddress; - IPAddress addressMask; - - AuthAllowedSubnet(IPAddress const& baseAddress, IPAddress const& addressMask) - : baseAddress(baseAddress), addressMask(addressMask) { - ASSERT(baseAddress.isV4() == addressMask.isV4()); - } - - static AuthAllowedSubnet fromString(std::string_view addressString) { - auto pos = addressString.find('/'); - if (pos == std::string_view::npos) { - fmt::print("ERROR: {} is not a valid (use Network-Prefix/hostcount syntax)\n"); - throw invalid_option(); - } - auto address = addressString.substr(0, pos); - auto hostCount = std::stoi(std::string(addressString.substr(pos + 1))); - auto addr = boost::asio::ip::make_address(address); - if (addr.is_v4()) { - auto bM = createBitMask(addr.to_v4().to_bytes(), hostCount); - // we typically would expect a base address has been passed, but to be safe we still - // will make the last bits 0 - auto mask = boost::asio::ip::address_v4(bM).to_uint(); - auto baseAddress = addr.to_v4().to_uint() & mask; - return AuthAllowedSubnet(IPAddress(baseAddress), IPAddress(mask)); - } else { - auto mask = createBitMask(addr.to_v6().to_bytes(), hostCount); - auto baseAddress = addr.to_v6().to_bytes(); - for (int i = 0; i < mask.size(); ++i) { - baseAddress[i] &= mask[i]; - } - return AuthAllowedSubnet(IPAddress(baseAddress), IPAddress(mask)); - } - } - - template - static auto createBitMask(std::array const& addr, int hostCount) -> std::array { - std::array res; - res.fill((unsigned char)0xff); - for (auto idx = (hostCount / 8) - 1; idx < res.size(); ++idx) { - if (hostCount > 0) { - // 2^(hostCount % 8) - 1 sets the last (hostCount % 8) number of bits to 1 - // everything else will be zero. For example: 2^3 - 1 == 7 == 0b111 - unsigned char bitmask = (1 << (hostCount % 8)) - ((unsigned char)1); - res[idx] ^= bitmask; - } else { - res[idx] = (unsigned char)0; - } - hostCount = 0; - } - return res; - } - - bool operator() (IPAddress const& address) const { - if (addressMask.isV4() != address.isV4()) { - return false; - } - if (addressMask.isV4()) { - return (addressMask.toV4() & address.toV4()) == baseAddress.toV4(); - } else { - auto res = address.toV6(); - auto const& mask = addressMask.toV6(); - for (int i = 0; i < res.size(); ++i) { - res[i] &= mask[i]; - } - return res == baseAddress.toV6(); - } - } -}; - -template -void printIP(std::array const& addr) { - for (auto c : addr) { - fmt::print(" {:02x}", int(c)); - } -} - -void printIP(std::string_view txt, IPAddress const& address) { - fmt::print("{}:", txt); - if (address.isV4()) { - printIP(boost::asio::ip::address_v4(address.toV4()).to_bytes()); - } else { - printIP(address.toV6()); - } - fmt::print("\n"); -} - int main(int argc, char* argv[]) { - //auto allowed = AuthAllowedSubnet::fromString(argv[1]); - //printIP("Base Address", allowed.baseAddress); - //printIP("Address Mask", allowed.addressMask); - //for (int idx = 1; idx < argc; ++idx) { - // auto addr = IPAddress::parse(argv[idx]); - //} - //return 0; // TODO: Remove later, this is just to force the statics to be initialized // otherwise the unit test won't run #ifdef ENABLE_SAMPLING @@ -1892,7 +1809,7 @@ int main(int argc, char* argv[]) { } else { g_network = newNet2(opts.tlsConfig, opts.useThreadPool, true); g_network->addStopCallback(Net2FileSystem::stop); - FlowTransport::createInstance(false, 1, WLTOKEN_RESERVED_COUNT); + FlowTransport::createInstance(false, 1, WLTOKEN_RESERVED_COUNT, &opts.allowList); const bool expectsPublicAddress = (role == ServerRole::FDBD || role == ServerRole::NetworkTestServer || role == ServerRole::Restore); diff --git a/fdbserver/worker.actor.cpp b/fdbserver/worker.actor.cpp index 9c19e13512..e2715f15b1 100644 --- a/fdbserver/worker.actor.cpp +++ b/fdbserver/worker.actor.cpp @@ -749,14 +749,14 @@ TEST_CASE("/fdbserver/worker/addressInDbAndPrimaryDc") { NetworkAddress grvProxyAddress(IPAddress(0x26262626), 1); GrvProxyInterface grvProxyInterf; grvProxyInterf.getConsistentReadVersion = - RequestStream(Endpoint({ grvProxyAddress }, UID(1, 2))); + RequestStream(Endpoint({ grvProxyAddress }, UID(1, 2))); testDbInfo.client.grvProxies.push_back(grvProxyInterf); ASSERT(addressInDbAndPrimaryDc(grvProxyAddress, makeReference>(testDbInfo))); NetworkAddress commitProxyAddress(IPAddress(0x37373737), 1); CommitProxyInterface commitProxyInterf; commitProxyInterf.commit = - RequestStream(Endpoint({ commitProxyAddress }, UID(1, 2))); + RequestStream(Endpoint({ commitProxyAddress }, UID(1, 2))); testDbInfo.client.commitProxies.push_back(commitProxyInterf); ASSERT(addressInDbAndPrimaryDc(commitProxyAddress, makeReference>(testDbInfo))); diff --git a/flow/CMakeLists.txt b/flow/CMakeLists.txt index a64b028974..990632f290 100644 --- a/flow/CMakeLists.txt +++ b/flow/CMakeLists.txt @@ -134,6 +134,7 @@ target_link_libraries(flow PUBLIC fmt::fmt) add_flow_target(STATIC_LIBRARY NAME flow_sampling SRCS ${FLOW_SRCS}) target_link_libraries(flow_sampling PRIVATE stacktrace) +target_link_libraries(flow_sampling PUBLIC fmt::fmt) target_compile_definitions(flow_sampling PRIVATE -DENABLE_SAMPLING) if(WIN32) add_dependencies(flow_sampling_actors flow_actors) From 53b4d8a307b4154427f2260618fa12707f948ae5 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Tue, 22 Feb 2022 14:29:51 +0100 Subject: [PATCH 05/24] Added ClientWorkload implementation --- fdbserver/CMakeLists.txt | 1 + fdbserver/tester.actor.cpp | 12 +- fdbserver/workloads/ClientWorkload.actor.cpp | 120 +++++++++++++++++++ fdbserver/workloads/workloads.actor.h | 28 ++++- 4 files changed, 155 insertions(+), 6 deletions(-) create mode 100644 fdbserver/workloads/ClientWorkload.actor.cpp diff --git a/fdbserver/CMakeLists.txt b/fdbserver/CMakeLists.txt index 4dea381051..4118f831ee 100644 --- a/fdbserver/CMakeLists.txt +++ b/fdbserver/CMakeLists.txt @@ -172,6 +172,7 @@ set(FDBSERVER_SRCS workloads/ClearSingleRange.actor.cpp workloads/ClientLibManagementWorkload.actor.cpp workloads/ClientTransactionProfileCorrectness.actor.cpp + workloads/ClientWorkload.actor.cpp workloads/TriggerRecovery.actor.cpp workloads/SuspendProcesses.actor.cpp workloads/CommitBugCheck.actor.cpp diff --git a/fdbserver/tester.actor.cpp b/fdbserver/tester.actor.cpp index 1581732625..8eb5724229 100644 --- a/fdbserver/tester.actor.cpp +++ b/fdbserver/tester.actor.cpp @@ -342,12 +342,14 @@ struct CompoundWorkload : TestWorkload { }; Reference getWorkloadIface(WorkloadRequest work, + Reference ccr, VectorRef options, Reference const> dbInfo) { Value testName = getOption(options, LiteralStringRef("testName"), LiteralStringRef("no-test-specified")); WorkloadContext wcx; wcx.clientId = work.clientId; wcx.clientCount = work.clientCount; + wcx.ccr = ccr; wcx.dbInfo = dbInfo; wcx.options = options; wcx.sharedRandomNumber = work.sharedRandomNumber; @@ -377,14 +379,16 @@ Reference getWorkloadIface(WorkloadRequest work, return workload; } -Reference getWorkloadIface(WorkloadRequest work, Reference const> dbInfo) { +Reference getWorkloadIface(WorkloadRequest work, + Reference ccr, + Reference const> dbInfo) { if (work.options.size() < 1) { TraceEvent(SevError, "TestCreationError").detail("Reason", "No options provided"); fprintf(stderr, "ERROR: No options were provided for workload.\n"); throw test_specification_invalid(); } if (work.options.size() == 1) - return getWorkloadIface(work, work.options[0], dbInfo); + return getWorkloadIface(work, ccr, work.options[0], dbInfo); WorkloadContext wcx; wcx.clientId = work.clientId; @@ -394,7 +398,7 @@ Reference getWorkloadIface(WorkloadRequest work, Reference(wcx); for (int i = 0; i < work.options.size(); i++) { - compound->add(getWorkloadIface(work, work.options[i], dbInfo)); + compound->add(getWorkloadIface(work, ccr, work.options[i], dbInfo)); } return compound; } @@ -647,7 +651,7 @@ ACTOR Future testerServerWorkload(WorkloadRequest work, // add test for "done" ? TraceEvent("WorkloadReceived", workIface.id()).detail("Title", work.title); - auto workload = getWorkloadIface(work, dbInfo); + auto workload = getWorkloadIface(work, ccr, dbInfo); if (!workload) { TraceEvent("TestCreationError").detail("Reason", "Workload could not be created"); fprintf(stderr, "ERROR: The workload could not be created.\n"); diff --git a/fdbserver/workloads/ClientWorkload.actor.cpp b/fdbserver/workloads/ClientWorkload.actor.cpp new file mode 100644 index 0000000000..8fd104a1e9 --- /dev/null +++ b/fdbserver/workloads/ClientWorkload.actor.cpp @@ -0,0 +1,120 @@ +/* + * workloads.actor.h + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2018 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "fdbserver/workloads/workloads.actor.h" +#include "fdbrpc/simulator.h" + +#include + +#include "flow/actorcompiler.h" // has to be last include + +struct ClientWorkloadImpl { + Reference child; + ISimulator::ProcessInfo* self = g_pSimulator->getCurrentProcess(); + ISimulator::ProcessInfo* childProcess = nullptr; + IPAddress childAddress; + std::string processName; + Database cx; + Future databaseOpened; + + ClientWorkloadImpl(Reference const& child) : child(child) { + if (self->address.isV6()) { + childAddress = + IPAddress::parse(fmt::format("2001:fdb1:fdb2:fdb3:fdb4:fdb5:fdb6:{:40x}", child->clientId + 2)).get(); + } else { + childAddress = IPAddress::parse(fmt::format("192.168.0.{}", child->clientId + 2)).get(); + } + processName = fmt::format("TestClient{}", child->clientId); + childProcess = g_simulator.newProcess(processName.c_str(), + childAddress, + 0, + self->address.isTLS(), + 1, + self->locality, + ProcessClass(ProcessClass::TesterClass, ProcessClass::AutoSource), + self->dataFolder, + self->coordinationFolder, + self->protocolVersion); + databaseOpened = openDatabase(this); + } + + ~ClientWorkloadImpl() { + g_simulator.destroyProcess(childProcess); + } + + + ACTOR static Future openDatabase(ClientWorkloadImpl* self) { + wait(g_simulator.onProcess(self->childProcess)); + self->cx = Database::createDatabase(self->child->ccr, -1); + wait(g_simulator.onProcess(self->self)); + return Void(); + } + + ACTOR template Future runActor(ClientWorkloadImpl* self, Fun f) { + state Optional err; + state Ret res; + wait(g_simulator.onProcess(self->childProcess)); + wait(self->databaseOpened); + try { + Ret r = wait(f(self->cx)); + res = r; + } catch(Error& e) { + if (e.code() == error_code_actor_cancelled) { + throw; + } + err = e; + } + wait(g_simulator.onProcess(self->self)); + if (err.present()) { + throw err.get(); + } + return res; + } + +}; + +ClientWorkload::ClientWorkload(Reference const& child, WorkloadContext const& wcx) + : TestWorkload(wcx), impl(new ClientWorkloadImpl(child)) {} + +std::string ClientWorkload::description() const { + return impl->child->description(); +} +Future ClientWorkload::setup(Database const& cx) { + return impl->runActor(impl, [this](Database const& db) { + return impl->child->setup(db); + }); +} +Future ClientWorkload::start(Database const& cx) { + return impl->runActor(impl, [this](Database const& db) { + return impl->child->start(db); + }); +} +Future ClientWorkload::check(Database const& cx) { + return impl->runActor(impl, [this](Database const& db) { + return impl->child->check(db); + }); +} +void ClientWorkload::getMetrics(std::vector& m) { + return impl->child->getMetrics(m); +} + +double ClientWorkload::getCheckTimeout() const { + return impl->child->getCheckTimeout(); +} diff --git a/fdbserver/workloads/workloads.actor.h b/fdbserver/workloads/workloads.actor.h index 1770c7eb52..9de4316e62 100644 --- a/fdbserver/workloads/workloads.actor.h +++ b/fdbserver/workloads/workloads.actor.h @@ -50,6 +50,7 @@ struct WorkloadContext { int clientId, clientCount; int64_t sharedRandomNumber; Reference const> dbInfo; + Reference ccr; WorkloadContext(); WorkloadContext(const WorkloadContext&); @@ -79,6 +80,20 @@ struct TestWorkload : NonCopyable, WorkloadContext, ReferenceCounted const& child, WorkloadContext const& wcx); + ~ClientWorkload(); + std::string description() const override; + Future setup(Database const& cx) override; + Future start(Database const& cx) override; + Future check(Database const& cx) override; + void getMetrics(std::vector& m) override; + + double getCheckTimeout() const override; +}; + struct KVWorkload : TestWorkload { uint64_t nodeCount; int64_t nodePrefix; @@ -121,8 +136,17 @@ struct IWorkloadFactory : ReferenceCounted { template struct WorkloadFactory : IWorkloadFactory { - WorkloadFactory(const char* name) { factories()[name] = Reference::addRef(this); } - Reference create(WorkloadContext const& wcx) override { return makeReference(wcx); } + bool asClient; + WorkloadFactory(const char* name, bool asClient = false) : asClient(asClient) { factories()[name] = Reference::addRef(this); } + Reference create(WorkloadContext const& wcx) override { + if (g_network->isSimulated() && asClient) { + WorkloadContext clientContext = wcx; + clientContext.dbInfo = decltype(clientContext.dbInfo)(); + auto child = makeReference(wcx); + return makeReference(child, wcx); + } + return makeReference(wcx); + } }; #define REGISTER_WORKLOAD(classname) WorkloadFactory classname##WorkloadFactory(#classname) From 174237efbcdc3092f555ee95ce999f2cda9c0149 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Wed, 23 Feb 2022 10:19:11 +0100 Subject: [PATCH 06/24] Various bug fixes --- fdbserver/tester.actor.cpp | 2 +- fdbserver/workloads/ClientWorkload.actor.cpp | 15 +++++++++++++-- fdbserver/workloads/Cycle.actor.cpp | 2 +- fdbserver/workloads/workloads.actor.h | 2 -- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/fdbserver/tester.actor.cpp b/fdbserver/tester.actor.cpp index 8eb5724229..633bc32bf6 100644 --- a/fdbserver/tester.actor.cpp +++ b/fdbserver/tester.actor.cpp @@ -47,7 +47,7 @@ WorkloadContext::WorkloadContext() {} WorkloadContext::WorkloadContext(const WorkloadContext& r) : options(r.options), clientId(r.clientId), clientCount(r.clientCount), sharedRandomNumber(r.sharedRandomNumber), - dbInfo(r.dbInfo) {} + dbInfo(r.dbInfo), ccr(r.ccr) {} WorkloadContext::~WorkloadContext() {} diff --git a/fdbserver/workloads/ClientWorkload.actor.cpp b/fdbserver/workloads/ClientWorkload.actor.cpp index 8fd104a1e9..cd938bda65 100644 --- a/fdbserver/workloads/ClientWorkload.actor.cpp +++ b/fdbserver/workloads/ClientWorkload.actor.cpp @@ -18,6 +18,7 @@ * limitations under the License. */ +#include "fdbserver/ServerDBInfo.actor.h" #include "fdbserver/workloads/workloads.actor.h" #include "fdbrpc/simulator.h" @@ -26,6 +27,7 @@ #include "flow/actorcompiler.h" // has to be last include struct ClientWorkloadImpl { + UID id = deterministicRandom()->randomUniqueID(); Reference child; ISimulator::ProcessInfo* self = g_pSimulator->getCurrentProcess(); ISimulator::ProcessInfo* childProcess = nullptr; @@ -37,10 +39,11 @@ struct ClientWorkloadImpl { ClientWorkloadImpl(Reference const& child) : child(child) { if (self->address.isV6()) { childAddress = - IPAddress::parse(fmt::format("2001:fdb1:fdb2:fdb3:fdb4:fdb5:fdb6:{:40x}", child->clientId + 2)).get(); + IPAddress::parse(fmt::format("2001:fdb1:fdb2:fdb3:fdb4:fdb5:fdb6:{:04x}", child->clientId + 2)).get(); } else { childAddress = IPAddress::parse(fmt::format("192.168.0.{}", child->clientId + 2)).get(); } + TraceEvent("TestClientStart", id).detail("ClusterFileLocation", child->ccr->getLocation()).log(); processName = fmt::format("TestClient{}", child->clientId); childProcess = g_simulator.newProcess(processName.c_str(), childAddress, @@ -62,6 +65,10 @@ struct ClientWorkloadImpl { ACTOR static Future openDatabase(ClientWorkloadImpl* self) { wait(g_simulator.onProcess(self->childProcess)); + FlowTransport::createInstance(true, 1, WLTOKEN_RESERVED_COUNT); + Sim2FileSystem::newFileSystem(); + TraceEvent("ClientWorkloadOpenDatabase", self->id) + .detail("ClusterFileLocation", self->child->ccr->getLocation()); self->cx = Database::createDatabase(self->child->ccr, -1); wait(g_simulator.onProcess(self->self)); return Void(); @@ -91,7 +98,11 @@ struct ClientWorkloadImpl { }; ClientWorkload::ClientWorkload(Reference const& child, WorkloadContext const& wcx) - : TestWorkload(wcx), impl(new ClientWorkloadImpl(child)) {} + : TestWorkload(wcx), impl(new ClientWorkloadImpl(child)) { + child->dbInfo = Reference>(); +} + +ClientWorkload::~ClientWorkload() {} std::string ClientWorkload::description() const { return impl->child->description(); diff --git a/fdbserver/workloads/Cycle.actor.cpp b/fdbserver/workloads/Cycle.actor.cpp index 87c477eab4..81cb6c438c 100644 --- a/fdbserver/workloads/Cycle.actor.cpp +++ b/fdbserver/workloads/Cycle.actor.cpp @@ -266,4 +266,4 @@ struct CycleWorkload : TestWorkload { } }; -WorkloadFactory CycleWorkloadFactory("Cycle"); +WorkloadFactory CycleWorkloadFactory("Cycle", true); diff --git a/fdbserver/workloads/workloads.actor.h b/fdbserver/workloads/workloads.actor.h index 9de4316e62..8ab99c709a 100644 --- a/fdbserver/workloads/workloads.actor.h +++ b/fdbserver/workloads/workloads.actor.h @@ -140,8 +140,6 @@ struct WorkloadFactory : IWorkloadFactory { WorkloadFactory(const char* name, bool asClient = false) : asClient(asClient) { factories()[name] = Reference::addRef(this); } Reference create(WorkloadContext const& wcx) override { if (g_network->isSimulated() && asClient) { - WorkloadContext clientContext = wcx; - clientContext.dbInfo = decltype(clientContext.dbInfo)(); auto child = makeReference(wcx); return makeReference(child, wcx); } From 102169ba3393467083dbd43feb90a76c454d7304 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Wed, 23 Feb 2022 10:23:27 +0100 Subject: [PATCH 07/24] Ran clang-format --- fdbclient/StorageServerInterface.h | 18 +++++++------ fdbrpc/FlowTransport.actor.cpp | 23 +++++++++++------ fdbrpc/FlowTransport.h | 5 +++- fdbrpc/IPAllowList.cpp | 12 +++------ fdbrpc/IPAllowList.h | 20 ++++++--------- fdbserver/ClusterController.actor.cpp | 6 +++-- fdbserver/fdbserver.actor.cpp | 2 +- fdbserver/workloads/ClientWorkload.actor.cpp | 27 +++++++------------- fdbserver/workloads/workloads.actor.h | 4 ++- 9 files changed, 58 insertions(+), 59 deletions(-) diff --git a/fdbclient/StorageServerInterface.h b/fdbclient/StorageServerInterface.h index 9636367ec9..a4d39e0315 100644 --- a/fdbclient/StorageServerInterface.h +++ b/fdbclient/StorageServerInterface.h @@ -107,7 +107,8 @@ struct StorageServerInterface { } if (Ar::isDeserializing) { getKey = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(1)); - getKeyValues = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(2)); + getKeyValues = + RequestStream(getValue.getEndpoint().getAdjustedEndpoint(2)); getShardState = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(3)); waitMetrics = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(4)); @@ -119,19 +120,20 @@ struct StorageServerInterface { RequestStream(getValue.getEndpoint().getAdjustedEndpoint(8)); getKeyValueStoreType = RequestStream>(getValue.getEndpoint().getAdjustedEndpoint(9)); - watchValue = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(10)); + watchValue = + RequestStream(getValue.getEndpoint().getAdjustedEndpoint(10)); getReadHotRanges = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(11)); getRangeSplitPoints = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(12)); - getKeyValuesStream = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(13)); - getKeyValuesAndFlatMap = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(14)); + getKeyValuesStream = RequestStream( + getValue.getEndpoint().getAdjustedEndpoint(13)); + getKeyValuesAndFlatMap = RequestStream( + getValue.getEndpoint().getAdjustedEndpoint(14)); changeFeedStream = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(15)); - overlappingChangeFeeds = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(16)); + overlappingChangeFeeds = RequestStream( + getValue.getEndpoint().getAdjustedEndpoint(16)); changeFeedPop = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(17)); changeFeedVersionUpdate = RequestStream( diff --git a/fdbrpc/FlowTransport.actor.cpp b/fdbrpc/FlowTransport.actor.cpp index a9dc7d8c1f..905b9a5440 100644 --- a/fdbrpc/FlowTransport.actor.cpp +++ b/fdbrpc/FlowTransport.actor.cpp @@ -944,8 +944,8 @@ ACTOR static void deliver(TransportData* self, Endpoint destination, TaskPriority priority, ArenaReader reader, - NetworkAddress peerAddress, - AuthorizedTenants authorizedTenants, + NetworkAddress peerAddress, + AuthorizedTenants authorizedTenants, bool inReadSocket) { // We want to run the task at the right priority. If the priority is higher than the current priority (which is // ReadSocket) we can just upgrade. Otherwise we'll context switch so that we don't block other tasks that might run @@ -961,8 +961,7 @@ ACTOR static void deliver(TransportData* self, auto receiver = self->endpoints.get(destination.token); if (receiver) { if (!authorizedTenants.trusted && !receiver->isPublic()) { - TraceEvent(SevWarnAlways, "AttemptedRPCToPrivatePrevented") - .detail("From", peerAddress); + TraceEvent(SevWarnAlways, "AttemptedRPCToPrivatePrevented").detail("From", peerAddress); throw connection_failed(); } if (!checkCompatible(receiver->peerCompatibilityPolicy(), reader.protocolVersion())) { @@ -1013,7 +1012,7 @@ static void scanPackets(TransportData* transport, const uint8_t* e, Arena& arena, NetworkAddress const& peerAddress, - AuthorizedTenants authorizedTenants, + AuthorizedTenants authorizedTenants, ProtocolVersion peerProtocolVersion) { // Find each complete packet in the given byte range and queue a ready task to deliver it. // Remove the complete packets from the range by increasing unprocessed_begin. @@ -1331,8 +1330,13 @@ ACTOR static Future connectionReader(TransportData* transport, if (!expectConnectPacket) { if (compatible || peerProtocolVersion.hasStableInterfaces()) { - scanPackets( - transport, unprocessed_begin, unprocessed_end, arena, peerAddress, authorizedTenants, peerProtocolVersion); + scanPackets(transport, + unprocessed_begin, + unprocessed_end, + arena, + peerAddress, + authorizedTenants, + peerProtocolVersion); } else { unprocessed_begin = unprocessed_end; peer->resetPing.trigger(); @@ -1808,7 +1812,10 @@ bool FlowTransport::incompatibleOutgoingConnectionsPresent() { return self->numIncompatibleConnections > 0; } -void FlowTransport::createInstance(bool isClient, uint64_t transportId, int maxWellKnownEndpoints, IPAllowList const* allowList) { +void FlowTransport::createInstance(bool isClient, + uint64_t transportId, + int maxWellKnownEndpoints, + IPAllowList const* allowList) { g_network->setGlobal(INetwork::enFlowTransport, (flowGlobalType) new FlowTransport(transportId, maxWellKnownEndpoints)); g_network->setGlobal(INetwork::enNetworkAddressFunc, (flowGlobalType)&FlowTransport::getGlobalLocalAddress); diff --git a/fdbrpc/FlowTransport.h b/fdbrpc/FlowTransport.h index 9ff39a5596..ced04ecfed 100644 --- a/fdbrpc/FlowTransport.h +++ b/fdbrpc/FlowTransport.h @@ -191,7 +191,10 @@ public: // Creates a new FlowTransport and makes FlowTransport::transport() return it. This uses g_network->global() // variables, so it will be private to a simulation. - static void createInstance(bool isClient, uint64_t transportId, int maxWellKnownEndpoints, IPAllowList const* allowList = nullptr); + static void createInstance(bool isClient, + uint64_t transportId, + int maxWellKnownEndpoints, + IPAllowList const* allowList = nullptr); static bool isClient() { return g_network->global(INetwork::enClientFailureMonitor) != nullptr; } diff --git a/fdbrpc/IPAllowList.cpp b/fdbrpc/IPAllowList.cpp index dbe7cfa012..f82e3939d5 100644 --- a/fdbrpc/IPAllowList.cpp +++ b/fdbrpc/IPAllowList.cpp @@ -63,7 +63,6 @@ IPAddress AuthAllowedSubnet::netmask() const { } } - int AuthAllowedSubnet::hostCount() const { if (addressMask.isV4()) { boost::asio::ip::address_v4 addr(addressMask.toV4()); @@ -150,11 +149,8 @@ IPAddress parseAddr(std::string const& str) { struct SubNetTest { AuthAllowedSubnet subnet; - SubNetTest(AuthAllowedSubnet&& subnet) - : subnet(subnet) - { - } - template + SubNetTest(AuthAllowedSubnet&& subnet) : subnet(subnet) {} + template static SubNetTest randomSubNetImpl() { constexpr int width = V4 ? 4 : 16; std::array binAddr; @@ -185,7 +181,7 @@ struct SubNetTest { } } - template + template IPAddress intArrayToAddress(uint32_t* arr) { if constexpr (V4) { return IPAddress(arr[0]); @@ -196,7 +192,7 @@ struct SubNetTest { } } - template + template I transformIntToSubnet(I val, I subnetMask, I baseAddress) { return (val & subnetMask) ^ baseAddress; } diff --git a/fdbrpc/IPAllowList.h b/fdbrpc/IPAllowList.h index 9860029e4a..f9bc51f795 100644 --- a/fdbrpc/IPAllowList.h +++ b/fdbrpc/IPAllowList.h @@ -37,7 +37,8 @@ struct AuthAllowedSubnet { static AuthAllowedSubnet fromString(std::string_view addressString); template - static auto createBitMask(std::array const& addr, int hostCount) -> std::array { + static auto createBitMask(std::array const& addr, int hostCount) + -> std::array { std::array res; res.fill((unsigned char)0xff); int idx = hostCount / 8; @@ -54,7 +55,7 @@ struct AuthAllowedSubnet { return res; } - bool operator() (IPAddress const& address) const { + bool operator()(IPAddress const& address) const { if (addressMask.isV4() != address.isV4()) { return false; } @@ -80,20 +81,15 @@ struct AuthAllowedSubnet { class IPAllowList { std::vector subnetList; + public: - void addTrustedSubnet(std::string_view str) { - subnetList.push_back(AuthAllowedSubnet::fromString(str)); - } + void addTrustedSubnet(std::string_view str) { subnetList.push_back(AuthAllowedSubnet::fromString(str)); } - void addTrustedSubnet(AuthAllowedSubnet const& subnet) { - subnetList.push_back(subnet); - } + void addTrustedSubnet(AuthAllowedSubnet const& subnet) { subnetList.push_back(subnet); } - std::vector const& subnets() const { - return subnetList; - } + std::vector const& subnets() const { return subnetList; } - bool operator() (IPAddress address) const { + bool operator()(IPAddress address) const { if (subnetList.empty()) { return true; } diff --git a/fdbserver/ClusterController.actor.cpp b/fdbserver/ClusterController.actor.cpp index eacef60897..b834eb298c 100644 --- a/fdbserver/ClusterController.actor.cpp +++ b/fdbserver/ClusterController.actor.cpp @@ -2919,7 +2919,8 @@ TEST_CASE("/fdbserver/clustercontroller/shouldTriggerRecoveryDueToDegradedServer testDbInfo.logSystemConfig.tLogs.push_back(remoteTLogSet); GrvProxyInterface proxyInterf; - proxyInterf.getConsistentReadVersion = RequestStream(Endpoint({ proxy }, testUID)); + proxyInterf.getConsistentReadVersion = + RequestStream(Endpoint({ proxy }, testUID)); testDbInfo.client.grvProxies.push_back(proxyInterf); ResolverInterface resolverInterf; @@ -3028,7 +3029,8 @@ TEST_CASE("/fdbserver/clustercontroller/shouldTriggerFailoverDueToDegradedServer testDbInfo.logSystemConfig.tLogs.push_back(remoteTLogSet); GrvProxyInterface grvProxyInterf; - grvProxyInterf.getConsistentReadVersion = RequestStream(Endpoint({ proxy }, testUID)); + grvProxyInterf.getConsistentReadVersion = + RequestStream(Endpoint({ proxy }, testUID)); testDbInfo.client.grvProxies.push_back(grvProxyInterf); CommitProxyInterface commitProxyInterf; diff --git a/fdbserver/fdbserver.actor.cpp b/fdbserver/fdbserver.actor.cpp index 2c2915f3f4..cc0217c72b 100644 --- a/fdbserver/fdbserver.actor.cpp +++ b/fdbserver/fdbserver.actor.cpp @@ -35,7 +35,7 @@ #include #include -#include +#include #include "fdbclient/ActorLineageProfiler.h" #include "fdbclient/ClusterConnectionFile.h" diff --git a/fdbserver/workloads/ClientWorkload.actor.cpp b/fdbserver/workloads/ClientWorkload.actor.cpp index cd938bda65..22b98c1ffd 100644 --- a/fdbserver/workloads/ClientWorkload.actor.cpp +++ b/fdbserver/workloads/ClientWorkload.actor.cpp @@ -58,23 +58,21 @@ struct ClientWorkloadImpl { databaseOpened = openDatabase(this); } - ~ClientWorkloadImpl() { - g_simulator.destroyProcess(childProcess); - } - + ~ClientWorkloadImpl() { g_simulator.destroyProcess(childProcess); } ACTOR static Future openDatabase(ClientWorkloadImpl* self) { wait(g_simulator.onProcess(self->childProcess)); FlowTransport::createInstance(true, 1, WLTOKEN_RESERVED_COUNT); Sim2FileSystem::newFileSystem(); TraceEvent("ClientWorkloadOpenDatabase", self->id) - .detail("ClusterFileLocation", self->child->ccr->getLocation()); + .detail("ClusterFileLocation", self->child->ccr->getLocation()); self->cx = Database::createDatabase(self->child->ccr, -1); wait(g_simulator.onProcess(self->self)); return Void(); } - ACTOR template Future runActor(ClientWorkloadImpl* self, Fun f) { + ACTOR template + Future runActor(ClientWorkloadImpl* self, Fun f) { state Optional err; state Ret res; wait(g_simulator.onProcess(self->childProcess)); @@ -82,7 +80,7 @@ struct ClientWorkloadImpl { try { Ret r = wait(f(self->cx)); res = r; - } catch(Error& e) { + } catch (Error& e) { if (e.code() == error_code_actor_cancelled) { throw; } @@ -94,11 +92,10 @@ struct ClientWorkloadImpl { } return res; } - }; ClientWorkload::ClientWorkload(Reference const& child, WorkloadContext const& wcx) - : TestWorkload(wcx), impl(new ClientWorkloadImpl(child)) { + : TestWorkload(wcx), impl(new ClientWorkloadImpl(child)) { child->dbInfo = Reference>(); } @@ -108,19 +105,13 @@ std::string ClientWorkload::description() const { return impl->child->description(); } Future ClientWorkload::setup(Database const& cx) { - return impl->runActor(impl, [this](Database const& db) { - return impl->child->setup(db); - }); + return impl->runActor(impl, [this](Database const& db) { return impl->child->setup(db); }); } Future ClientWorkload::start(Database const& cx) { - return impl->runActor(impl, [this](Database const& db) { - return impl->child->start(db); - }); + return impl->runActor(impl, [this](Database const& db) { return impl->child->start(db); }); } Future ClientWorkload::check(Database const& cx) { - return impl->runActor(impl, [this](Database const& db) { - return impl->child->check(db); - }); + return impl->runActor(impl, [this](Database const& db) { return impl->child->check(db); }); } void ClientWorkload::getMetrics(std::vector& m) { return impl->child->getMetrics(m); diff --git a/fdbserver/workloads/workloads.actor.h b/fdbserver/workloads/workloads.actor.h index 8ab99c709a..5f9b8bd6cc 100644 --- a/fdbserver/workloads/workloads.actor.h +++ b/fdbserver/workloads/workloads.actor.h @@ -137,7 +137,9 @@ struct IWorkloadFactory : ReferenceCounted { template struct WorkloadFactory : IWorkloadFactory { bool asClient; - WorkloadFactory(const char* name, bool asClient = false) : asClient(asClient) { factories()[name] = Reference::addRef(this); } + WorkloadFactory(const char* name, bool asClient = false) : asClient(asClient) { + factories()[name] = Reference::addRef(this); + } Reference create(WorkloadContext const& wcx) override { if (g_network->isSimulated() && asClient) { auto child = makeReference(wcx); From 20bf3e1599455eb54751741a877486c1e898230c Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Wed, 23 Feb 2022 19:02:29 +0100 Subject: [PATCH 08/24] Address review coomments --- fdbrpc/FlowTransport.actor.cpp | 30 +++++--- fdbrpc/FlowTransport.h | 2 +- fdbrpc/IPAllowList.cpp | 79 +++++++++++++++----- fdbrpc/IPAllowList.h | 12 +-- fdbserver/workloads/ClientWorkload.actor.cpp | 4 +- flow/ObjectSerializer.h | 16 ++-- 6 files changed, 101 insertions(+), 42 deletions(-) diff --git a/fdbrpc/FlowTransport.actor.cpp b/fdbrpc/FlowTransport.actor.cpp index 819be8b148..154750d214 100644 --- a/fdbrpc/FlowTransport.actor.cpp +++ b/fdbrpc/FlowTransport.actor.cpp @@ -258,7 +258,7 @@ struct TenantAuthorizer final : NetworkMessageReceiver { class TransportData { public: - TransportData(uint64_t transportId, int maxWellKnownEndpoints); + TransportData(uint64_t transportId, int maxWellKnownEndpoints, IPAllowList const* allowList); ~TransportData(); @@ -303,6 +303,7 @@ public: std::map multiVersionConnections; double lastIncompatibleMessage; uint64_t transportId; + IPAllowList allowList; Future multiVersionCleanup; Future pingLogger; @@ -365,9 +366,10 @@ ACTOR Future pingLatencyLogger(TransportData* self) { } } -TransportData::TransportData(uint64_t transportId, int maxWellKnownEndpoints) +TransportData::TransportData(uint64_t transportId, int maxWellKnownEndpoints, IPAllowList const* allowList) : warnAlwaysForLargePacket(true), endpoints(maxWellKnownEndpoints), endpointNotFoundReceiver(endpoints), - pingReceiver(endpoints), numIncompatibleConnections(0), lastIncompatibleMessage(0), transportId(transportId) { + pingReceiver(endpoints), numIncompatibleConnections(0), lastIncompatibleMessage(0), transportId(transportId), + allowList(allowList == nullptr ? IPAllowList() : *allowList) { degraded = makeReference>(false); pingLogger = pingLatencyLogger(this); } @@ -946,6 +948,7 @@ ACTOR static void deliver(TransportData* self, ArenaReader reader, NetworkAddress peerAddress, AuthorizedTenants authorizedTenants, + ContextVariableMap* cvm, bool inReadSocket) { // We want to run the task at the right priority. If the priority is higher than the current priority (which is // ReadSocket) we can just upgrade. Otherwise we'll context switch so that we don't block other tasks that might run @@ -972,9 +975,7 @@ ACTOR static void deliver(TransportData* self, StringRef data = reader.arenaReadAll(); ASSERT(data.size() > 8); ArenaObjectReader objReader(reader.arena(), reader.arenaReadAll(), AssumeVersion(reader.protocolVersion())); - bool didInsert = objReader.variable("AuthorizedTenants", &authorizedTenants); - didInsert = didInsert && objReader.variable("PeerAddress", &peerAddress); - ASSERT(didInsert); // check that we could set both context variables + objReader.setContextVariableMap(cvm); receiver->receive(objReader); g_currentDeliveryPeerAddress = { NetworkAddress() }; } catch (Error& e) { @@ -1012,7 +1013,8 @@ static void scanPackets(TransportData* transport, const uint8_t* e, Arena& arena, NetworkAddress const& peerAddress, - AuthorizedTenants authorizedTenants, + AuthorizedTenants const& authorizedTenants, + ContextVariableMap* cvm, ProtocolVersion peerProtocolVersion) { // Find each complete packet in the given byte range and queue a ready task to deliver it. // Remove the complete packets from the range by increasing unprocessed_begin. @@ -1132,6 +1134,7 @@ static void scanPackets(TransportData* transport, std::move(reader), peerAddress, authorizedTenants, + cvm, true); } @@ -1179,6 +1182,10 @@ ACTOR static Future connectionReader(TransportData* transport, state NetworkAddress peerAddress; state ProtocolVersion peerProtocolVersion; state AuthorizedTenants authorizedTenants; + authorizedTenants.trusted = transport->allowList(conn->getPeerAddress().ip); + ContextVariableMap cvm; + cvm["AuthorizedTenants"] = &authorizedTenants; + cvm["PeerAddress"] = &peerAddress; peerAddress = conn->getPeerAddress(); // TODO: check whether peers ip is in trusted range @@ -1336,6 +1343,7 @@ ACTOR static Future connectionReader(TransportData* transport, arena, peerAddress, authorizedTenants, + &cvm, peerProtocolVersion); } else { unprocessed_begin = unprocessed_end; @@ -1473,8 +1481,8 @@ ACTOR static Future multiVersionCleanupWorker(TransportData* self) { } } -FlowTransport::FlowTransport(uint64_t transportId, int maxWellKnownEndpoints) - : self(new TransportData(transportId, maxWellKnownEndpoints)) { +FlowTransport::FlowTransport(uint64_t transportId, int maxWellKnownEndpoints, IPAllowList const* allowList) + : self(new TransportData(transportId, maxWellKnownEndpoints, allowList)) { self->multiVersionCleanup = multiVersionCleanupWorker(self); } @@ -1594,6 +1602,7 @@ static void sendLocal(TransportData* self, ISerializeSource const& what, const E // SOMEDAY: Would it be better to avoid (de)serialization by doing this check in flow? Standalone copy; + ContextVariableMap cvm; ObjectWriter wr(AssumeVersion(g_network->protocolVersion())); what.serializeObjectWriter(wr); copy = wr.toStringRef(); @@ -1612,6 +1621,7 @@ static void sendLocal(TransportData* self, ISerializeSource const& what, const E ArenaReader(copy.arena(), copy, AssumeVersion(currentProtocolVersion)), NetworkAddress(), authorizedTenants, + &cvm, false); } } @@ -1817,7 +1827,7 @@ void FlowTransport::createInstance(bool isClient, int maxWellKnownEndpoints, IPAllowList const* allowList) { g_network->setGlobal(INetwork::enFlowTransport, - (flowGlobalType) new FlowTransport(transportId, maxWellKnownEndpoints)); + (flowGlobalType) new FlowTransport(transportId, maxWellKnownEndpoints, allowList)); g_network->setGlobal(INetwork::enNetworkAddressFunc, (flowGlobalType)&FlowTransport::getGlobalLocalAddress); g_network->setGlobal(INetwork::enNetworkAddressesFunc, (flowGlobalType)&FlowTransport::getGlobalLocalAddresses); g_network->setGlobal(INetwork::enFailureMonitor, (flowGlobalType) new SimpleFailureMonitor()); diff --git a/fdbrpc/FlowTransport.h b/fdbrpc/FlowTransport.h index ced04ecfed..8aa2e90dfc 100644 --- a/fdbrpc/FlowTransport.h +++ b/fdbrpc/FlowTransport.h @@ -186,7 +186,7 @@ class IPAllowList; class FlowTransport { public: - FlowTransport(uint64_t transportId, int maxWellKnownEndpoints); + FlowTransport(uint64_t transportId, int maxWellKnownEndpoints, IPAllowList const* allowList); ~FlowTransport(); // Creates a new FlowTransport and makes FlowTransport::transport() return it. This uses g_network->global() diff --git a/fdbrpc/IPAllowList.cpp b/fdbrpc/IPAllowList.cpp index f82e3939d5..a62d661692 100644 --- a/fdbrpc/IPAllowList.cpp +++ b/fdbrpc/IPAllowList.cpp @@ -1,5 +1,5 @@ /* - * IPAllowList.h + * IPAllowList.cpp * * This source file is part of the FoundationDB open source project * @@ -37,7 +37,7 @@ void printIP(std::array const& addr) { } template -int hostCountImpl(std::array const& addr) { +int netmaskWeightImpl(std::array const& addr) { int count = 0; for (int i = 0; i < addr.size() && addr[i] != 0xff; ++i) { std::bitset<8> b(addr[i]); @@ -63,26 +63,26 @@ IPAddress AuthAllowedSubnet::netmask() const { } } -int AuthAllowedSubnet::hostCount() const { +int AuthAllowedSubnet::netmaskWeight() const { if (addressMask.isV4()) { boost::asio::ip::address_v4 addr(addressMask.toV4()); - return hostCountImpl(addr.to_bytes()); + return netmaskWeightImpl(addr.to_bytes()); } else { - return hostCountImpl(addressMask.toV6()); + return netmaskWeightImpl(addressMask.toV6()); } } AuthAllowedSubnet AuthAllowedSubnet::fromString(std::string_view addressString) { auto pos = addressString.find('/'); if (pos == std::string_view::npos) { - fmt::print("ERROR: {} is not a valid (use Network-Prefix/hostcount syntax)\n"); + fmt::print("ERROR: {} is not a valid (use Network-Prefix/netmaskWeight syntax)\n"); throw invalid_option(); } auto address = addressString.substr(0, pos); - auto hostCount = std::stoi(std::string(addressString.substr(pos + 1))); + auto netmaskWeight = std::stoi(std::string(addressString.substr(pos + 1))); auto addr = boost::asio::ip::make_address(address); if (addr.is_v4()) { - auto bM = createBitMask(addr.to_v4().to_bytes(), hostCount); + auto bM = createBitMask(addr.to_v4().to_bytes(), netmaskWeight); // we typically would expect a base address has been passed, but to be safe we still // will make the last bits 0 auto mask = boost::asio::ip::address_v4(bM).to_uint(); @@ -92,7 +92,7 @@ AuthAllowedSubnet AuthAllowedSubnet::fromString(std::string_view addressString) printIP("Mask:", IPAddress(mask)); return AuthAllowedSubnet(IPAddress(baseAddress), IPAddress(mask)); } else { - auto mask = createBitMask(addr.to_v6().to_bytes(), hostCount); + auto mask = createBitMask(addr.to_v6().to_bytes(), netmaskWeight); auto baseAddress = addr.to_v6().to_bytes(); for (int i = 0; i < mask.size(); ++i) { baseAddress[i] &= mask[i]; @@ -149,7 +149,8 @@ IPAddress parseAddr(std::string const& str) { struct SubNetTest { AuthAllowedSubnet subnet; - SubNetTest(AuthAllowedSubnet&& subnet) : subnet(subnet) {} + SubNetTest(AuthAllowedSubnet&& subnet) : subnet(std::move(subnet)) {} + SubNetTest(AuthAllowedSubnet const& subnet) : subnet(subnet) {} template static SubNetTest randomSubNetImpl() { constexpr int width = V4 ? 4 : 16; @@ -162,7 +163,7 @@ struct SubNetTest { } binAddr[i] = rnd[i % 4]; } - auto hostCount = deterministicRandom()->randomInt(1, width); + auto netmaskWeight = deterministicRandom()->randomInt(1, width); std::string address; if constexpr (V4) { address_v4 a(binAddr); @@ -171,7 +172,7 @@ struct SubNetTest { address_v6 a(binAddr); address = a.to_string(); } - return SubNetTest(AuthAllowedSubnet::fromString(fmt::format("{}/{}", address, hostCount))); + return SubNetTest(AuthAllowedSubnet::fromString(fmt::format("{}/{}", address, netmaskWeight))); } static SubNetTest randomSubNet() { if (deterministicRandom()->coinflip()) { @@ -182,7 +183,7 @@ struct SubNetTest { } template - IPAddress intArrayToAddress(uint32_t* arr) { + static IPAddress intArrayToAddress(uint32_t* arr) { if constexpr (V4) { return IPAddress(arr[0]); } else { @@ -197,16 +198,22 @@ struct SubNetTest { return (val & subnetMask) ^ baseAddress; } + template + static IPAddress randomAddress() { + constexpr int width = V4 ? 4 : 16; + uint32_t rnd[width / 4]; + for (int i = 0; i < width / 4; ++i) { + rnd[i] = deterministicRandom()->randomUInt32(); + } + return intArrayToAddress(rnd); + } + template IPAddress randomAddress(bool inSubnet) { ASSERT(V4 == subnet.baseAddress.isV4() || !inSubnet); constexpr int width = V4 ? 4 : 16; for (;;) { - uint32_t rnd[width / 4]; - for (int i = 0; i < width / 4; ++i) { - rnd[i] = deterministicRandom()->randomUInt32(); - } - auto res = intArrayToAddress(rnd); + auto res = randomAddress(); if (V4 != subnet.baseAddress.isV4()) { return res; } @@ -272,6 +279,42 @@ TEST_CASE("/fdbrpc/allow_list") { ::subnetAssert(allowList, parseAddr("5.2.1.1"), true); ::subnetAssert(allowList, parseAddr("128.0.1.1"), false); ::subnetAssert(allowList, parseAddr("192.168.3.1"), false); + allowList = IPAllowList(); + allowList.addTrustedSubnet("0.0.0.0/0"); + SubNetTest subnetTest(allowList.subnets()[0]); + for (int i = 0; i < 10; ++i) { + // All IPv4 addresses are in the allow list + ::subnetAssert(allowList, subnetTest.randomAddress(), true); + // No IPv6 addresses are in the allow list + ::subnetAssert(allowList, subnetTest.randomAddress(), false); + } + allowList = IPAllowList(); + allowList.addTrustedSubnet("::/0"); + subnetTest = SubNetTest(allowList.subnets()[0]); + for (int i = 0; i < 10; ++i) { + // All IPv6 addresses are in the allow list + ::subnetAssert(allowList, subnetTest.randomAddress(), true); + // No IPv4 addresses are ub the allow list + ::subnetAssert(allowList, subnetTest.randomAddress(), false); + } + allowList = IPAllowList(); + IPAddress baseAddress = SubNetTest::randomAddress(); + allowList.addTrustedSubnet(fmt::format("{}/32", baseAddress.toString())); + for (int i = 0; i < 10; ++i) { + auto rnd = SubNetTest::randomAddress(); + ::subnetAssert(allowList, rnd, rnd == baseAddress); + rnd = SubNetTest::randomAddress(); + ::subnetAssert(allowList, rnd, false); + } + allowList = IPAllowList(); + baseAddress = SubNetTest::randomAddress(); + allowList.addTrustedSubnet(fmt::format("{}/128", baseAddress.toString())); + for (int i = 0; i < 10; ++i) { + auto rnd = SubNetTest::randomAddress(); + ::subnetAssert(allowList, rnd, rnd == baseAddress); + rnd = SubNetTest::randomAddress(); + ::subnetAssert(allowList, rnd, false); + } for (int i = 0; i < 100; ++i) { SubNetTest subnetTest(SubNetTest::randomSubNet()); allowList = IPAllowList(); diff --git a/fdbrpc/IPAllowList.h b/fdbrpc/IPAllowList.h index f9bc51f795..a89939add6 100644 --- a/fdbrpc/IPAllowList.h +++ b/fdbrpc/IPAllowList.h @@ -37,15 +37,15 @@ struct AuthAllowedSubnet { static AuthAllowedSubnet fromString(std::string_view addressString); template - static auto createBitMask(std::array const& addr, int hostCount) + static auto createBitMask(std::array const& addr, int netmaskWeight) -> std::array { std::array res; res.fill((unsigned char)0xff); - int idx = hostCount / 8; - if (hostCount % 8 > 0) { - // 2^(hostCount % 8) - 1 sets the last (hostCount % 8) number of bits to 1 + int idx = netmaskWeight / 8; + if (netmaskWeight % 8 > 0) { + // 2^(netmaskWeight % 8) - 1 sets the last (netmaskWeight % 8) number of bits to 1 // everything else will be zero. For example: 2^3 - 1 == 7 == 0b111 - unsigned char bitmask = (1 << (hostCount % 8)) - ((unsigned char)1); + unsigned char bitmask = (1 << (netmaskWeight % 8)) - ((unsigned char)1); res[idx] ^= bitmask; ++idx; } @@ -73,7 +73,7 @@ struct AuthAllowedSubnet { IPAddress netmask() const; - int hostCount() const; + int netmaskWeight() const; // some useful helper functions if we need to debug ip masks etc static void printIP(std::string_view txt, IPAddress const& address); diff --git a/fdbserver/workloads/ClientWorkload.actor.cpp b/fdbserver/workloads/ClientWorkload.actor.cpp index 22b98c1ffd..44ef221eaf 100644 --- a/fdbserver/workloads/ClientWorkload.actor.cpp +++ b/fdbserver/workloads/ClientWorkload.actor.cpp @@ -1,9 +1,9 @@ /* - * workloads.actor.h + * ClientWorkload.actor.cpp * * This source file is part of the FoundationDB open source project * - * Copyright 2013-2018 Apple Inc. and the FoundationDB project authors + * Copyright 2013-2022 Apple Inc. and the FoundationDB project authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/flow/ObjectSerializer.h b/flow/ObjectSerializer.h index 45f4fac6f3..f8843df9a6 100644 --- a/flow/ObjectSerializer.h +++ b/flow/ObjectSerializer.h @@ -26,12 +26,14 @@ #include +using ContextVariableMap = std::unordered_map; + template struct LoadContext { Ar* ar; - std::unordered_map variables; + ContextVariableMap* variables; - LoadContext(Ar* ar) : ar(ar) {} + LoadContext(Ar* ar, ContextVariableMap* variables = nullptr) : ar(ar), variables(variables) {} Arena& arena() { return ar->arena(); } ProtocolVersion protocolVersion() const { return ar->protocolVersion(); } @@ -54,19 +56,20 @@ struct LoadContext { template bool variable(std::string_view name, T* val) { - auto p = variables.insert(std::make_pair(name, val)); + auto p = variables->insert(std::make_pair(name, val)); return p.second; } template T& variable(std::string_view name) { - auto res = variables.at(name); + auto res = variables->at(name); return *reinterpret_cast(res); } template T const& variable(std::string_view name) const { - return const_cast*>(this)->variable(name); + auto res = variables->at(name); + return *reinterpret_cast(res); } }; @@ -96,6 +99,9 @@ public: _ObjectReader() : context(static_cast(this)) {} ProtocolVersion protocolVersion() const { return mProtocolVersion; } void setProtocolVersion(ProtocolVersion v) { mProtocolVersion = v; } + void setContextVariableMap(ContextVariableMap* cvm) { + context.variables = cvm; + } template void deserialize(FileIdentifier file_identifier, Items&... items) { From 0aa78236222e0a286944d30087fd3ce95892a66f Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Fri, 25 Feb 2022 14:54:52 +0100 Subject: [PATCH 09/24] Pass object as reference --- fdbrpc/FlowTransport.actor.cpp | 17 ++++++++--------- fdbrpc/IPAllowList.cpp | 1 - fdbrpc/TenantInfo.h | 2 +- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/fdbrpc/FlowTransport.actor.cpp b/fdbrpc/FlowTransport.actor.cpp index 154750d214..ced86de7d6 100644 --- a/fdbrpc/FlowTransport.actor.cpp +++ b/fdbrpc/FlowTransport.actor.cpp @@ -947,7 +947,7 @@ ACTOR static void deliver(TransportData* self, TaskPriority priority, ArenaReader reader, NetworkAddress peerAddress, - AuthorizedTenants authorizedTenants, + Reference authorizedTenants, ContextVariableMap* cvm, bool inReadSocket) { // We want to run the task at the right priority. If the priority is higher than the current priority (which is @@ -963,7 +963,7 @@ ACTOR static void deliver(TransportData* self, auto receiver = self->endpoints.get(destination.token); if (receiver) { - if (!authorizedTenants.trusted && !receiver->isPublic()) { + if (!authorizedTenants->trusted && !receiver->isPublic()) { TraceEvent(SevWarnAlways, "AttemptedRPCToPrivatePrevented").detail("From", peerAddress); throw connection_failed(); } @@ -1013,7 +1013,7 @@ static void scanPackets(TransportData* transport, const uint8_t* e, Arena& arena, NetworkAddress const& peerAddress, - AuthorizedTenants const& authorizedTenants, + Reference const& authorizedTenants, ContextVariableMap* cvm, ProtocolVersion peerProtocolVersion) { // Find each complete packet in the given byte range and queue a ready task to deliver it. @@ -1181,15 +1181,14 @@ ACTOR static Future connectionReader(TransportData* transport, state bool incompatibleProtocolVersionNewer = false; state NetworkAddress peerAddress; state ProtocolVersion peerProtocolVersion; - state AuthorizedTenants authorizedTenants; - authorizedTenants.trusted = transport->allowList(conn->getPeerAddress().ip); + state Reference authorizedTenants = makeReference(); + authorizedTenants->trusted = transport->allowList(conn->getPeerAddress().ip); ContextVariableMap cvm; cvm["AuthorizedTenants"] = &authorizedTenants; cvm["PeerAddress"] = &peerAddress; peerAddress = conn->getPeerAddress(); - // TODO: check whether peers ip is in trusted range - authorizedTenants.trusted = true; + authorizedTenants->trusted = transport->allowList(peerAddress.ip); if (!peer) { ASSERT(!peerAddress.isPublic()); } @@ -1613,8 +1612,8 @@ static void sendLocal(TransportData* self, ISerializeSource const& what, const E ASSERT(copy.size() > 0); TaskPriority priority = self->endpoints.getPriority(destination.token); if (priority != TaskPriority::UnknownEndpoint || (destination.token.first() & TOKEN_STREAM_FLAG) != 0) { - AuthorizedTenants authorizedTenants; - authorizedTenants.trusted = true; + Reference authorizedTenants; + authorizedTenants->trusted = true; deliver(self, destination, priority, diff --git a/fdbrpc/IPAllowList.cpp b/fdbrpc/IPAllowList.cpp index a62d661692..b018c696d5 100644 --- a/fdbrpc/IPAllowList.cpp +++ b/fdbrpc/IPAllowList.cpp @@ -211,7 +211,6 @@ struct SubNetTest { template IPAddress randomAddress(bool inSubnet) { ASSERT(V4 == subnet.baseAddress.isV4() || !inSubnet); - constexpr int width = V4 ? 4 : 16; for (;;) { auto res = randomAddress(); if (V4 != subnet.baseAddress.isV4()) { diff --git a/fdbrpc/TenantInfo.h b/fdbrpc/TenantInfo.h index ad350f5b39..3efec71845 100644 --- a/fdbrpc/TenantInfo.h +++ b/fdbrpc/TenantInfo.h @@ -50,7 +50,7 @@ struct TenantInfoRef { } }; -struct AuthorizedTenants { +struct AuthorizedTenants : ReferenceCounted { Arena arena; std::set authorizedTenants; bool trusted = false; From bed799220a3539e131417aa4c0f88f46d04474ca Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Tue, 15 Mar 2022 16:57:26 +0100 Subject: [PATCH 10/24] Addressed review comments, added test --- fdbclient/BackupAgent.actor.h | 2 +- fdbclient/BackupAgentBase.actor.cpp | 6 +- fdbclient/CommitProxyInterface.h | 10 +- fdbclient/CoordinationInterface.h | 4 +- fdbclient/GrvProxyInterface.h | 2 +- fdbclient/NativeAPI.actor.cpp | 2 +- fdbclient/StorageServerInterface.h | 38 +++--- fdbclient/WellKnownEndpoints.h | 40 +++--- fdbrpc/FlowTransport.actor.cpp | 20 +-- fdbrpc/FlowTransport.h | 2 +- fdbrpc/IPAllowList.cpp | 4 +- fdbrpc/fdbrpc.h | 5 + fdbrpc/simulator.h | 1 + fdbserver/ApplyMetadataMutation.cpp | 2 +- fdbserver/CMakeLists.txt | 1 + fdbserver/ClusterController.actor.cpp | 6 +- fdbserver/GrvProxyServer.actor.cpp | 4 +- fdbserver/ProxyCommitData.actor.h | 8 +- fdbserver/SimulatedCluster.actor.cpp | 20 ++- fdbserver/worker.actor.cpp | 4 +- fdbserver/workloads/ClientWorkload.actor.cpp | 12 +- .../workloads/PrivateEndpoints.actor.cpp | 127 ++++++++++++++++++ fdbserver/workloads/SaveAndKill.actor.cpp | 2 +- flow/ObjectSerializer.h | 13 +- tests/SpecificUnitTest.txt | 4 +- tests/fast/CycleTest.toml | 32 +++++ tests/fast/PrivateEndpoints.toml | 5 + 27 files changed, 283 insertions(+), 93 deletions(-) create mode 100644 fdbserver/workloads/PrivateEndpoints.actor.cpp create mode 100644 tests/fast/PrivateEndpoints.toml diff --git a/fdbclient/BackupAgent.actor.h b/fdbclient/BackupAgent.actor.h index 1f18b10a4c..37c42ef482 100644 --- a/fdbclient/BackupAgent.actor.h +++ b/fdbclient/BackupAgent.actor.h @@ -561,7 +561,7 @@ ACTOR Future applyMutations(Database cx, Key removePrefix, Version beginVersion, Version* endVersion, - RequestStream commit, + PublicRequestStream commit, NotifiedVersion* committedVersion, Reference> keyVersion); ACTOR Future cleanupBackup(Database cx, DeleteData deleteData); diff --git a/fdbclient/BackupAgentBase.actor.cpp b/fdbclient/BackupAgentBase.actor.cpp index 75445d52f3..a26f5fe88b 100644 --- a/fdbclient/BackupAgentBase.actor.cpp +++ b/fdbclient/BackupAgentBase.actor.cpp @@ -598,7 +598,7 @@ ACTOR Future dumpData(Database cx, Key uid, Key addPrefix, Key removePrefix, - RequestStream commit, + PublicRequestStream commit, NotifiedVersion* committedVersion, Optional endVersion, Key rangeBegin, @@ -675,7 +675,7 @@ ACTOR Future dumpData(Database cx, ACTOR Future coalesceKeyVersionCache(Key uid, Version endVersion, Reference> keyVersion, - RequestStream commit, + PublicRequestStream commit, NotifiedVersion* committedVersion, PromiseStream> addActor, FlowLock* commitLock) { @@ -725,7 +725,7 @@ ACTOR Future applyMutations(Database cx, Key removePrefix, Version beginVersion, Version* endVersion, - RequestStream commit, + PublicRequestStream commit, NotifiedVersion* committedVersion, Reference> keyVersion) { state FlowLock commitLock(CLIENT_KNOBS->BACKUP_LOCK_BYTES); diff --git a/fdbclient/CommitProxyInterface.h b/fdbclient/CommitProxyInterface.h index 90b39adb9a..eb4bde47a0 100644 --- a/fdbclient/CommitProxyInterface.h +++ b/fdbclient/CommitProxyInterface.h @@ -42,13 +42,13 @@ struct CommitProxyInterface { Optional processId; bool provisional; - RequestStream commit; - RequestStream + PublicRequestStream commit; + PublicRequestStream getConsistentReadVersion; // Returns a version which (1) is committed, and (2) is >= the latest version reported // committed (by a commit response) when this request was sent // (at some point between when this request is sent and when its response is // received, the latest version reported committed) - RequestStream getKeyServersLocations; + PublicRequestStream getKeyServersLocations; RequestStream getStorageServerRejoinInfo; RequestStream> waitFailure; @@ -71,9 +71,9 @@ struct CommitProxyInterface { serializer(ar, processId, provisional, commit); if (Archive::isDeserializing) { getConsistentReadVersion = - RequestStream(commit.getEndpoint().getAdjustedEndpoint(1)); + PublicRequestStream(commit.getEndpoint().getAdjustedEndpoint(1)); getKeyServersLocations = - RequestStream(commit.getEndpoint().getAdjustedEndpoint(2)); + PublicRequestStream(commit.getEndpoint().getAdjustedEndpoint(2)); getStorageServerRejoinInfo = RequestStream(commit.getEndpoint().getAdjustedEndpoint(3)); waitFailure = RequestStream>(commit.getEndpoint().getAdjustedEndpoint(4)); diff --git a/fdbclient/CoordinationInterface.h b/fdbclient/CoordinationInterface.h index 8e6d430c64..deeabd08d7 100644 --- a/fdbclient/CoordinationInterface.h +++ b/fdbclient/CoordinationInterface.h @@ -32,8 +32,8 @@ const int MAX_CLUSTER_FILE_BYTES = 60000; struct ClientLeaderRegInterface { - RequestStream getLeader; - RequestStream openDatabase; + PublicRequestStream getLeader; + PublicRequestStream openDatabase; RequestStream checkDescriptorMutable; ClientLeaderRegInterface() {} diff --git a/fdbclient/GrvProxyInterface.h b/fdbclient/GrvProxyInterface.h index a555f2efd5..f52bdac6ad 100644 --- a/fdbclient/GrvProxyInterface.h +++ b/fdbclient/GrvProxyInterface.h @@ -36,7 +36,7 @@ struct GrvProxyInterface { Optional processId; bool provisional; - RequestStream + PublicRequestStream getConsistentReadVersion; // Returns a version which (1) is committed, and (2) is >= the latest version reported // committed (by a commit response) when this request was sent // (at some point between when this request is sent and when its response is received, the latest version reported diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp index d0efa50a2a..3dd151e05d 100644 --- a/fdbclient/NativeAPI.actor.cpp +++ b/fdbclient/NativeAPI.actor.cpp @@ -3352,7 +3352,7 @@ void transformRangeLimits(GetRangeLimits limits, Reverse reverse, GetKeyValuesFa } template -RequestStream StorageServerInterface::*getRangeRequestStream() { +PublicRequestStream StorageServerInterface::*getRangeRequestStream() { if constexpr (std::is_same::value) { return &StorageServerInterface::getKeyValues; } else if (std::is_same::value) { diff --git a/fdbclient/StorageServerInterface.h b/fdbclient/StorageServerInterface.h index 01248eb71d..9bbb118041 100644 --- a/fdbclient/StorageServerInterface.h +++ b/fdbclient/StorageServerInterface.h @@ -61,13 +61,13 @@ struct StorageServerInterface { UID uniqueID; Optional tssPairID; - RequestStream getValue; - RequestStream getKey; + PublicRequestStream getValue; + PublicRequestStream getKey; // Throws a wrong_shard_server if the keys in the request or result depend on data outside this server OR if a large // selector offset prevents all data from being read in one range read - RequestStream getKeyValues; - RequestStream getKeyValuesAndFlatMap; + PublicRequestStream getKeyValues; + PublicRequestStream getKeyValuesAndFlatMap; RequestStream getShardState; RequestStream waitMetrics; @@ -77,14 +77,14 @@ struct StorageServerInterface { RequestStream getQueuingMetrics; RequestStream> getKeyValueStoreType; - RequestStream watchValue; + PublicRequestStream watchValue; RequestStream getReadHotRanges; RequestStream getRangeSplitPoints; - RequestStream getKeyValuesStream; - RequestStream changeFeedStream; - RequestStream overlappingChangeFeeds; - RequestStream changeFeedPop; - RequestStream changeFeedVersionUpdate; + PublicRequestStream getKeyValuesStream; + PublicRequestStream changeFeedStream; + PublicRequestStream overlappingChangeFeeds; + PublicRequestStream changeFeedPop; + PublicRequestStream changeFeedVersionUpdate; explicit StorageServerInterface(UID uid) : uniqueID(uid) {} StorageServerInterface() : uniqueID(deterministicRandom()->randomUniqueID()) {} @@ -107,9 +107,9 @@ struct StorageServerInterface { serializer(ar, uniqueID, locality, getValue); } if (Ar::isDeserializing) { - getKey = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(1)); + getKey = PublicRequestStream(getValue.getEndpoint().getAdjustedEndpoint(1)); getKeyValues = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(2)); + PublicRequestStream(getValue.getEndpoint().getAdjustedEndpoint(2)); getShardState = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(3)); waitMetrics = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(4)); @@ -122,22 +122,22 @@ struct StorageServerInterface { getKeyValueStoreType = RequestStream>(getValue.getEndpoint().getAdjustedEndpoint(9)); watchValue = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(10)); + PublicRequestStream(getValue.getEndpoint().getAdjustedEndpoint(10)); getReadHotRanges = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(11)); getRangeSplitPoints = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(12)); - getKeyValuesStream = RequestStream( + getKeyValuesStream = PublicRequestStream( getValue.getEndpoint().getAdjustedEndpoint(13)); - getKeyValuesAndFlatMap = RequestStream( + getKeyValuesAndFlatMap = PublicRequestStream( getValue.getEndpoint().getAdjustedEndpoint(14)); changeFeedStream = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(15)); - overlappingChangeFeeds = RequestStream( + PublicRequestStream(getValue.getEndpoint().getAdjustedEndpoint(15)); + overlappingChangeFeeds = PublicRequestStream( getValue.getEndpoint().getAdjustedEndpoint(16)); changeFeedPop = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(17)); - changeFeedVersionUpdate = RequestStream( + PublicRequestStream(getValue.getEndpoint().getAdjustedEndpoint(17)); + changeFeedVersionUpdate = PublicRequestStream( getValue.getEndpoint().getAdjustedEndpoint(18)); } } else { diff --git a/fdbclient/WellKnownEndpoints.h b/fdbclient/WellKnownEndpoints.h index 6452e62958..79c882aefa 100644 --- a/fdbclient/WellKnownEndpoints.h +++ b/fdbclient/WellKnownEndpoints.h @@ -29,27 +29,27 @@ */ enum WellKnownEndpoints { WLTOKEN_CLIENTLEADERREG_GETLEADER = WLTOKEN_FIRST_AVAILABLE, // 2 - WLTOKEN_CLIENTLEADERREG_OPENDATABASE, // 3 - WLTOKEN_LEADERELECTIONREG_CANDIDACY, // 4 - WLTOKEN_LEADERELECTIONREG_ELECTIONRESULT, // 5 - WLTOKEN_LEADERELECTIONREG_LEADERHEARTBEAT, // 6 - WLTOKEN_LEADERELECTIONREG_FORWARD, // 7 - WLTOKEN_GENERATIONREG_READ, // 8 - WLTOKEN_GENERATIONREG_WRITE, // 9 + WLTOKEN_CLIENTLEADERREG_OPENDATABASE, // 4 + WLTOKEN_LEADERELECTIONREG_CANDIDACY, // 5 + WLTOKEN_LEADERELECTIONREG_ELECTIONRESULT, // 6 + WLTOKEN_LEADERELECTIONREG_LEADERHEARTBEAT, // 7 + WLTOKEN_LEADERELECTIONREG_FORWARD, // 8 + WLTOKEN_GENERATIONREG_READ, // 9 WLTOKEN_PROTOCOL_INFO, // 10 : the value of this endpoint should be stable and not change. - WLTOKEN_CLIENTLEADERREG_DESCRIPTOR_MUTABLE, // 11 - WLTOKEN_CONFIGTXN_GETGENERATION, // 12 - WLTOKEN_CONFIGTXN_GET, // 13 - WLTOKEN_CONFIGTXN_GETCLASSES, // 14 - WLTOKEN_CONFIGTXN_GETKNOBS, // 15 - WLTOKEN_CONFIGTXN_COMMIT, // 16 - WLTOKEN_CONFIGFOLLOWER_GETSNAPSHOTANDCHANGES, // 17 - WLTOKEN_CONFIGFOLLOWER_GETCHANGES, // 18 - WLTOKEN_CONFIGFOLLOWER_COMPACT, // 19 - WLTOKEN_CONFIGFOLLOWER_ROLLFORWARD, // 20 - WLTOKEN_CONFIGFOLLOWER_GETCOMMITTEDVERSION, // 21 - WLTOKEN_PROCESS, // 22 - WLTOKEN_RESERVED_COUNT // 23 + WLTOKEN_GENERATIONREG_WRITE, // 11 + WLTOKEN_CLIENTLEADERREG_DESCRIPTOR_MUTABLE, // 12 + WLTOKEN_CONFIGTXN_GETGENERATION, // 13 + WLTOKEN_CONFIGTXN_GET, // 14 + WLTOKEN_CONFIGTXN_GETCLASSES, // 15 + WLTOKEN_CONFIGTXN_GETKNOBS, // 16 + WLTOKEN_CONFIGTXN_COMMIT, // 17 + WLTOKEN_CONFIGFOLLOWER_GETSNAPSHOTANDCHANGES, // 18 + WLTOKEN_CONFIGFOLLOWER_GETCHANGES, // 19 + WLTOKEN_CONFIGFOLLOWER_COMPACT, // 20 + WLTOKEN_CONFIGFOLLOWER_ROLLFORWARD, // 21 + WLTOKEN_CONFIGFOLLOWER_GETCOMMITTEDVERSION, // 22 + WLTOKEN_PROCESS, // 23 + WLTOKEN_RESERVED_COUNT // 24 }; #endif diff --git a/fdbrpc/FlowTransport.actor.cpp b/fdbrpc/FlowTransport.actor.cpp index 1f9a1bbce4..3951fe3f69 100644 --- a/fdbrpc/FlowTransport.actor.cpp +++ b/fdbrpc/FlowTransport.actor.cpp @@ -235,6 +235,9 @@ struct PingReceiver final : NetworkMessageReceiver { }; struct TenantAuthorizer final : NetworkMessageReceiver { + TenantAuthorizer(EndpointMap& endpoints) { + endpoints.insertWellKnown(this, Endpoint::wellKnownToken(WLTOKEN_AUTH_TENANT), TaskPriority::ReadSocket); + } void receive(ArenaObjectReader& reader) override { AuthorizationRequest req; try { @@ -289,6 +292,7 @@ public: EndpointMap endpoints; EndpointNotFoundReceiver endpointNotFoundReceiver{ endpoints }; PingReceiver pingReceiver{ endpoints }; + TenantAuthorizer tenantReceiver{ endpoints }; Int64MetricHandle bytesSent; Int64MetricHandle countPacketsReceived; @@ -948,7 +952,7 @@ ACTOR static void deliver(TransportData* self, ArenaReader reader, NetworkAddress peerAddress, Reference authorizedTenants, - ContextVariableMap* cvm, + ContextVariableMap* cvm, bool inReadSocket) { // We want to run the task at the right priority. If the priority is higher than the current priority (which is // ReadSocket) we can just upgrade. Otherwise we'll context switch so that we don't block other tasks that might run @@ -1014,7 +1018,7 @@ static void scanPackets(TransportData* transport, Arena& arena, NetworkAddress const& peerAddress, Reference const& authorizedTenants, - ContextVariableMap* cvm, + ContextVariableMap* cvm, ProtocolVersion peerProtocolVersion) { // Find each complete packet in the given byte range and queue a ready task to deliver it. // Remove the complete packets from the range by increasing unprocessed_begin. @@ -1134,7 +1138,7 @@ static void scanPackets(TransportData* transport, std::move(reader), peerAddress, authorizedTenants, - cvm, + cvm, true); } @@ -1182,12 +1186,12 @@ ACTOR static Future connectionReader(TransportData* transport, state NetworkAddress peerAddress; state ProtocolVersion peerProtocolVersion; state Reference authorizedTenants = makeReference(); + state ContextVariableMap cvm; + peerAddress = conn->getPeerAddress(); authorizedTenants->trusted = transport->allowList(conn->getPeerAddress().ip); - ContextVariableMap cvm; cvm["AuthorizedTenants"] = &authorizedTenants; cvm["PeerAddress"] = &peerAddress; - peerAddress = conn->getPeerAddress(); authorizedTenants->trusted = transport->allowList(peerAddress.ip); if (!peer) { ASSERT(!peerAddress.isPublic()); @@ -1342,7 +1346,7 @@ ACTOR static Future connectionReader(TransportData* transport, arena, peerAddress, authorizedTenants, - &cvm, + &cvm, peerProtocolVersion); } else { unprocessed_begin = unprocessed_end; @@ -1614,7 +1618,7 @@ static void sendLocal(TransportData* self, ISerializeSource const& what, const E ASSERT(copy.size() > 0); TaskPriority priority = self->endpoints.getPriority(destination.token); if (priority != TaskPriority::UnknownEndpoint || (destination.token.first() & TOKEN_STREAM_FLAG) != 0) { - Reference authorizedTenants; + Reference authorizedTenants = makeReference(); authorizedTenants->trusted = true; deliver(self, destination, @@ -1622,7 +1626,7 @@ static void sendLocal(TransportData* self, ISerializeSource const& what, const E ArenaReader(copy.arena(), copy, AssumeVersion(currentProtocolVersion)), NetworkAddress(), authorizedTenants, - &cvm, + &cvm, false); } } diff --git a/fdbrpc/FlowTransport.h b/fdbrpc/FlowTransport.h index 8aa2e90dfc..466ff7cfb6 100644 --- a/fdbrpc/FlowTransport.h +++ b/fdbrpc/FlowTransport.h @@ -31,7 +31,7 @@ #include "flow/Net2Packet.h" #include "fdbrpc/ContinuousSample.h" -enum { WLTOKEN_ENDPOINT_NOT_FOUND = 0, WLTOKEN_PING_PACKET, WLTOKEN_FIRST_AVAILABLE }; +enum { WLTOKEN_ENDPOINT_NOT_FOUND = 0, WLTOKEN_PING_PACKET, WLTOKEN_AUTH_TENANT, WLTOKEN_FIRST_AVAILABLE }; #pragma pack(push, 4) class Endpoint { diff --git a/fdbrpc/IPAllowList.cpp b/fdbrpc/IPAllowList.cpp index b018c696d5..e49e530f54 100644 --- a/fdbrpc/IPAllowList.cpp +++ b/fdbrpc/IPAllowList.cpp @@ -198,7 +198,7 @@ struct SubNetTest { return (val & subnetMask) ^ baseAddress; } - template + template static IPAddress randomAddress() { constexpr int width = V4 ? 4 : 16; uint32_t rnd[width / 4]; @@ -255,7 +255,7 @@ struct SubNetTest { if (subnet.addressMask.isV4()) { return randomAddress(inSubnet); } else { - return randomAddress(inSubnet); + return randomAddress(inSubnet); } } }; diff --git a/fdbrpc/fdbrpc.h b/fdbrpc/fdbrpc.h index d78816910a..4bbaccae5b 100644 --- a/fdbrpc/fdbrpc.h +++ b/fdbrpc/fdbrpc.h @@ -838,6 +838,11 @@ private: NetNotifiedQueue* queue; }; +template +using PrivateRequestStream = RequestStream; +template +using PublicRequestStream = RequestStream; + template void save(Ar& ar, const RequestStream& value) { auto const& ep = value.getEndpoint(); diff --git a/fdbrpc/simulator.h b/fdbrpc/simulator.h index e03f16cfef..bc2912f490 100644 --- a/fdbrpc/simulator.h +++ b/fdbrpc/simulator.h @@ -92,6 +92,7 @@ public: UID uid; ProtocolVersion protocolVersion; + bool excludeFromRestarts = false; ProcessInfo(const char* name, LocalityData locality, diff --git a/fdbserver/ApplyMetadataMutation.cpp b/fdbserver/ApplyMetadataMutation.cpp index 7b935adc5b..b08f347348 100644 --- a/fdbserver/ApplyMetadataMutation.cpp +++ b/fdbserver/ApplyMetadataMutation.cpp @@ -111,7 +111,7 @@ private: KeyRangeMap* keyInfo = nullptr; KeyRangeMap* cacheInfo = nullptr; std::map* uid_applyMutationsData = nullptr; - RequestStream commit = RequestStream(); + PublicRequestStream commit = PublicRequestStream(); Database cx = Database(); NotifiedVersion* committedVersion = nullptr; std::map>* storageCache = nullptr; diff --git a/fdbserver/CMakeLists.txt b/fdbserver/CMakeLists.txt index 88ff8f2b44..769b4d7d3f 100644 --- a/fdbserver/CMakeLists.txt +++ b/fdbserver/CMakeLists.txt @@ -225,6 +225,7 @@ set(FDBSERVER_SRCS workloads/MetricLogging.actor.cpp workloads/MiniCycle.actor.cpp workloads/MutationLogReaderCorrectness.actor.cpp + workloads/PrivateEndpoints.actor.cpp workloads/GetRangeAndMap.actor.cpp workloads/ParallelRestore.actor.cpp workloads/Performance.actor.cpp diff --git a/fdbserver/ClusterController.actor.cpp b/fdbserver/ClusterController.actor.cpp index 974343c3a5..fc54592cc8 100644 --- a/fdbserver/ClusterController.actor.cpp +++ b/fdbserver/ClusterController.actor.cpp @@ -2896,7 +2896,7 @@ TEST_CASE("/fdbserver/clustercontroller/shouldTriggerRecoveryDueToDegradedServer GrvProxyInterface proxyInterf; proxyInterf.getConsistentReadVersion = - RequestStream(Endpoint({ proxy }, testUID)); + PublicRequestStream(Endpoint({ proxy }, testUID)); testDbInfo.client.grvProxies.push_back(proxyInterf); ResolverInterface resolverInterf; @@ -3006,11 +3006,11 @@ TEST_CASE("/fdbserver/clustercontroller/shouldTriggerFailoverDueToDegradedServer GrvProxyInterface grvProxyInterf; grvProxyInterf.getConsistentReadVersion = - RequestStream(Endpoint({ proxy }, testUID)); + PublicRequestStream(Endpoint({ proxy }, testUID)); testDbInfo.client.grvProxies.push_back(grvProxyInterf); CommitProxyInterface commitProxyInterf; - commitProxyInterf.commit = RequestStream(Endpoint({ proxy2 }, testUID)); + commitProxyInterf.commit = PublicRequestStream(Endpoint({ proxy2 }, testUID)); testDbInfo.client.commitProxies.push_back(commitProxyInterf); ResolverInterface resolverInterf; diff --git a/fdbserver/GrvProxyServer.actor.cpp b/fdbserver/GrvProxyServer.actor.cpp index 3da481a947..9ca806c489 100644 --- a/fdbserver/GrvProxyServer.actor.cpp +++ b/fdbserver/GrvProxyServer.actor.cpp @@ -238,7 +238,7 @@ struct GrvProxyData { GrvProxyStats stats; MasterInterface master; - RequestStream getConsistentReadVersion; + PublicRequestStream getConsistentReadVersion; Reference logSystem; Database cx; @@ -270,7 +270,7 @@ struct GrvProxyData { GrvProxyData(UID dbgid, MasterInterface master, - RequestStream getConsistentReadVersion, + PublicRequestStream getConsistentReadVersion, Reference const> db) : dbgid(dbgid), stats(dbgid), master(master), getConsistentReadVersion(getConsistentReadVersion), cx(openDBOnServer(db, TaskPriority::DefaultEndpoint, LockAware::True)), db(db), lastStartCommit(0), diff --git a/fdbserver/ProxyCommitData.actor.h b/fdbserver/ProxyCommitData.actor.h index 089b8e74d3..8dedd024d9 100644 --- a/fdbserver/ProxyCommitData.actor.h +++ b/fdbserver/ProxyCommitData.actor.h @@ -193,8 +193,8 @@ struct ProxyCommitData { NotifiedVersion latestLocalCommitBatchResolving; NotifiedVersion latestLocalCommitBatchLogging; - RequestStream getConsistentReadVersion; - RequestStream commit; + PublicRequestStream getConsistentReadVersion; + PublicRequestStream commit; Database cx; Reference const> db; EventMetricHandle singleKeyMutationEvent; @@ -273,9 +273,9 @@ struct ProxyCommitData { ProxyCommitData(UID dbgid, MasterInterface master, - RequestStream getConsistentReadVersion, + PublicRequestStream getConsistentReadVersion, Version recoveryTransactionVersion, - RequestStream commit, + PublicRequestStream commit, Reference const> db, bool firstProxy) : dbgid(dbgid), commitBatchesMemBytesCount(0), diff --git a/fdbserver/SimulatedCluster.actor.cpp b/fdbserver/SimulatedCluster.actor.cpp index 4408fd500b..a5e0299b1c 100644 --- a/fdbserver/SimulatedCluster.actor.cpp +++ b/fdbserver/SimulatedCluster.actor.cpp @@ -26,6 +26,7 @@ #include #include "fdbrpc/Locality.h" #include "fdbrpc/simulator.h" +#include "fdbrpc/IPAllowList.h" #include "fdbclient/ClusterConnectionFile.h" #include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbclient/DatabaseContext.h" @@ -488,6 +489,9 @@ ACTOR Future simulatedFDBDRebooter(ReferencerandomUniqueID(); state int cycles = 0; + state IPAllowList allowList; + + allowList.addTrustedSubnet("0.0.0.0/2"sv); loop { auto waitTime = @@ -547,7 +551,8 @@ ACTOR Future simulatedFDBDRebooter(Reference> futures; @@ -2265,10 +2270,14 @@ ACTOR void setupAndRun(std::string dataFolder, state Standalone startingConfiguration; state int testerCount = 1; state TestConfig testConfig; + state IPAllowList allowList; testConfig.readFromConfig(testFile); g_simulator.hasDiffProtocolProcess = testConfig.startIncompatibleProcess; g_simulator.setDiffProtocol = false; + // Build simulator allow list + allowList.addTrustedSubnet("0.0.0.0/2"sv); + // The RocksDB storage engine does not support the restarting tests because you cannot consistently get a clean // snapshot of the storage engine without a snapshotting file system. // https://github.com/apple/foundationdb/issues/5155 @@ -2298,7 +2307,7 @@ ACTOR void setupAndRun(std::string dataFolder, } // TODO (IPv6) Use IPv6? - wait(g_simulator.onProcess( + auto testSystem = g_simulator.newProcess("TestSystem", IPAddress(0x01010101), 1, @@ -2311,10 +2320,11 @@ ACTOR void setupAndRun(std::string dataFolder, ProcessClass(ProcessClass::TesterClass, ProcessClass::CommandLineSource), "", "", - currentProtocolVersion), - TaskPriority::DefaultYield)); + currentProtocolVersion); + testSystem->excludeFromRestarts = true; + wait(g_simulator.onProcess(testSystem, TaskPriority::DefaultYield)); Sim2FileSystem::newFileSystem(); - FlowTransport::createInstance(true, 1, WLTOKEN_RESERVED_COUNT); + FlowTransport::createInstance(true, 1, WLTOKEN_RESERVED_COUNT, &allowList); TEST(true); // Simulation start try { diff --git a/fdbserver/worker.actor.cpp b/fdbserver/worker.actor.cpp index b0f94cdc21..205f8fa8bf 100644 --- a/fdbserver/worker.actor.cpp +++ b/fdbserver/worker.actor.cpp @@ -751,14 +751,14 @@ TEST_CASE("/fdbserver/worker/addressInDbAndPrimaryDc") { NetworkAddress grvProxyAddress(IPAddress(0x26262626), 1); GrvProxyInterface grvProxyInterf; grvProxyInterf.getConsistentReadVersion = - RequestStream(Endpoint({ grvProxyAddress }, UID(1, 2))); + PublicRequestStream(Endpoint({ grvProxyAddress }, UID(1, 2))); testDbInfo.client.grvProxies.push_back(grvProxyInterf); ASSERT(addressInDbAndPrimaryDc(grvProxyAddress, makeReference>(testDbInfo))); NetworkAddress commitProxyAddress(IPAddress(0x37373737), 1); CommitProxyInterface commitProxyInterf; commitProxyInterf.commit = - RequestStream(Endpoint({ commitProxyAddress }, UID(1, 2))); + PublicRequestStream(Endpoint({ commitProxyAddress }, UID(1, 2))); testDbInfo.client.commitProxies.push_back(commitProxyInterf); ASSERT(addressInDbAndPrimaryDc(commitProxyAddress, makeReference>(testDbInfo))); diff --git a/fdbserver/workloads/ClientWorkload.actor.cpp b/fdbserver/workloads/ClientWorkload.actor.cpp index 44ef221eaf..c93ea6474d 100644 --- a/fdbserver/workloads/ClientWorkload.actor.cpp +++ b/fdbserver/workloads/ClientWorkload.actor.cpp @@ -35,6 +35,7 @@ struct ClientWorkloadImpl { std::string processName; Database cx; Future databaseOpened; + std::string dataFolder; ClientWorkloadImpl(Reference const& child) : child(child) { if (self->address.isV6()) { @@ -45,16 +46,21 @@ struct ClientWorkloadImpl { } TraceEvent("TestClientStart", id).detail("ClusterFileLocation", child->ccr->getLocation()).log(); processName = fmt::format("TestClient{}", child->clientId); + Standalone newZoneId(deterministicRandom()->randomUniqueID().toString()); + auto locality = LocalityData(Optional>(), newZoneId, newZoneId, self->locality.dcId()); + dataFolder = joinPath(popPath(self->dataFolder), deterministicRandom()->randomUniqueID().toString()); + platform::createDirectory(dataFolder); childProcess = g_simulator.newProcess(processName.c_str(), childAddress, - 0, + 1, self->address.isTLS(), 1, - self->locality, + locality, ProcessClass(ProcessClass::TesterClass, ProcessClass::AutoSource), - self->dataFolder, + dataFolder.c_str(), self->coordinationFolder, self->protocolVersion); + childProcess->excludeFromRestarts = true; databaseOpened = openDatabase(this); } diff --git a/fdbserver/workloads/PrivateEndpoints.actor.cpp b/fdbserver/workloads/PrivateEndpoints.actor.cpp new file mode 100644 index 0000000000..e0353215b5 --- /dev/null +++ b/fdbserver/workloads/PrivateEndpoints.actor.cpp @@ -0,0 +1,127 @@ +/* + * PrivateEndpoints.actor.cpp + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2022 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "fdbserver/workloads/workloads.actor.h" + +#include "flow/actorcompiler.h" // has to be last include + +namespace { + +struct PrivateEndpoints : TestWorkload { + static constexpr const char* WorkloadName = "PrivateEndpoints"; + bool success = true; + int numSuccesses = 0; + double startAfter; + double runFor; + + std::vector(Reference> const&)>> testFunctions; + + template + static Optional getRandom(std::vector const& v) { + if (v.empty()) { + return Optional(); + } else { + return deterministicRandom()->randomChoice(v); + } + } + + template + static Optional getInterface(Reference> const& clientDBInfo) { + if constexpr (std::is_same_v) { + return getRandom(clientDBInfo->get().grvProxies); + } else if constexpr (std::is_same_v) { + return getRandom(clientDBInfo->get().commitProxies); + } else { + ASSERT(false); // don't know how to handle this type + } + } + + ACTOR template + static Future assumeFailure(Future f) { + try { + T t = wait(f); + (void)t; + ASSERT(false); + return Void(); + } catch (Error& e) { + if (e.code() == error_code_actor_cancelled) { + throw; + } else if (e.code() == error_code_unauthorized_attempt) { + return Void(); + } else { + TraceEvent(SevError, "WrongErrorCode").error(e); + return Void(); + } + } + } + + template + void addTestFor(RequestStream I::*channel) { + testFunctions.push_back([channel](Reference> const& clientDBInfo) { + auto optintf = getInterface(clientDBInfo); + if (!optintf.present()) { + return clientDBInfo->onChange(); + } + RequestStream s = optintf.get().*channel; + RT req; + return assumeFailure(s.getReply(req)); + }); + } + + explicit PrivateEndpoints(WorkloadContext const& wcx) : TestWorkload(wcx) { + startAfter = getOption(options, "startAfter"_sr, 10.0); + runFor = getOption(options, "runFor"_sr, 10.0); + addTestFor(&GrvProxyInterface::waitFailure); + addTestFor(&GrvProxyInterface::getHealthMetrics); + addTestFor(&CommitProxyInterface::getStorageServerRejoinInfo); + addTestFor(&CommitProxyInterface::waitFailure); + addTestFor(&CommitProxyInterface::txnState); + addTestFor(&CommitProxyInterface::getHealthMetrics); + addTestFor(&CommitProxyInterface::proxySnapReq); + addTestFor(&CommitProxyInterface::exclusionSafetyCheckReq); + addTestFor(&CommitProxyInterface::getDDMetrics); + } + std::string description() const override { return WorkloadName; } + Future start(Database const& cx) override { return _start(this, cx); } + Future check(Database const& cx) override { return success; } + void getMetrics(std::vector& m) override { + m.emplace_back("Successes", double(numSuccesses), Averaged::True); + } + + ACTOR static Future _start(PrivateEndpoints* self, Database cx) { + state Reference> clientInfo = cx->clientInfo; + state Future end; + wait(delay(self->startAfter)); + end = delay(self->runFor); + loop { + auto testFuture = deterministicRandom()->randomChoice(self->testFunctions)(cx->clientInfo); + choose { + when(wait(testFuture)) { ++self->numSuccesses; } + when(wait(end)) { return Void(); } + } + break; + } + return Void(); + } +}; + +} // namespace + +WorkloadFactory PrivateEndpointsFactory(PrivateEndpoints::WorkloadName, true); diff --git a/fdbserver/workloads/SaveAndKill.actor.cpp b/fdbserver/workloads/SaveAndKill.actor.cpp index 43d3e0d2b3..e804b2ba93 100644 --- a/fdbserver/workloads/SaveAndKill.actor.cpp +++ b/fdbserver/workloads/SaveAndKill.actor.cpp @@ -89,7 +89,7 @@ struct SaveAndKillWorkload : TestWorkload { ISimulator::ProcessInfo* process = processIterator->second; std::string machineId = printable(process->locality.machineId()); const char* machineIdString = machineId.c_str(); - if (strcmp(process->name, "TestSystem") != 0) { + if (!process->excludeFromRestarts) { if (machines.find(machineId) == machines.end()) { machines.insert(std::pair(machineId, 1)); ini.SetValue("META", format("%d", j).c_str(), machineIdString); diff --git a/flow/ObjectSerializer.h b/flow/ObjectSerializer.h index f8843df9a6..7e9a78b946 100644 --- a/flow/ObjectSerializer.h +++ b/flow/ObjectSerializer.h @@ -31,7 +31,7 @@ using ContextVariableMap = std::unordered_map; template struct LoadContext { Ar* ar; - ContextVariableMap* variables; + ContextVariableMap* variables = nullptr; LoadContext(Ar* ar, ContextVariableMap* variables = nullptr) : ar(ar), variables(variables) {} Arena& arena() { return ar->arena(); } @@ -92,16 +92,15 @@ struct SaveContext { template class _ObjectReader { protected: - ProtocolVersion mProtocolVersion; + Optional mProtocolVersion; + bool versionSet = false; LoadContext context; public: _ObjectReader() : context(static_cast(this)) {} - ProtocolVersion protocolVersion() const { return mProtocolVersion; } + ProtocolVersion protocolVersion() const { return mProtocolVersion.get(); } void setProtocolVersion(ProtocolVersion v) { mProtocolVersion = v; } - void setContextVariableMap(ContextVariableMap* cvm) { - context.variables = cvm; - } + void setContextVariableMap(ContextVariableMap* cvm) { context.variables = cvm; } template void deserialize(FileIdentifier file_identifier, Items&... items) { @@ -109,7 +108,7 @@ public: if (read_file_identifier(data) != file_identifier) { // Some file identifiers are changed in 7.0, so file identifier mismatches // are expected during a downgrade from 7.0 to 6.3 - bool expectMismatch = mProtocolVersion >= ProtocolVersion(0x0FDB00B070000000LL); + bool expectMismatch = mProtocolVersion.get() >= ProtocolVersion(0x0FDB00B070000000LL); { TraceEvent te(expectMismatch ? SevInfo : SevError, "MismatchedFileIdentifier"); if (expectMismatch) { diff --git a/tests/SpecificUnitTest.txt b/tests/SpecificUnitTest.txt index 5cd4fdc5d8..905c65fc98 100644 --- a/tests/SpecificUnitTest.txt +++ b/tests/SpecificUnitTest.txt @@ -3,5 +3,5 @@ startDelay=0 useDB=false testName=UnitTests - maxTestCases=0 - testsMatching=/status/json/builder + maxTestCases=1 + testsMatching=/authorization/key_sign_success diff --git a/tests/fast/CycleTest.toml b/tests/fast/CycleTest.toml index ad61effc4f..a97dfd9969 100644 --- a/tests/fast/CycleTest.toml +++ b/tests/fast/CycleTest.toml @@ -6,3 +6,35 @@ testTitle = 'Clogged' transactionsPerSecond = 2500.0 testDuration = 10.0 expectedRate = 0 + + [[test.workload]] + testName = 'RandomClogging' + testDuration = 10.0 + + [[test.workload]] + testName = 'Rollback' + meanDelay = 10.0 + testDuration = 10.0 + + [[test.workload]] + testName = 'Attrition' + machinesToKill = 10 + machinesToLeave = 3 + reboot = true + testDuration = 10.0 + + [[test.workload]] + testName = 'Attrition' + machinesToKill = 10 + machinesToLeave = 3 + reboot = true + testDuration = 10.0 + +[[test]] +testTitle = 'Unclogged' + + [[test.workload]] + testName = 'Cycle' + transactionsPerSecond = 250.0 + testDuration = 10.0 + expectedRate = 0.80 diff --git a/tests/fast/PrivateEndpoints.toml b/tests/fast/PrivateEndpoints.toml new file mode 100644 index 0000000000..67be2c6b01 --- /dev/null +++ b/tests/fast/PrivateEndpoints.toml @@ -0,0 +1,5 @@ +[[test]] +testTitle = 'PrivateEndpoints' + +[[test.workload]] +testName = 'PrivateEndpoints' From f2749cdde908f1a161754a82a7a7af61fab28095 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Tue, 15 Mar 2022 17:04:24 +0100 Subject: [PATCH 11/24] add test to cmake --- tests/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 48315c5f94..7f0d399c43 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -156,6 +156,7 @@ if(WITH_PYTHON) add_fdb_test(TEST_FILES fast/MoveKeysCycle.toml) add_fdb_test(TEST_FILES fast/MutationLogReaderCorrectness.toml) add_fdb_test(TEST_FILES fast/GetRangeAndMap.toml) + add_fdb_test(TEST_FILES fast/PrivateEndpoints.toml) add_fdb_test(TEST_FILES fast/ProtocolVersion.toml) add_fdb_test(TEST_FILES fast/RandomSelector.toml) add_fdb_test(TEST_FILES fast/RandomUnitTests.toml) From f346a7ea1cb0a8cf15015216d4f3d4025c4ed03a Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Tue, 15 Mar 2022 17:15:26 +0100 Subject: [PATCH 12/24] Fix Windows build --- fdbrpc/IPAllowList.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/fdbrpc/IPAllowList.cpp b/fdbrpc/IPAllowList.cpp index e49e530f54..3eb6078ffe 100644 --- a/fdbrpc/IPAllowList.cpp +++ b/fdbrpc/IPAllowList.cpp @@ -23,6 +23,7 @@ #include #include +#include namespace { From 806c9f5afb2f25c689b58233502f2d039260c15c Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Tue, 15 Mar 2022 17:28:36 +0100 Subject: [PATCH 13/24] fix test changes --- tests/CMakeLists.txt | 2 +- tests/SpecificUnitTest.txt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 7f0d399c43..62c6613f0e 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -155,7 +155,7 @@ if(WITH_PYTHON) add_fdb_test(TEST_FILES fast/MemoryLifetime.toml) add_fdb_test(TEST_FILES fast/MoveKeysCycle.toml) add_fdb_test(TEST_FILES fast/MutationLogReaderCorrectness.toml) - add_fdb_test(TEST_FILES fast/GetRangeAndMap.toml) + add_fdb_test(TEST_FILES fast/GetMappedRange.toml) add_fdb_test(TEST_FILES fast/PrivateEndpoints.toml) add_fdb_test(TEST_FILES fast/ProtocolVersion.toml) add_fdb_test(TEST_FILES fast/RandomSelector.toml) diff --git a/tests/SpecificUnitTest.txt b/tests/SpecificUnitTest.txt index 905c65fc98..5cd4fdc5d8 100644 --- a/tests/SpecificUnitTest.txt +++ b/tests/SpecificUnitTest.txt @@ -3,5 +3,5 @@ startDelay=0 useDB=false testName=UnitTests - maxTestCases=1 - testsMatching=/authorization/key_sign_success + maxTestCases=0 + testsMatching=/status/json/builder From cca795a396e38789172f3d24f2e40fd0b95b9f26 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Thu, 17 Mar 2022 13:07:23 +0100 Subject: [PATCH 14/24] fixed merge mistakes --- fdbclient/StorageServerInterface.h | 10 +++++----- fdbserver/CMakeLists.txt | 1 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/fdbclient/StorageServerInterface.h b/fdbclient/StorageServerInterface.h index 1e3cc62231..49dd404761 100644 --- a/fdbclient/StorageServerInterface.h +++ b/fdbclient/StorageServerInterface.h @@ -68,7 +68,7 @@ struct StorageServerInterface { // Throws a wrong_shard_server if the keys in the request or result depend on data outside this server OR if a large // selector offset prevents all data from being read in one range read PublicRequestStream getKeyValues; - PublicRequestStream getKeyValuesAndFlatMap; + PublicRequestStream getMappedKeyValues; RequestStream getShardState; RequestStream waitMetrics; @@ -132,7 +132,7 @@ struct StorageServerInterface { RequestStream(getValue.getEndpoint().getAdjustedEndpoint(12)); getKeyValuesStream = PublicRequestStream( getValue.getEndpoint().getAdjustedEndpoint(13)); - getKeyValuesAndFlatMap = PublicRequestStream( + getMappedKeyValues = PublicRequestStream( getValue.getEndpoint().getAdjustedEndpoint(14)); changeFeedStream = PublicRequestStream(getValue.getEndpoint().getAdjustedEndpoint(15)); @@ -142,9 +142,10 @@ struct StorageServerInterface { PublicRequestStream(getValue.getEndpoint().getAdjustedEndpoint(17)); changeFeedVersionUpdate = PublicRequestStream( getValue.getEndpoint().getAdjustedEndpoint(18)); - checkpoint = RequestStream(getValue.getEndpoint().getAdjustedEndpoint(19)); + checkpoint = + PublicRequestStream(getValue.getEndpoint().getAdjustedEndpoint(19)); fetchCheckpoint = - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(20)); + PublicRequestStream(getValue.getEndpoint().getAdjustedEndpoint(20)); } } else { ASSERT(Ar::isDeserializing); @@ -187,7 +188,6 @@ struct StorageServerInterface { streams.push_back(getReadHotRanges.getReceiver()); streams.push_back(getRangeSplitPoints.getReceiver()); streams.push_back(getKeyValuesStream.getReceiver(TaskPriority::LoadBalancedEndpoint)); - streams.push_back(getMappedKeyValues.getReceiver(TaskPriority::LoadBalancedEndpoint)); streams.push_back(changeFeedStream.getReceiver()); streams.push_back(overlappingChangeFeeds.getReceiver()); streams.push_back(changeFeedPop.getReceiver()); diff --git a/fdbserver/CMakeLists.txt b/fdbserver/CMakeLists.txt index ae1ad516e9..3d11057800 100644 --- a/fdbserver/CMakeLists.txt +++ b/fdbserver/CMakeLists.txt @@ -233,7 +233,6 @@ set(FDBSERVER_SRCS workloads/MiniCycle.actor.cpp workloads/MutationLogReaderCorrectness.actor.cpp workloads/PrivateEndpoints.actor.cpp - workloads/GetRangeAndMap.actor.cpp workloads/ParallelRestore.actor.cpp workloads/Performance.actor.cpp workloads/Ping.actor.cpp From 158da462f7d0fa9ed8b4650682ff789ae78c6634 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Thu, 17 Mar 2022 14:37:05 +0100 Subject: [PATCH 15/24] fix memory bug --- fdbrpc/FlowTransport.actor.cpp | 16 ++++++++-------- flow/ObjectSerializer.h | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/fdbrpc/FlowTransport.actor.cpp b/fdbrpc/FlowTransport.actor.cpp index 7032ab9b9b..caba4077dc 100644 --- a/fdbrpc/FlowTransport.actor.cpp +++ b/fdbrpc/FlowTransport.actor.cpp @@ -308,6 +308,7 @@ public: double lastIncompatibleMessage; uint64_t transportId; IPAllowList allowList; + std::shared_ptr localCVM = std::make_shared(); // for local delivery Future multiVersionCleanup; Future pingLogger; @@ -952,7 +953,7 @@ ACTOR static void deliver(TransportData* self, ArenaReader reader, NetworkAddress peerAddress, Reference authorizedTenants, - ContextVariableMap* cvm, + std::shared_ptr cvm, bool inReadSocket) { // We want to run the task at the right priority. If the priority is higher than the current priority (which is // ReadSocket) we can just upgrade. Otherwise we'll context switch so that we don't block other tasks that might run @@ -1018,7 +1019,7 @@ static void scanPackets(TransportData* transport, Arena& arena, NetworkAddress const& peerAddress, Reference const& authorizedTenants, - ContextVariableMap* cvm, + std::shared_ptr cvm, ProtocolVersion peerProtocolVersion) { // Find each complete packet in the given byte range and queue a ready task to deliver it. // Remove the complete packets from the range by increasing unprocessed_begin. @@ -1186,11 +1187,11 @@ ACTOR static Future connectionReader(TransportData* transport, state NetworkAddress peerAddress; state ProtocolVersion peerProtocolVersion; state Reference authorizedTenants = makeReference(); - state ContextVariableMap cvm; + state std::shared_ptr cvm = std::make_shared(); peerAddress = conn->getPeerAddress(); authorizedTenants->trusted = transport->allowList(conn->getPeerAddress().ip); - cvm["AuthorizedTenants"] = &authorizedTenants; - cvm["PeerAddress"] = &peerAddress; + (*cvm)["AuthorizedTenants"] = &authorizedTenants; + (*cvm)["PeerAddress"] = &peerAddress; authorizedTenants->trusted = transport->allowList(peerAddress.ip); if (!peer) { @@ -1346,7 +1347,7 @@ ACTOR static Future connectionReader(TransportData* transport, arena, peerAddress, authorizedTenants, - &cvm, + cvm, peerProtocolVersion); } else { unprocessed_begin = unprocessed_end; @@ -1607,7 +1608,6 @@ static void sendLocal(TransportData* self, ISerializeSource const& what, const E // SOMEDAY: Would it be better to avoid (de)serialization by doing this check in flow? Standalone copy; - ContextVariableMap cvm; ObjectWriter wr(AssumeVersion(g_network->protocolVersion())); what.serializeObjectWriter(wr); copy = wr.toStringRef(); @@ -1626,7 +1626,7 @@ static void sendLocal(TransportData* self, ISerializeSource const& what, const E ArenaReader(copy.arena(), copy, AssumeVersion(currentProtocolVersion)), NetworkAddress(), authorizedTenants, - &cvm, + self->localCVM, false); } } diff --git a/flow/ObjectSerializer.h b/flow/ObjectSerializer.h index 7e9a78b946..c320aeb1a3 100644 --- a/flow/ObjectSerializer.h +++ b/flow/ObjectSerializer.h @@ -31,7 +31,7 @@ using ContextVariableMap = std::unordered_map; template struct LoadContext { Ar* ar; - ContextVariableMap* variables = nullptr; + std::shared_ptr variables = nullptr; LoadContext(Ar* ar, ContextVariableMap* variables = nullptr) : ar(ar), variables(variables) {} Arena& arena() { return ar->arena(); } @@ -100,7 +100,7 @@ public: _ObjectReader() : context(static_cast(this)) {} ProtocolVersion protocolVersion() const { return mProtocolVersion.get(); } void setProtocolVersion(ProtocolVersion v) { mProtocolVersion = v; } - void setContextVariableMap(ContextVariableMap* cvm) { context.variables = cvm; } + void setContextVariableMap(std::shared_ptr cvm) { context.variables = cvm; } template void deserialize(FileIdentifier file_identifier, Items&... items) { From 1fbeca8038c4176572be804a458f129bfecd736f Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Sat, 19 Mar 2022 11:03:32 +0100 Subject: [PATCH 16/24] fix memory issue --- fdbrpc/FlowTransport.actor.cpp | 9 ++++---- fdbserver/CMakeLists.txt | 1 + flow/ObjectSerializer.h | 41 +++++++++++----------------------- 3 files changed, 18 insertions(+), 33 deletions(-) diff --git a/fdbrpc/FlowTransport.actor.cpp b/fdbrpc/FlowTransport.actor.cpp index caba4077dc..175688f35c 100644 --- a/fdbrpc/FlowTransport.actor.cpp +++ b/fdbrpc/FlowTransport.actor.cpp @@ -967,11 +967,7 @@ ACTOR static void deliver(TransportData* self, } auto receiver = self->endpoints.get(destination.token); - if (receiver) { - if (!authorizedTenants->trusted && !receiver->isPublic()) { - TraceEvent(SevWarnAlways, "AttemptedRPCToPrivatePrevented").detail("From", peerAddress); - throw connection_failed(); - } + if (receiver && (authorizedTenants->trusted || receiver->isPublic())) { if (!checkCompatible(receiver->peerCompatibilityPolicy(), reader.protocolVersion())) { return; } @@ -996,6 +992,9 @@ ACTOR static void deliver(TransportData* self, } } else if (destination.token.first() & TOKEN_STREAM_FLAG) { // We don't have the (stream) endpoint 'token', notify the remote machine + if (receiver) { + TraceEvent(SevWarnAlways, "AttemptedRPCToPrivatePrevented").detail("From", peerAddress); + } if (destination.token.first() != -1) { if (self->isLocalAddress(destination.getPrimaryAddress())) { sendLocal(self, diff --git a/fdbserver/CMakeLists.txt b/fdbserver/CMakeLists.txt index 3d11057800..912137a015 100644 --- a/fdbserver/CMakeLists.txt +++ b/fdbserver/CMakeLists.txt @@ -233,6 +233,7 @@ set(FDBSERVER_SRCS workloads/MiniCycle.actor.cpp workloads/MutationLogReaderCorrectness.actor.cpp workloads/PrivateEndpoints.actor.cpp + workloads/GetRangeStream.actor.cpp workloads/ParallelRestore.actor.cpp workloads/Performance.actor.cpp workloads/Ping.actor.cpp diff --git a/flow/ObjectSerializer.h b/flow/ObjectSerializer.h index c320aeb1a3..2e9f6c064d 100644 --- a/flow/ObjectSerializer.h +++ b/flow/ObjectSerializer.h @@ -31,9 +31,9 @@ using ContextVariableMap = std::unordered_map; template struct LoadContext { Ar* ar; - std::shared_ptr variables = nullptr; - LoadContext(Ar* ar, ContextVariableMap* variables = nullptr) : ar(ar), variables(variables) {} + LoadContext(Ar* ar) : ar(ar) {} + Arena& arena() { return ar->arena(); } ProtocolVersion protocolVersion() const { return ar->protocolVersion(); } @@ -53,24 +53,6 @@ struct LoadContext { void addArena(Arena& arena) { arena = ar->arena(); } LoadContext& context() { return *this; } - - template - bool variable(std::string_view name, T* val) { - auto p = variables->insert(std::make_pair(name, val)); - return p.second; - } - - template - T& variable(std::string_view name) { - auto res = variables->at(name); - return *reinterpret_cast(res); - } - - template - T const& variable(std::string_view name) const { - auto res = variables->at(name); - return *reinterpret_cast(res); - } }; template @@ -93,22 +75,22 @@ template class _ObjectReader { protected: Optional mProtocolVersion; - bool versionSet = false; - LoadContext context; + std::shared_ptr variables; public: - _ObjectReader() : context(static_cast(this)) {} ProtocolVersion protocolVersion() const { return mProtocolVersion.get(); } void setProtocolVersion(ProtocolVersion v) { mProtocolVersion = v; } - void setContextVariableMap(std::shared_ptr cvm) { context.variables = cvm; } + void setContextVariableMap(std::shared_ptr const& cvm) { variables = cvm; } template void deserialize(FileIdentifier file_identifier, Items&... items) { + LoadContext context(static_cast(this)); const uint8_t* data = static_cast(this)->data(); if (read_file_identifier(data) != file_identifier) { // Some file identifiers are changed in 7.0, so file identifier mismatches // are expected during a downgrade from 7.0 to 6.3 - bool expectMismatch = mProtocolVersion.get() >= ProtocolVersion(0x0FDB00B070000000LL); + bool expectMismatch = mProtocolVersion.get() >= ProtocolVersion(0x0FDB00B070000000LL) && + currentProtocolVersion < ProtocolVersion(0x0FDB00B070000000LL); { TraceEvent te(expectMismatch ? SevInfo : SevError, "MismatchedFileIdentifier"); if (expectMismatch) { @@ -130,17 +112,20 @@ public: template bool variable(std::string_view name, T* val) { - return context.template variable(name, val); + auto p = variables->insert(std::make_pair(name, val)); + return p.second; } template T& variable(std::string_view name) { - return context.template variable(name); + auto res = variables->at(name); + return *reinterpret_cast(res); } template T const& variable(std::string_view name) const { - return context.template variable(name); + auto res = variables->at(name); + return *reinterpret_cast(res); } }; From b595d4462f5eaae5c46e77683e7c994bf2fefbd3 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Tue, 29 Mar 2022 14:58:09 -0600 Subject: [PATCH 17/24] Throw error on unauthorized access --- fdbclient/StorageServerInterface.h | 1 + fdbclient/WellKnownEndpoints.h | 40 +++++++-------- fdbrpc/FailureMonitor.actor.cpp | 18 ++++++- fdbrpc/FailureMonitor.h | 11 +++- fdbrpc/FlowTransport.actor.cpp | 50 ++++++++++++++----- fdbrpc/FlowTransport.h | 8 ++- fdbrpc/fdbrpc.h | 6 +++ fdbrpc/genericactors.actor.h | 34 +++++++++++-- .../workloads/PrivateEndpoints.actor.cpp | 6 +-- flow/error_definitions.h | 6 +-- 10 files changed, 132 insertions(+), 48 deletions(-) diff --git a/fdbclient/StorageServerInterface.h b/fdbclient/StorageServerInterface.h index 49dd404761..8bcafba7b6 100644 --- a/fdbclient/StorageServerInterface.h +++ b/fdbclient/StorageServerInterface.h @@ -188,6 +188,7 @@ struct StorageServerInterface { streams.push_back(getReadHotRanges.getReceiver()); streams.push_back(getRangeSplitPoints.getReceiver()); streams.push_back(getKeyValuesStream.getReceiver(TaskPriority::LoadBalancedEndpoint)); + streams.push_back(getMappedKeyValues.getReceiver(TaskPriority::LoadBalancedEndpoint)); streams.push_back(changeFeedStream.getReceiver()); streams.push_back(overlappingChangeFeeds.getReceiver()); streams.push_back(changeFeedPop.getReceiver()); diff --git a/fdbclient/WellKnownEndpoints.h b/fdbclient/WellKnownEndpoints.h index 79c882aefa..ab220060e3 100644 --- a/fdbclient/WellKnownEndpoints.h +++ b/fdbclient/WellKnownEndpoints.h @@ -29,27 +29,27 @@ */ enum WellKnownEndpoints { WLTOKEN_CLIENTLEADERREG_GETLEADER = WLTOKEN_FIRST_AVAILABLE, // 2 - WLTOKEN_CLIENTLEADERREG_OPENDATABASE, // 4 - WLTOKEN_LEADERELECTIONREG_CANDIDACY, // 5 - WLTOKEN_LEADERELECTIONREG_ELECTIONRESULT, // 6 - WLTOKEN_LEADERELECTIONREG_LEADERHEARTBEAT, // 7 - WLTOKEN_LEADERELECTIONREG_FORWARD, // 8 - WLTOKEN_GENERATIONREG_READ, // 9 + WLTOKEN_CLIENTLEADERREG_OPENDATABASE, // 5 + WLTOKEN_LEADERELECTIONREG_CANDIDACY, // 6 + WLTOKEN_LEADERELECTIONREG_ELECTIONRESULT, // 7 + WLTOKEN_LEADERELECTIONREG_LEADERHEARTBEAT, // 8 + WLTOKEN_LEADERELECTIONREG_FORWARD, // 9 WLTOKEN_PROTOCOL_INFO, // 10 : the value of this endpoint should be stable and not change. - WLTOKEN_GENERATIONREG_WRITE, // 11 - WLTOKEN_CLIENTLEADERREG_DESCRIPTOR_MUTABLE, // 12 - WLTOKEN_CONFIGTXN_GETGENERATION, // 13 - WLTOKEN_CONFIGTXN_GET, // 14 - WLTOKEN_CONFIGTXN_GETCLASSES, // 15 - WLTOKEN_CONFIGTXN_GETKNOBS, // 16 - WLTOKEN_CONFIGTXN_COMMIT, // 17 - WLTOKEN_CONFIGFOLLOWER_GETSNAPSHOTANDCHANGES, // 18 - WLTOKEN_CONFIGFOLLOWER_GETCHANGES, // 19 - WLTOKEN_CONFIGFOLLOWER_COMPACT, // 20 - WLTOKEN_CONFIGFOLLOWER_ROLLFORWARD, // 21 - WLTOKEN_CONFIGFOLLOWER_GETCOMMITTEDVERSION, // 22 - WLTOKEN_PROCESS, // 23 - WLTOKEN_RESERVED_COUNT // 24 + WLTOKEN_GENERATIONREG_READ, // 11 + WLTOKEN_GENERATIONREG_WRITE, // 12 + WLTOKEN_CLIENTLEADERREG_DESCRIPTOR_MUTABLE, // 13 + WLTOKEN_CONFIGTXN_GETGENERATION, // 14 + WLTOKEN_CONFIGTXN_GET, // 15 + WLTOKEN_CONFIGTXN_GETCLASSES, // 16 + WLTOKEN_CONFIGTXN_GETKNOBS, // 17 + WLTOKEN_CONFIGTXN_COMMIT, // 18 + WLTOKEN_CONFIGFOLLOWER_GETSNAPSHOTANDCHANGES, // 19 + WLTOKEN_CONFIGFOLLOWER_GETCHANGES, // 20 + WLTOKEN_CONFIGFOLLOWER_COMPACT, // 21 + WLTOKEN_CONFIGFOLLOWER_ROLLFORWARD, // 22 + WLTOKEN_CONFIGFOLLOWER_GETCOMMITTEDVERSION, // 23 + WLTOKEN_PROCESS, // 24 + WLTOKEN_RESERVED_COUNT // 25 }; #endif diff --git a/fdbrpc/FailureMonitor.actor.cpp b/fdbrpc/FailureMonitor.actor.cpp index 31ad94d4f9..807c071b09 100644 --- a/fdbrpc/FailureMonitor.actor.cpp +++ b/fdbrpc/FailureMonitor.actor.cpp @@ -131,11 +131,20 @@ void SimpleFailureMonitor::endpointNotFound(Endpoint const& endpoint) { TraceEvent(SevWarnAlways, "TooManyFailedEndpoints").suppressFor(1.0); failedEndpoints.clear(); } - failedEndpoints.insert(endpoint); + failedEndpoints.emplace(endpoint, FailedReason::NOT_FOUND); } endpointKnownFailed.trigger(endpoint); } +void SimpleFailureMonitor::unauthorizedEndpoint(Endpoint const& endpoint) { + TraceEvent(g_network->isSimulated() ? SevWarnAlways : SevError, "TriedAccessPrivateEndpoint") + .suppressFor(1.0) + .detail("Address", endpoint.getPrimaryAddress()) + .detail("Token", endpoint.token); + failedEndpoints.emplace(endpoint, FailedReason::UNAUTHORIZED); + endpointKnownFailed.trigger(endpoint); +} + void SimpleFailureMonitor::notifyDisconnect(NetworkAddress const& address) { //TraceEvent("NotifyDisconnect").detail("Address", address); endpointKnownFailed.triggerRange(Endpoint({ address }, UID()), Endpoint({ address }, UID(-1, -1))); @@ -208,8 +217,13 @@ bool SimpleFailureMonitor::permanentlyFailed(Endpoint const& endpoint) const { return failedEndpoints.count(endpoint); } +bool SimpleFailureMonitor::knownUnauthorized(Endpoint const& endpoint) const { + auto iter = failedEndpoints.find(endpoint); + return iter != failedEndpoints.end() && iter->second == FailedReason::UNAUTHORIZED; +} + void SimpleFailureMonitor::reset() { addressStatus = std::unordered_map(); - failedEndpoints = std::unordered_set(); + failedEndpoints = std::unordered_map(); endpointKnownFailed.resetNoWaiting(); } diff --git a/fdbrpc/FailureMonitor.h b/fdbrpc/FailureMonitor.h index 795a6e9953..c5c22f415e 100644 --- a/fdbrpc/FailureMonitor.h +++ b/fdbrpc/FailureMonitor.h @@ -93,6 +93,9 @@ public: // Only use this function when the endpoint is known to be failed virtual void endpointNotFound(Endpoint const&) = 0; + // Inform client that it was trying to send a message to a private endpoint + virtual void unauthorizedEndpoint(Endpoint const&) = 0; + // The next time the known status for the endpoint changes, returns the new status. virtual Future onStateChanged(Endpoint const& endpoint) = 0; @@ -108,6 +111,9 @@ public: // Returns true if the endpoint will never become available. virtual bool permanentlyFailed(Endpoint const& endpoint) const = 0; + // Returns true if we known we're not allowed to send messages to the remote endpoint + virtual bool knownUnauthorized(Endpoint const&) const = 0; + // Called by FlowTransport when a connection closes and a prior request or reply might be lost virtual void notifyDisconnect(NetworkAddress const&) = 0; @@ -139,9 +145,11 @@ public: class SimpleFailureMonitor : public IFailureMonitor { public: + enum class FailedReason { NOT_FOUND, UNAUTHORIZED }; SimpleFailureMonitor(); void setStatus(NetworkAddress const& address, FailureStatus const& status) override; void endpointNotFound(Endpoint const&) override; + void unauthorizedEndpoint(Endpoint const&) override; void notifyDisconnect(NetworkAddress const&) override; Future onStateChanged(Endpoint const& endpoint) override; @@ -151,6 +159,7 @@ public: Future onDisconnect(NetworkAddress const& address) override; bool onlyEndpointFailed(Endpoint const& endpoint) const override; bool permanentlyFailed(Endpoint const& endpoint) const override; + bool knownUnauthorized(Endpoint const&) const override; void reset(); @@ -158,7 +167,7 @@ private: std::unordered_map addressStatus; YieldedAsyncMap endpointKnownFailed; AsyncMap disconnectTriggers; - std::unordered_set failedEndpoints; + std::unordered_map failedEndpoints; friend class OnStateChangedActorActor; }; diff --git a/fdbrpc/FlowTransport.actor.cpp b/fdbrpc/FlowTransport.actor.cpp index 175688f35c..6af1786684 100644 --- a/fdbrpc/FlowTransport.actor.cpp +++ b/fdbrpc/FlowTransport.actor.cpp @@ -259,6 +259,21 @@ struct TenantAuthorizer final : NetworkMessageReceiver { bool isPublic() const override { return true; } }; +struct UnauthorizedEndpointReceiver final : NetworkMessageReceiver { + UnauthorizedEndpointReceiver(EndpointMap& endpoints) { + endpoints.insertWellKnown( + this, Endpoint::wellKnownToken(WLTOKEN_UNAUTHORIZED_ENDPOINT), TaskPriority::ReadSocket); + } + + void receive(ArenaObjectReader& reader) override { + UID token; + reader.deserialize(token); + Endpoint e = FlowTransport::transport().loadedEndpoint(token); + IFailureMonitor::failureMonitor().unauthorizedEndpoint(e); + } + bool isPublic() const override { return true; } +}; + class TransportData { public: TransportData(uint64_t transportId, int maxWellKnownEndpoints, IPAllowList const* allowList); @@ -293,6 +308,7 @@ public: EndpointNotFoundReceiver endpointNotFoundReceiver{ endpoints }; PingReceiver pingReceiver{ endpoints }; TenantAuthorizer tenantReceiver{ endpoints }; + UnauthorizedEndpointReceiver unauthorizedEndpointReceiver{ endpoints }; Int64MetricHandle bytesSent; Int64MetricHandle countPacketsReceived; @@ -994,19 +1010,27 @@ ACTOR static void deliver(TransportData* self, // We don't have the (stream) endpoint 'token', notify the remote machine if (receiver) { TraceEvent(SevWarnAlways, "AttemptedRPCToPrivatePrevented").detail("From", peerAddress); - } - if (destination.token.first() != -1) { - if (self->isLocalAddress(destination.getPrimaryAddress())) { - sendLocal(self, - SerializeSource(destination.token), - Endpoint::wellKnown(destination.addresses, WLTOKEN_ENDPOINT_NOT_FOUND)); - } else { - Reference peer = self->getOrOpenPeer(destination.getPrimaryAddress()); - sendPacket(self, - peer, - SerializeSource(destination.token), - Endpoint::wellKnown(destination.addresses, WLTOKEN_ENDPOINT_NOT_FOUND), - false); + ASSERT(!self->isLocalAddress(destination.getPrimaryAddress())); + Reference peer = self->getOrOpenPeer(destination.getPrimaryAddress()); + sendPacket(self, + peer, + SerializeSource(destination.token), + Endpoint::wellKnown(destination.addresses, WLTOKEN_UNAUTHORIZED_ENDPOINT), + false); + } else { + if (destination.token.first() != -1) { + if (self->isLocalAddress(destination.getPrimaryAddress())) { + sendLocal(self, + SerializeSource(destination.token), + Endpoint::wellKnown(destination.addresses, WLTOKEN_ENDPOINT_NOT_FOUND)); + } else { + Reference peer = self->getOrOpenPeer(destination.getPrimaryAddress()); + sendPacket(self, + peer, + SerializeSource(destination.token), + Endpoint::wellKnown(destination.addresses, WLTOKEN_ENDPOINT_NOT_FOUND), + false); + } } } } diff --git a/fdbrpc/FlowTransport.h b/fdbrpc/FlowTransport.h index 466ff7cfb6..5ee4762195 100644 --- a/fdbrpc/FlowTransport.h +++ b/fdbrpc/FlowTransport.h @@ -31,7 +31,13 @@ #include "flow/Net2Packet.h" #include "fdbrpc/ContinuousSample.h" -enum { WLTOKEN_ENDPOINT_NOT_FOUND = 0, WLTOKEN_PING_PACKET, WLTOKEN_AUTH_TENANT, WLTOKEN_FIRST_AVAILABLE }; +enum { + WLTOKEN_ENDPOINT_NOT_FOUND = 0, + WLTOKEN_PING_PACKET, + WLTOKEN_AUTH_TENANT, + WLTOKEN_UNAUTHORIZED_ENDPOINT, + WLTOKEN_FIRST_AVAILABLE +}; #pragma pack(push, 4) class Endpoint { diff --git a/fdbrpc/fdbrpc.h b/fdbrpc/fdbrpc.h index a0bb4d4bb8..340ec657fd 100644 --- a/fdbrpc/fdbrpc.h +++ b/fdbrpc/fdbrpc.h @@ -693,6 +693,9 @@ public: Future disc = makeDependent(IFailureMonitor::failureMonitor()).onDisconnectOrFailure(getEndpoint(taskID)); if (disc.isReady()) { + if (IFailureMonitor::failureMonitor().knownUnauthorized(getEndpoint(taskID))) { + return ErrorOr(unauthorized_attempt()); + } return ErrorOr(request_maybe_delivered()); } Reference peer = @@ -711,6 +714,9 @@ public: Future disc = makeDependent(IFailureMonitor::failureMonitor()).onDisconnectOrFailure(getEndpoint()); if (disc.isReady()) { + if (IFailureMonitor::failureMonitor().knownUnauthorized(getEndpoint())) { + return ErrorOr(unauthorized_attempt()); + } return ErrorOr(request_maybe_delivered()); } Reference peer = diff --git a/fdbrpc/genericactors.actor.h b/fdbrpc/genericactors.actor.h index da476bf339..75bf199b64 100644 --- a/fdbrpc/genericactors.actor.h +++ b/fdbrpc/genericactors.actor.h @@ -228,7 +228,11 @@ Future> waitValueOrSignal(Future value, try { choose { when(X x = wait(value)) { return x; } - when(wait(signal)) { return ErrorOr(request_maybe_delivered()); } + when(wait(signal)) { + return ErrorOr(IFailureMonitor::failureMonitor().knownUnauthorized(endpoint) + ? unauthorized_attempt() + : request_maybe_delivered()); + } } } catch (Error& e) { if (signal.isError()) { @@ -251,12 +255,32 @@ Future> waitValueOrSignal(Future value, ACTOR template Future sendCanceler(ReplyPromise reply, ReliablePacket* send, Endpoint endpoint) { + state bool didCancelReliable = false; try { - T t = wait(reply.getFuture()); - FlowTransport::transport().cancelReliable(send); - return t; + loop { + choose { + when(T t = wait(reply.getFuture())) { + FlowTransport::transport().cancelReliable(send); + didCancelReliable = true; + return t; + } + when(wait(IFailureMonitor::failureMonitor().onDisconnectOrFailure(endpoint))) { + if (IFailureMonitor::failureMonitor().permanentlyFailed(endpoint)) { + FlowTransport::transport().cancelReliable(send); + didCancelReliable = true; + if (IFailureMonitor::failureMonitor().knownUnauthorized(endpoint)) { + throw unauthorized_attempt(); + } else { + wait(Never()); + } + } + } + } + } } catch (Error& e) { - FlowTransport::transport().cancelReliable(send); + if (!didCancelReliable) { + FlowTransport::transport().cancelReliable(send); + } if (e.code() == error_code_broken_promise) { IFailureMonitor::failureMonitor().endpointNotFound(endpoint); } diff --git a/fdbserver/workloads/PrivateEndpoints.actor.cpp b/fdbserver/workloads/PrivateEndpoints.actor.cpp index e0353215b5..3fc36193fc 100644 --- a/fdbserver/workloads/PrivateEndpoints.actor.cpp +++ b/fdbserver/workloads/PrivateEndpoints.actor.cpp @@ -81,7 +81,8 @@ struct PrivateEndpoints : TestWorkload { } RequestStream s = optintf.get().*channel; RT req; - return assumeFailure(s.getReply(req)); + return assumeFailure(deterministicRandom()->coinflip() ? throwErrorOr(s.tryGetReply(req)) + : s.getReply(req)); }); } @@ -116,9 +117,8 @@ struct PrivateEndpoints : TestWorkload { when(wait(testFuture)) { ++self->numSuccesses; } when(wait(end)) { return Void(); } } - break; + wait(delay(0.2)); } - return Void(); } }; diff --git a/flow/error_definitions.h b/flow/error_definitions.h index 973bffdf8f..3e60a6c71a 100755 --- a/flow/error_definitions.h +++ b/flow/error_definitions.h @@ -286,9 +286,9 @@ ERROR( unknown_error, 4000, "An unknown error occurred" ) // C++ exception not ERROR( internal_error, 4100, "An internal error occurred" ) ERROR( not_implemented, 4200, "Not implemented yet" ) -// 5xxx Authorization and authentication error codes -ERROR( permission_denied, 5000, "Client is not allowed to access endpoint" ) -ERROR( unauthorized_attempt, 5001, "A untrusted client tried to send a message to a private endpoint" ) +// 6xxx Authorization and authentication error codes +ERROR( permission_denied, 6000, "Client tried to access unauthorized data" ) +ERROR( unauthorized_attempt, 6001, "A untrusted client tried to send a message to a private endpoint" ) // clang-format on #undef ERROR From 2f6e6c0d4895b1f315530880bee66300839749de Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Tue, 29 Mar 2022 15:17:09 -0600 Subject: [PATCH 18/24] fix typo --- fdbclient/WellKnownEndpoints.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fdbclient/WellKnownEndpoints.h b/fdbclient/WellKnownEndpoints.h index ab220060e3..1b91ddcf9a 100644 --- a/fdbclient/WellKnownEndpoints.h +++ b/fdbclient/WellKnownEndpoints.h @@ -28,7 +28,7 @@ * All well-known endpoints of FDB must be listed here to guarantee their uniqueness */ enum WellKnownEndpoints { - WLTOKEN_CLIENTLEADERREG_GETLEADER = WLTOKEN_FIRST_AVAILABLE, // 2 + WLTOKEN_CLIENTLEADERREG_GETLEADER = WLTOKEN_FIRST_AVAILABLE, // 4 WLTOKEN_CLIENTLEADERREG_OPENDATABASE, // 5 WLTOKEN_LEADERELECTIONREG_CANDIDACY, // 6 WLTOKEN_LEADERELECTIONREG_ELECTIONRESULT, // 7 From e2d7d4075d137d017a91ea0d0b5981e769309649 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Thu, 7 Apr 2022 11:08:07 -0600 Subject: [PATCH 19/24] multiple bug fixes --- fdbrpc/FlowTransport.actor.cpp | 11 +- fdbrpc/FlowTransport.h | 3 + fdbrpc/IPAllowList.cpp | 40 +++- fdbrpc/IPAllowList.h | 2 +- fdbrpc/PerfMetric.h | 2 +- fdbrpc/genericactors.actor.h | 21 +- fdbserver/SimulatedCluster.actor.cpp | 2 + fdbserver/tester.actor.cpp | 74 ++++--- fdbserver/workloads/ClientWorkload.actor.cpp | 184 ++++++++++++++---- .../workloads/PrivateEndpoints.actor.cpp | 25 ++- fdbserver/workloads/workloads.actor.h | 28 ++- 11 files changed, 296 insertions(+), 96 deletions(-) diff --git a/fdbrpc/FlowTransport.actor.cpp b/fdbrpc/FlowTransport.actor.cpp index 6af1786684..d897588652 100644 --- a/fdbrpc/FlowTransport.actor.cpp +++ b/fdbrpc/FlowTransport.actor.cpp @@ -920,7 +920,8 @@ void Peer::onIncomingConnection(Reference self, Reference con .suppressFor(1.0) .detail("FromAddr", conn->getPeerAddress()) .detail("CanonicalAddr", destination) - .detail("IsPublic", destination.isPublic()); + .detail("IsPublic", destination.isPublic()) + .detail("Trusted", self->transport->allowList(conn->getPeerAddress().ip)); connect.cancel(); prependConnectPacket(); @@ -1009,7 +1010,9 @@ ACTOR static void deliver(TransportData* self, } else if (destination.token.first() & TOKEN_STREAM_FLAG) { // We don't have the (stream) endpoint 'token', notify the remote machine if (receiver) { - TraceEvent(SevWarnAlways, "AttemptedRPCToPrivatePrevented").detail("From", peerAddress); + TraceEvent(SevWarnAlways, "AttemptedRPCToPrivatePrevented") + .detail("From", peerAddress) + .detail("Token", destination.token); ASSERT(!self->isLocalAddress(destination.getPrimaryAddress())); Reference peer = self->getOrOpenPeer(destination.getPrimaryAddress()); sendPacket(self, @@ -1531,6 +1534,10 @@ NetworkAddress FlowTransport::getLocalAddress() const { return self->localAddresses.address; } +void FlowTransport::setLocalAddress(NetworkAddress const& address) { + self->localAddresses.address = address; +} + const std::unordered_map>& FlowTransport::getAllPeers() const { return self->peers; } diff --git a/fdbrpc/FlowTransport.h b/fdbrpc/FlowTransport.h index 5ee4762195..8b8e5659c3 100644 --- a/fdbrpc/FlowTransport.h +++ b/fdbrpc/FlowTransport.h @@ -214,6 +214,9 @@ public: // Returns first local NetworkAddress. NetworkAddress getLocalAddress() const; + // Returns first local NetworkAddress. + void setLocalAddress(NetworkAddress const&); + // Returns all local NetworkAddress. NetworkAddressList getLocalAddresses() const; diff --git a/fdbrpc/IPAllowList.cpp b/fdbrpc/IPAllowList.cpp index 3eb6078ffe..563f296e02 100644 --- a/fdbrpc/IPAllowList.cpp +++ b/fdbrpc/IPAllowList.cpp @@ -18,8 +18,9 @@ * limitations under the License. */ -#include "fdbrpc/IPAllowList.h" #include "flow/UnitTest.h" +#include "flow/Error.h" +#include "fdbrpc/IPAllowList.h" #include #include @@ -66,10 +67,10 @@ IPAddress AuthAllowedSubnet::netmask() const { int AuthAllowedSubnet::netmaskWeight() const { if (addressMask.isV4()) { - boost::asio::ip::address_v4 addr(addressMask.toV4()); + boost::asio::ip::address_v4 addr(netmask().toV4()); return netmaskWeightImpl(addr.to_bytes()); } else { - return netmaskWeightImpl(addressMask.toV6()); + return netmaskWeightImpl(netmask().toV6()); } } @@ -88,9 +89,6 @@ AuthAllowedSubnet AuthAllowedSubnet::fromString(std::string_view addressString) // will make the last bits 0 auto mask = boost::asio::ip::address_v4(bM).to_uint(); auto baseAddress = addr.to_v4().to_uint() & mask; - fmt::print("For address {}:", addressString); - printIP("Base Address", IPAddress(baseAddress)); - printIP("Mask:", IPAddress(mask)); return AuthAllowedSubnet(IPAddress(baseAddress), IPAddress(mask)); } else { auto mask = createBitMask(addr.to_v6().to_bytes(), netmaskWeight); @@ -264,7 +262,26 @@ struct SubNetTest { } // namespace TEST_CASE("/fdbrpc/allow_list") { + // test correct weight calculation + // IPv4 + for (int i = 0; i < 33; ++i) { + auto str = fmt::format("0.0.0.0/{}", i); + auto subnet = AuthAllowedSubnet::fromString(str); + if (i != subnet.netmaskWeight()) { + fmt::print("Wrong calculated weight {} for {}\n", subnet.netmaskWeight(), str); + fmt::print("\tBase address: {}\n", subnet.baseAddress.toString()); + fmt::print("\tAddress Mask: {}\n", subnet.addressMask.toString()); + fmt::print("\tNetmask: {}\n", subnet.netmask().toString()); + ASSERT_EQ(i, subnet.netmaskWeight()); + } + } + // IPv6 + for (int i = 0; i < 129; ++i) { + auto subnet = AuthAllowedSubnet::fromString(fmt::format("0::/{}", i)); + ASSERT_EQ(i, subnet.netmaskWeight()); + } IPAllowList allowList; + // Simulated v4 addresses allowList.addTrustedSubnet("1.0.0.0/8"); allowList.addTrustedSubnet("2.0.0.0/4"); ::subnetAssert(allowList, parseAddr("1.0.1.1"), true); @@ -273,14 +290,25 @@ TEST_CASE("/fdbrpc/allow_list") { ::subnetAssert(allowList, parseAddr("128.0.1.1"), false); allowList = IPAllowList(); allowList.addTrustedSubnet("0.0.0.0/2"); + allowList.addTrustedSubnet("abcd::/16"); ::subnetAssert(allowList, parseAddr("1.0.1.1"), true); ::subnetAssert(allowList, parseAddr("1.1.2.2"), true); ::subnetAssert(allowList, parseAddr("2.2.1.1"), true); + ::subnetAssert(allowList, parseAddr("4.0.1.2"), true); ::subnetAssert(allowList, parseAddr("5.2.1.1"), true); ::subnetAssert(allowList, parseAddr("128.0.1.1"), false); ::subnetAssert(allowList, parseAddr("192.168.3.1"), false); + // Simulated v6 addresses + ::subnetAssert(allowList, parseAddr("abcd::1:2:3:4"), true); + ::subnetAssert(allowList, parseAddr("abcd::2:3:3:4"), true); + ::subnetAssert(allowList, parseAddr("abcd:ab:ab:fdb:2:3:3:4"), true); + ::subnetAssert(allowList, parseAddr("2001:fdb1:fdb2:fdb3:fdb4:fdb5:fdb6:12"), false); + ::subnetAssert(allowList, parseAddr("2001:fdb1:fdb2:fdb3:fdb4:fdb5:fdb6:1"), false); + ::subnetAssert(allowList, parseAddr("2001:fdb1:fdb2:fdb3:fdb4:fdb5:fdb6:fdb"), false); + // Corner Cases allowList = IPAllowList(); allowList.addTrustedSubnet("0.0.0.0/0"); + // Random address tests SubNetTest subnetTest(allowList.subnets()[0]); for (int i = 0; i < 10; ++i) { // All IPv4 addresses are in the allow list diff --git a/fdbrpc/IPAllowList.h b/fdbrpc/IPAllowList.h index a89939add6..b3a44e30fc 100644 --- a/fdbrpc/IPAllowList.h +++ b/fdbrpc/IPAllowList.h @@ -45,7 +45,7 @@ struct AuthAllowedSubnet { if (netmaskWeight % 8 > 0) { // 2^(netmaskWeight % 8) - 1 sets the last (netmaskWeight % 8) number of bits to 1 // everything else will be zero. For example: 2^3 - 1 == 7 == 0b111 - unsigned char bitmask = (1 << (netmaskWeight % 8)) - ((unsigned char)1); + unsigned char bitmask = (1 << (8 - (netmaskWeight % 8))) - ((unsigned char)1); res[idx] ^= bitmask; ++idx; } diff --git a/fdbrpc/PerfMetric.h b/fdbrpc/PerfMetric.h index 2f1967ecda..8513ae7f97 100644 --- a/fdbrpc/PerfMetric.h +++ b/fdbrpc/PerfMetric.h @@ -43,7 +43,7 @@ struct PerfMetric { std::string format_code() const { return m_format_code; } bool averaged() const { return m_averaged; } - PerfMetric withPrefix(const std::string& pre) { + PerfMetric withPrefix(const std::string& pre) const { return PerfMetric(pre + name(), value(), Averaged{ averaged() }, format_code()); } diff --git a/fdbrpc/genericactors.actor.h b/fdbrpc/genericactors.actor.h index 75bf199b64..c80dd9d1e9 100644 --- a/fdbrpc/genericactors.actor.h +++ b/fdbrpc/genericactors.actor.h @@ -258,23 +258,22 @@ Future sendCanceler(ReplyPromise reply, ReliablePacket* send, Endpoint end state bool didCancelReliable = false; try { loop { + if (IFailureMonitor::failureMonitor().permanentlyFailed(endpoint)) { + FlowTransport::transport().cancelReliable(send); + didCancelReliable = true; + if (IFailureMonitor::failureMonitor().knownUnauthorized(endpoint)) { + throw unauthorized_attempt(); + } else { + wait(Never()); + } + } choose { when(T t = wait(reply.getFuture())) { FlowTransport::transport().cancelReliable(send); didCancelReliable = true; return t; } - when(wait(IFailureMonitor::failureMonitor().onDisconnectOrFailure(endpoint))) { - if (IFailureMonitor::failureMonitor().permanentlyFailed(endpoint)) { - FlowTransport::transport().cancelReliable(send); - didCancelReliable = true; - if (IFailureMonitor::failureMonitor().knownUnauthorized(endpoint)) { - throw unauthorized_attempt(); - } else { - wait(Never()); - } - } - } + when(wait(IFailureMonitor::failureMonitor().onStateChanged(endpoint))) {} } } } catch (Error& e) { diff --git a/fdbserver/SimulatedCluster.actor.cpp b/fdbserver/SimulatedCluster.actor.cpp index 47272c243b..88621e0973 100644 --- a/fdbserver/SimulatedCluster.actor.cpp +++ b/fdbserver/SimulatedCluster.actor.cpp @@ -497,6 +497,7 @@ ACTOR Future simulatedFDBDRebooter(Reference& m) override { - for (int w = 0; w < workloads.size(); w++) { + + ACTOR static Future> getMetrics(CompoundWorkload* self) { + state std::vector>> results; + for (int w = 0; w < self->workloads.size(); w++) { std::vector p; - workloads[w]->getMetrics(p); - for (int i = 0; i < p.size(); i++) - m.push_back(p[i].withPrefix(workloads[w]->description() + ".")); + results.push_back(self->workloads[w]->getMetrics()); } + wait(waitForAll(results)); + std::vector res; + for (int i = 0; i < results.size(); ++i) { + auto const& p = results[i].get(); + for (auto const& m : p) { + res.push_back(m.withPrefix(self->workloads[i]->description() + ".")); + } + } + return res; } + + Future> getMetrics() override { return getMetrics(this); } double getCheckTimeout() const override { double m = 0; for (int w = 0; w < workloads.size(); w++) m = std::max(workloads[w]->getCheckTimeout(), m); return m; } + + void getMetrics(std::vector&) override { ASSERT(false); } }; -Reference getWorkloadIface(WorkloadRequest work, - Reference ccr, - VectorRef options, - Reference const> dbInfo) { - Value testName = getOption(options, LiteralStringRef("testName"), LiteralStringRef("no-test-specified")); +ACTOR Future> getWorkloadIface(WorkloadRequest work, + Reference ccr, + VectorRef options, + Reference const> dbInfo) { + state Reference workload; + state Value testName = getOption(options, LiteralStringRef("testName"), LiteralStringRef("no-test-specified")); WorkloadContext wcx; wcx.clientId = work.clientId; wcx.clientCount = work.clientCount; @@ -355,7 +369,8 @@ Reference getWorkloadIface(WorkloadRequest work, wcx.options = options; wcx.sharedRandomNumber = work.sharedRandomNumber; - auto workload = IWorkloadFactory::create(testName.toString(), wcx); + workload = IWorkloadFactory::create(testName.toString(), wcx); + wait(workload->initialized()); auto unconsumedOptions = checkAllOptionsConsumed(workload ? workload->options : VectorRef()); if (!workload || unconsumedOptions.size()) { @@ -380,26 +395,33 @@ Reference getWorkloadIface(WorkloadRequest work, return workload; } -Reference getWorkloadIface(WorkloadRequest work, - Reference ccr, - Reference const> dbInfo) { +ACTOR Future> getWorkloadIface(WorkloadRequest work, + Reference ccr, + Reference const> dbInfo) { + state WorkloadContext wcx; + state std::vector>> ifaces; if (work.options.size() < 1) { TraceEvent(SevError, "TestCreationError").detail("Reason", "No options provided"); fprintf(stderr, "ERROR: No options were provided for workload.\n"); throw test_specification_invalid(); } - if (work.options.size() == 1) - return getWorkloadIface(work, ccr, work.options[0], dbInfo); + if (work.options.size() == 1) { + Reference res = wait(getWorkloadIface(work, ccr, work.options[0], dbInfo)); + return res; + } - WorkloadContext wcx; wcx.clientId = work.clientId; wcx.clientCount = work.clientCount; wcx.sharedRandomNumber = work.sharedRandomNumber; // FIXME: Other stuff not filled in; why isn't this constructed here and passed down to the other // getWorkloadIface()? + for (int i = 0; i < work.options.size(); i++) { + ifaces.push_back(getWorkloadIface(work, ccr, work.options[i], dbInfo)); + } + wait(waitForAll(ifaces)); auto compound = makeReference(wcx); for (int i = 0; i < work.options.size(); i++) { - compound->add(getWorkloadIface(work, ccr, work.options[i], dbInfo)); + compound->add(ifaces[i].getValue()); } return compound; } @@ -498,7 +520,7 @@ ACTOR Future testDatabaseLiveness(Database cx, try { state double start = now(); auto traceMsg = "PingingDatabaseLiveness_" + context; - TraceEvent(traceMsg.c_str()); + TraceEvent(traceMsg.c_str()).log(); wait(timeoutError(pingDatabase(cx), databasePingDelay)); double pingTime = now() - start; ASSERT(pingTime > 0); @@ -611,10 +633,9 @@ ACTOR Future runWorkloadAsync(Database cx, when(ReplyPromise> req = waitNext(workIface.metrics.getFuture())) { state ReplyPromise> s_req = req; try { - std::vector m; - workload->getMetrics(m); + std::vector m = wait(workload->getMetrics()); TraceEvent("WorkloadSendMetrics", workIface.id()).detail("Count", m.size()); - req.send(m); + s_req.send(m); } catch (Error& e) { if (e.code() == error_code_please_reboot || e.code() == error_code_please_reboot_delete) throw; @@ -653,7 +674,7 @@ ACTOR Future testerServerWorkload(WorkloadRequest work, // add test for "done" ? TraceEvent("WorkloadReceived", workIface.id()).detail("Title", work.title); - auto workload = getWorkloadIface(work, ccr, dbInfo); + Reference workload = wait(getWorkloadIface(work, ccr, dbInfo)); if (!workload) { TraceEvent("TestCreationError").detail("Reason", "Workload could not be created"); fprintf(stderr, "ERROR: The workload could not be created.\n"); @@ -708,6 +729,9 @@ ACTOR Future testerServerCore(TesterInterface interf, ACTOR Future clearData(Database cx) { state Transaction tr(cx); + state UID debugID = debugRandom()->randomUniqueID(); + TraceEvent("TesterClearingDatabaseStart", debugID).log(); + tr.debugTransaction(debugID); loop { try { // This transaction needs to be self-conflicting, but not conflict consistently with @@ -716,10 +740,10 @@ ACTOR Future clearData(Database cx) { tr.makeSelfConflicting(); wait(success(tr.getReadVersion())); // required since we use addReadConflictRange but not get wait(tr.commit()); - TraceEvent("TesterClearingDatabase").detail("AtVersion", tr.getCommittedVersion()); + TraceEvent("TesterClearingDatabase", debugID).detail("AtVersion", tr.getCommittedVersion()); break; } catch (Error& e) { - TraceEvent(SevWarn, "TesterClearingDatabaseError").error(e); + TraceEvent(SevWarn, "TesterClearingDatabaseError", debugID).error(e); wait(tr.onError(e)); } } diff --git a/fdbserver/workloads/ClientWorkload.actor.cpp b/fdbserver/workloads/ClientWorkload.actor.cpp index c93ea6474d..3ac5aab921 100644 --- a/fdbserver/workloads/ClientWorkload.actor.cpp +++ b/fdbserver/workloads/ClientWorkload.actor.cpp @@ -26,30 +26,26 @@ #include "flow/actorcompiler.h" // has to be last include -struct ClientWorkloadImpl { - UID id = deterministicRandom()->randomUniqueID(); - Reference child; - ISimulator::ProcessInfo* self = g_pSimulator->getCurrentProcess(); - ISimulator::ProcessInfo* childProcess = nullptr; +class WorkloadProcessState { IPAddress childAddress; std::string processName; - Database cx; - Future databaseOpened; - std::string dataFolder; + Future init; - ClientWorkloadImpl(Reference const& child) : child(child) { + WorkloadProcessState(int clientId) : clientId(clientId) { + auto self = g_simulator.getCurrentProcess(); if (self->address.isV6()) { childAddress = - IPAddress::parse(fmt::format("2001:fdb1:fdb2:fdb3:fdb4:fdb5:fdb6:{:04x}", child->clientId + 2)).get(); + IPAddress::parse(fmt::format("2001:fdb1:fdb2:fdb3:fdb4:fdb5:fdb6:{:04x}", clientId + 2)).get(); } else { - childAddress = IPAddress::parse(fmt::format("192.168.0.{}", child->clientId + 2)).get(); + childAddress = IPAddress::parse(fmt::format("192.168.0.{}", clientId + 2)).get(); } - TraceEvent("TestClientStart", id).detail("ClusterFileLocation", child->ccr->getLocation()).log(); - processName = fmt::format("TestClient{}", child->clientId); + //TraceEvent("TestClientStart", id).detail("ClusterFileLocation", child->ccr->getLocation()).log(); + processName = fmt::format("TestClient{}", clientId); Standalone newZoneId(deterministicRandom()->randomUniqueID().toString()); auto locality = LocalityData(Optional>(), newZoneId, newZoneId, self->locality.dcId()); - dataFolder = joinPath(popPath(self->dataFolder), deterministicRandom()->randomUniqueID().toString()); + auto dataFolder = joinPath(popPath(self->dataFolder), deterministicRandom()->randomUniqueID().toString()); platform::createDirectory(dataFolder); + TraceEvent("StartingClientForWorkload", id).detail("Name", processName).detail("Address", childAddress); childProcess = g_simulator.newProcess(processName.c_str(), childAddress, 1, @@ -61,66 +57,180 @@ struct ClientWorkloadImpl { self->coordinationFolder, self->protocolVersion); childProcess->excludeFromRestarts = true; - databaseOpened = openDatabase(this); + init = doInitialize(this); } - ~ClientWorkloadImpl() { g_simulator.destroyProcess(childProcess); } + ~WorkloadProcessState() { + TraceEvent("ShutdownClientForWorkload", id).log(); + g_simulator.destroyProcess(childProcess); + } - ACTOR static Future openDatabase(ClientWorkloadImpl* self) { - wait(g_simulator.onProcess(self->childProcess)); - FlowTransport::createInstance(true, 1, WLTOKEN_RESERVED_COUNT); - Sim2FileSystem::newFileSystem(); - TraceEvent("ClientWorkloadOpenDatabase", self->id) - .detail("ClusterFileLocation", self->child->ccr->getLocation()); - self->cx = Database::createDatabase(self->child->ccr, -1); - wait(g_simulator.onProcess(self->self)); + static std::vector>& states() { + static std::vector> res; + return res; + } + + ACTOR static Future doInitialize(WorkloadProcessState* self) { + state ISimulator::ProcessInfo* parent = g_simulator.getCurrentProcess(); + wait(g_simulator.onProcess(self->childProcess, TaskPriority::DefaultYield)); + try { + FlowTransport::createInstance(true, 1, WLTOKEN_RESERVED_COUNT); + Sim2FileSystem::newFileSystem(); + auto addr = g_simulator.getCurrentProcess()->address; + FlowTransport::transport().setLocalAddress(addr); + TraceEvent("WorkloadProcessInitialized", self->id); + } catch (...) { + ASSERT(false); + } + wait(g_simulator.onProcess(parent, TaskPriority::DefaultYield)); return Void(); } +public: + static WorkloadProcessState* instance(int clientId) { + states().resize(std::max(states().size(), size_t(clientId + 1)), std::make_pair(nullptr, 0)); + auto& res = states()[clientId]; + if (res.first == nullptr) { + res = std::make_pair(new WorkloadProcessState(clientId), 0); + } + ++res.second; + return res.first; + } + static void done(WorkloadProcessState* processState) { + auto& p = states()[processState->clientId]; + ASSERT(p.first == processState); + if (--p.second == 0) { + // delete p.first; + // p.first = nullptr; + } + } + + Future initialized() const { return init; } + + UID id = deterministicRandom()->randomUniqueID(); + int clientId; + ISimulator::ProcessInfo* childProcess; +}; + +struct WorkloadProcess { + WorkloadProcessState* processState; + UID id = deterministicRandom()->randomUniqueID(); + Database cx; + Future databaseOpened; + Reference child; + std::string desc; + + void createDatabase(ClientWorkload::CreateWorkload const& childCreator, WorkloadContext const& wcx) { + try { + child = childCreator(wcx); + TraceEvent("ClientWorkloadOpenDatabase", id).detail("ClusterFileLocation", child->ccr->getLocation()); + cx = Database::createDatabase(child->ccr, -1); + desc = child->description(); + } catch (Error&) { + throw; + } catch (...) { + ASSERT(false); + } + } + + ACTOR static Future openDatabase(WorkloadProcess* self, + ClientWorkload::CreateWorkload childCreator, + WorkloadContext wcx) { + state ISimulator::ProcessInfo* parent = g_simulator.getCurrentProcess(); + state Optional err; + wcx.dbInfo = Reference const>(); + wait(self->processState->initialized()); + wait(g_simulator.onProcess(self->childProcess(), TaskPriority::DefaultYield)); + try { + self->createDatabase(childCreator, wcx); + } catch (Error& e) { + ASSERT(e.code() != error_code_actor_cancelled); + err = e; + } + wait(g_simulator.onProcess(parent, TaskPriority::DefaultYield)); + if (err.present()) { + throw err.get(); + } + return Void(); + } + + ISimulator::ProcessInfo* childProcess() { return processState->childProcess; } + + int clientId() const { return processState->clientId; } + + WorkloadProcess(ClientWorkload::CreateWorkload const& childCreator, WorkloadContext const& wcx) + : processState(WorkloadProcessState::instance(wcx.clientId)) { + TraceEvent("StartingClinetWorkload", id).detail("OnClientProcess", processState->id); + databaseOpened = openDatabase(this, childCreator, wcx); + } + + ACTOR static void destroy(WorkloadProcess* self) { + state ISimulator::ProcessInfo* parent = g_simulator.getCurrentProcess(); + wait(g_simulator.onProcess(self->childProcess(), TaskPriority::DefaultYield)); + delete self; + wait(g_simulator.onProcess(parent, TaskPriority::DefaultYield)); + } + + std::string description() { return desc; } + ACTOR template - Future runActor(ClientWorkloadImpl* self, Fun f) { + Future runActor(WorkloadProcess* self, Optional defaultTenant, Fun f) { state Optional err; state Ret res; - wait(g_simulator.onProcess(self->childProcess)); + state ISimulator::ProcessInfo* parent = g_simulator.getCurrentProcess(); wait(self->databaseOpened); + wait(g_simulator.onProcess(self->childProcess(), TaskPriority::DefaultYield)); + self->cx->defaultTenant = defaultTenant; try { Ret r = wait(f(self->cx)); res = r; } catch (Error& e) { if (e.code() == error_code_actor_cancelled) { + ASSERT(false); throw; } err = e; } - wait(g_simulator.onProcess(self->self)); + wait(g_simulator.onProcess(parent, TaskPriority::DefaultYield)); if (err.present()) { throw err.get(); } return res; } + + ~WorkloadProcess() { WorkloadProcessState::done(processState); } }; -ClientWorkload::ClientWorkload(Reference const& child, WorkloadContext const& wcx) - : TestWorkload(wcx), impl(new ClientWorkloadImpl(child)) { - child->dbInfo = Reference>(); -} +ClientWorkload::ClientWorkload(CreateWorkload const& childCreator, WorkloadContext const& wcx) + : TestWorkload(wcx), impl(new WorkloadProcess(childCreator, wcx)) {} -ClientWorkload::~ClientWorkload() {} +ClientWorkload::~ClientWorkload() { + WorkloadProcess::destroy(impl); +} std::string ClientWorkload::description() const { - return impl->child->description(); + return impl->description(); } + +Future ClientWorkload::initialized() { + return impl->databaseOpened; +} + Future ClientWorkload::setup(Database const& cx) { - return impl->runActor(impl, [this](Database const& db) { return impl->child->setup(db); }); + return impl->runActor(impl, cx->defaultTenant, [this](Database const& db) { return impl->child->setup(db); }); } Future ClientWorkload::start(Database const& cx) { - return impl->runActor(impl, [this](Database const& db) { return impl->child->start(db); }); + return impl->runActor(impl, cx->defaultTenant, [this](Database const& db) { return impl->child->start(db); }); } Future ClientWorkload::check(Database const& cx) { - return impl->runActor(impl, [this](Database const& db) { return impl->child->check(db); }); + return impl->runActor(impl, cx->defaultTenant, [this](Database const& db) { return impl->child->check(db); }); +} +Future> ClientWorkload::getMetrics() { + return impl->runActor>( + impl, Optional(), [this](Database const& db) { return impl->child->getMetrics(); }); } void ClientWorkload::getMetrics(std::vector& m) { - return impl->child->getMetrics(m); + ASSERT(false); } double ClientWorkload::getCheckTimeout() const { diff --git a/fdbserver/workloads/PrivateEndpoints.actor.cpp b/fdbserver/workloads/PrivateEndpoints.actor.cpp index 3fc36193fc..b4d95afe4f 100644 --- a/fdbserver/workloads/PrivateEndpoints.actor.cpp +++ b/fdbserver/workloads/PrivateEndpoints.actor.cpp @@ -64,6 +64,7 @@ struct PrivateEndpoints : TestWorkload { if (e.code() == error_code_actor_cancelled) { throw; } else if (e.code() == error_code_unauthorized_attempt) { + TraceEvent("SuccessPrivateEndpoint").log(); return Void(); } else { TraceEvent(SevError, "WrongErrorCode").error(e); @@ -109,16 +110,28 @@ struct PrivateEndpoints : TestWorkload { ACTOR static Future _start(PrivateEndpoints* self, Database cx) { state Reference> clientInfo = cx->clientInfo; state Future end; + TraceEvent("PrivateEndpointTestStartWait").detail("WaitTime", self->startAfter).log(); wait(delay(self->startAfter)); + TraceEvent("PrivateEndpointTestStart").detail("RunFor", self->runFor).log(); end = delay(self->runFor); - loop { - auto testFuture = deterministicRandom()->randomChoice(self->testFunctions)(cx->clientInfo); - choose { - when(wait(testFuture)) { ++self->numSuccesses; } - when(wait(end)) { return Void(); } + try { + loop { + auto testFuture = deterministicRandom()->randomChoice(self->testFunctions)(cx->clientInfo); + choose { + when(wait(end)) { + TraceEvent("PrivateEndpointTestDone").log(); + return Void(); + } + when(wait(testFuture)) { ++self->numSuccesses; } + } + wait(delay(0.2)); } - wait(delay(0.2)); + } catch (Error& e) { + TraceEvent(SevError, "PrivateEndpointTestError").errorUnsuppressed(e); + ASSERT(false); } + UNREACHABLE(); + return Void(); } }; diff --git a/fdbserver/workloads/workloads.actor.h b/fdbserver/workloads/workloads.actor.h index 7bc36893a6..63893384e5 100644 --- a/fdbserver/workloads/workloads.actor.h +++ b/fdbserver/workloads/workloads.actor.h @@ -30,7 +30,10 @@ #include "fdbserver/KnobProtectiveGroups.h" #include "fdbserver/TesterInterface.actor.h" #include "fdbrpc/simulator.h" -#include "flow/actorcompiler.h" + +#include + +#include "flow/actorcompiler.h" // has to be last import /* * Gets an Value from a list of key/value pairs, using a default value if the key is not present. @@ -70,27 +73,38 @@ struct TestWorkload : NonCopyable, WorkloadContext, ReferenceCounted initialized() { return Void(); } virtual std::string description() const = 0; virtual Future setup(Database const& cx) { return Void(); } virtual Future start(Database const& cx) = 0; virtual Future check(Database const& cx) = 0; - virtual void getMetrics(std::vector& m) = 0; + virtual Future> getMetrics() { + std::vector result; + getMetrics(result); + return result; + } virtual double getCheckTimeout() const { return 3000; } enum WorkloadPhase { SETUP = 1, EXECUTION = 2, CHECK = 4, METRICS = 8 }; + +private: + virtual void getMetrics(std::vector& m) = 0; }; -struct ClientWorkloadImpl; +struct WorkloadProcess; struct ClientWorkload : TestWorkload { - ClientWorkloadImpl* impl; - ClientWorkload(Reference const& child, WorkloadContext const& wcx); + WorkloadProcess* impl; + using CreateWorkload = std::function(WorkloadContext const&)>; + ClientWorkload(CreateWorkload const& childCreator, WorkloadContext const& wcx); ~ClientWorkload(); + Future initialized() override; std::string description() const override; Future setup(Database const& cx) override; Future start(Database const& cx) override; Future check(Database const& cx) override; void getMetrics(std::vector& m) override; + Future> getMetrics() override; double getCheckTimeout() const override; }; @@ -143,8 +157,8 @@ struct WorkloadFactory : IWorkloadFactory { } Reference create(WorkloadContext const& wcx) override { if (g_network->isSimulated() && asClient) { - auto child = makeReference(wcx); - return makeReference(child, wcx); + return makeReference( + [](WorkloadContext const& wcx) { return makeReference(wcx); }, wcx); } return makeReference(wcx); } From c82701dd00b3cd3ddca4a5a2ef707501654899ce Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Thu, 7 Apr 2022 14:40:28 -0600 Subject: [PATCH 20/24] fixed merge bug --- fdbserver/fdbserver.actor.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/fdbserver/fdbserver.actor.cpp b/fdbserver/fdbserver.actor.cpp index e132e0a37b..d27d65d97e 100644 --- a/fdbserver/fdbserver.actor.cpp +++ b/fdbserver/fdbserver.actor.cpp @@ -1869,6 +1869,7 @@ int main(int argc, char* argv[]) { g_network = newNet2(opts.tlsConfig, opts.useThreadPool, true); g_network->addStopCallback(Net2FileSystem::stop); FlowTransport::createInstance(false, 1, WLTOKEN_RESERVED_COUNT, &opts.allowList); + opts.buildNetwork(argv[0]); const bool expectsPublicAddress = (role == ServerRole::FDBD || role == ServerRole::NetworkTestServer || role == ServerRole::Restore || role == ServerRole::FlowProcess); From cf0757bbcafcd0d3267f44655fc95bd9dd8f2f08 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Fri, 8 Apr 2022 11:06:18 -0600 Subject: [PATCH 21/24] fix minor test bug request_maybe_delivered is a legitimate error and seeing this is not a bug --- fdbserver/workloads/PrivateEndpoints.actor.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fdbserver/workloads/PrivateEndpoints.actor.cpp b/fdbserver/workloads/PrivateEndpoints.actor.cpp index b4d95afe4f..29abb389ed 100644 --- a/fdbserver/workloads/PrivateEndpoints.actor.cpp +++ b/fdbserver/workloads/PrivateEndpoints.actor.cpp @@ -66,6 +66,11 @@ struct PrivateEndpoints : TestWorkload { } else if (e.code() == error_code_unauthorized_attempt) { TraceEvent("SuccessPrivateEndpoint").log(); return Void(); + } else if (e.code() == error_code_request_maybe_delivered) { + // this is also fine, because even when calling private endpoints + // we might see connection failures + TraceEvent("SuccessRequestMaybeDelivered").log(); + return Void(); } else { TraceEvent(SevError, "WrongErrorCode").error(e); return Void(); From d8a0b57b6c8eb301cc547245860efd494660fe83 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Sun, 10 Apr 2022 14:09:15 -0600 Subject: [PATCH 22/24] clients have to listen on a port in simulation --- fdbrpc/sim2.actor.cpp | 1 + fdbserver/workloads/ClientWorkload.actor.cpp | 87 ++++++++++++-------- 2 files changed, 52 insertions(+), 36 deletions(-) diff --git a/fdbrpc/sim2.actor.cpp b/fdbrpc/sim2.actor.cpp index 64817a5214..2b3cdb420c 100644 --- a/fdbrpc/sim2.actor.cpp +++ b/fdbrpc/sim2.actor.cpp @@ -445,6 +445,7 @@ private: TraceEvent(SevError, "LeakedConnection", self->dbgid) .error(connection_leaked()) .detail("MyAddr", self->process->address) + .detail("IsPublic", self->process->address.isPublic()) .detail("PeerAddr", self->peerEndpoint) .detail("PeerId", self->peerId) .detail("Opened", self->opened); diff --git a/fdbserver/workloads/ClientWorkload.actor.cpp b/fdbserver/workloads/ClientWorkload.actor.cpp index 3ac5aab921..390745177a 100644 --- a/fdbserver/workloads/ClientWorkload.actor.cpp +++ b/fdbserver/workloads/ClientWorkload.actor.cpp @@ -29,35 +29,11 @@ class WorkloadProcessState { IPAddress childAddress; std::string processName; - Future init; + Future processActor; + Promise init; WorkloadProcessState(int clientId) : clientId(clientId) { - auto self = g_simulator.getCurrentProcess(); - if (self->address.isV6()) { - childAddress = - IPAddress::parse(fmt::format("2001:fdb1:fdb2:fdb3:fdb4:fdb5:fdb6:{:04x}", clientId + 2)).get(); - } else { - childAddress = IPAddress::parse(fmt::format("192.168.0.{}", clientId + 2)).get(); - } - //TraceEvent("TestClientStart", id).detail("ClusterFileLocation", child->ccr->getLocation()).log(); - processName = fmt::format("TestClient{}", clientId); - Standalone newZoneId(deterministicRandom()->randomUniqueID().toString()); - auto locality = LocalityData(Optional>(), newZoneId, newZoneId, self->locality.dcId()); - auto dataFolder = joinPath(popPath(self->dataFolder), deterministicRandom()->randomUniqueID().toString()); - platform::createDirectory(dataFolder); - TraceEvent("StartingClientForWorkload", id).detail("Name", processName).detail("Address", childAddress); - childProcess = g_simulator.newProcess(processName.c_str(), - childAddress, - 1, - self->address.isTLS(), - 1, - locality, - ProcessClass(ProcessClass::TesterClass, ProcessClass::AutoSource), - dataFolder.c_str(), - self->coordinationFolder, - self->protocolVersion); - childProcess->excludeFromRestarts = true; - init = doInitialize(this); + processActor = processStart(this); } ~WorkloadProcessState() { @@ -65,27 +41,66 @@ class WorkloadProcessState { g_simulator.destroyProcess(childProcess); } - static std::vector>& states() { - static std::vector> res; - return res; + ACTOR static Future initializationDone(WorkloadProcessState* self, ISimulator::ProcessInfo* parent) { + wait(g_simulator.onProcess(parent, TaskPriority::DefaultYield)); + self->init.send(Void()); + wait(Never()); + ASSERT(false); // does not happen + return Void(); } - ACTOR static Future doInitialize(WorkloadProcessState* self) { + ACTOR static Future processStart(WorkloadProcessState* self) { state ISimulator::ProcessInfo* parent = g_simulator.getCurrentProcess(); + state std::vector> futures; + if (parent->address.isV6()) { + self->childAddress = + IPAddress::parse(fmt::format("2001:fdb1:fdb2:fdb3:fdb4:fdb5:fdb6:{:04x}", self->clientId + 2)).get(); + } else { + self->childAddress = IPAddress::parse(fmt::format("192.168.0.{}", self->clientId + 2)).get(); + } + //TraceEvent("TestClientStart", id).detail("ClusterFileLocation", child->ccr->getLocation()).log(); + self->processName = fmt::format("TestClient{}", self->clientId); + Standalone newZoneId(deterministicRandom()->randomUniqueID().toString()); + auto locality = LocalityData(Optional>(), newZoneId, newZoneId, parent->locality.dcId()); + auto dataFolder = joinPath(popPath(parent->dataFolder), deterministicRandom()->randomUniqueID().toString()); + platform::createDirectory(dataFolder); + TraceEvent("StartingClientForWorkload", self->id).detail("Name", self->processName).detail("Address", self->childAddress); + self->childProcess = g_simulator.newProcess(self->processName.c_str(), + self->childAddress, + 1, + parent->address.isTLS(), + 1, + locality, + ProcessClass(ProcessClass::TesterClass, ProcessClass::AutoSource), + dataFolder.c_str(), + parent->coordinationFolder, + parent->protocolVersion); + self->childProcess->excludeFromRestarts = true; wait(g_simulator.onProcess(self->childProcess, TaskPriority::DefaultYield)); try { FlowTransport::createInstance(true, 1, WLTOKEN_RESERVED_COUNT); Sim2FileSystem::newFileSystem(); auto addr = g_simulator.getCurrentProcess()->address; - FlowTransport::transport().setLocalAddress(addr); - TraceEvent("WorkloadProcessInitialized", self->id); - } catch (...) { + futures.push_back(FlowTransport::transport().bind(addr, addr)); + futures.push_back(success((self->childProcess->onShutdown()))); + TraceEvent("WorkloadProcessInitialized", self->id).log(); + futures.push_back(initializationDone(self, parent)); + wait(waitForAny(futures)); + } catch (Error& e) { + if (e.code() == error_code_actor_cancelled) { + return Void(); + } ASSERT(false); } - wait(g_simulator.onProcess(parent, TaskPriority::DefaultYield)); + ASSERT(false); return Void(); } + static std::vector>& states() { + static std::vector> res; + return res; + } + public: static WorkloadProcessState* instance(int clientId) { states().resize(std::max(states().size(), size_t(clientId + 1)), std::make_pair(nullptr, 0)); @@ -105,7 +120,7 @@ public: } } - Future initialized() const { return init; } + Future initialized() const { return init.getFuture(); } UID id = deterministicRandom()->randomUniqueID(); int clientId; From 64ac66c1d0552202f6a0ac86171d6d33813ddad7 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Sun, 10 Apr 2022 14:16:21 -0600 Subject: [PATCH 23/24] fix merge conflict --- fdbclient/StorageServerInterface.h | 6 +---- fdbrpc/sim2.actor.cpp | 2 +- fdbserver/workloads/ClientWorkload.actor.cpp | 26 ++++++++++---------- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/fdbclient/StorageServerInterface.h b/fdbclient/StorageServerInterface.h index 04bb375dd4..13ba8f1e18 100644 --- a/fdbclient/StorageServerInterface.h +++ b/fdbclient/StorageServerInterface.h @@ -158,13 +158,9 @@ public: checkpoint = PublicRequestStream(getValue.getEndpoint().getAdjustedEndpoint(19)); fetchCheckpoint = -<<<<<<< variant A PublicRequestStream(getValue.getEndpoint().getAdjustedEndpoint(20)); ->>>>>>> variant B - RequestStream(getValue.getEndpoint().getAdjustedEndpoint(20)); - fetchCheckpointKeyValues = RequestStream( + fetchCheckpointKeyValues = PublicRequestStream( getValue.getEndpoint().getAdjustedEndpoint(21)); -======= end } } else { ASSERT(Ar::isDeserializing); diff --git a/fdbrpc/sim2.actor.cpp b/fdbrpc/sim2.actor.cpp index 2b3cdb420c..05e2f513ce 100644 --- a/fdbrpc/sim2.actor.cpp +++ b/fdbrpc/sim2.actor.cpp @@ -445,7 +445,7 @@ private: TraceEvent(SevError, "LeakedConnection", self->dbgid) .error(connection_leaked()) .detail("MyAddr", self->process->address) - .detail("IsPublic", self->process->address.isPublic()) + .detail("IsPublic", self->process->address.isPublic()) .detail("PeerAddr", self->peerEndpoint) .detail("PeerId", self->peerId) .detail("Opened", self->opened); diff --git a/fdbserver/workloads/ClientWorkload.actor.cpp b/fdbserver/workloads/ClientWorkload.actor.cpp index 390745177a..831825ed7e 100644 --- a/fdbserver/workloads/ClientWorkload.actor.cpp +++ b/fdbserver/workloads/ClientWorkload.actor.cpp @@ -32,9 +32,7 @@ class WorkloadProcessState { Future processActor; Promise init; - WorkloadProcessState(int clientId) : clientId(clientId) { - processActor = processStart(this); - } + WorkloadProcessState(int clientId) : clientId(clientId) { processActor = processStart(this); } ~WorkloadProcessState() { TraceEvent("ShutdownClientForWorkload", id).log(); @@ -64,17 +62,19 @@ class WorkloadProcessState { auto locality = LocalityData(Optional>(), newZoneId, newZoneId, parent->locality.dcId()); auto dataFolder = joinPath(popPath(parent->dataFolder), deterministicRandom()->randomUniqueID().toString()); platform::createDirectory(dataFolder); - TraceEvent("StartingClientForWorkload", self->id).detail("Name", self->processName).detail("Address", self->childAddress); + TraceEvent("StartingClientForWorkload", self->id) + .detail("Name", self->processName) + .detail("Address", self->childAddress); self->childProcess = g_simulator.newProcess(self->processName.c_str(), - self->childAddress, - 1, - parent->address.isTLS(), - 1, - locality, - ProcessClass(ProcessClass::TesterClass, ProcessClass::AutoSource), - dataFolder.c_str(), - parent->coordinationFolder, - parent->protocolVersion); + self->childAddress, + 1, + parent->address.isTLS(), + 1, + locality, + ProcessClass(ProcessClass::TesterClass, ProcessClass::AutoSource), + dataFolder.c_str(), + parent->coordinationFolder, + parent->protocolVersion); self->childProcess->excludeFromRestarts = true; wait(g_simulator.onProcess(self->childProcess, TaskPriority::DefaultYield)); try { From 099385928ca5b6c476f9897b1c6e5a6e69ed55a4 Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Mon, 11 Apr 2022 09:17:10 -0600 Subject: [PATCH 24/24] Address review comments --- fdbrpc/IPAllowList.cpp | 29 ++++++++++++++++++ fdbrpc/IPAllowList.h | 23 ++------------ fdbserver/workloads/ClientWorkload.actor.cpp | 30 ++++++------------- .../workloads/PrivateEndpoints.actor.cpp | 5 +--- 4 files changed, 41 insertions(+), 46 deletions(-) diff --git a/fdbrpc/IPAllowList.cpp b/fdbrpc/IPAllowList.cpp index 563f296e02..be894b0e1a 100644 --- a/fdbrpc/IPAllowList.cpp +++ b/fdbrpc/IPAllowList.cpp @@ -50,6 +50,11 @@ int netmaskWeightImpl(std::array const& addr) { } // namespace +AuthAllowedSubnet::AuthAllowedSubnet(IPAddress const& baseAddress, IPAddress const& addressMask) + : baseAddress(baseAddress), addressMask(addressMask) { + ASSERT(baseAddress.isV4() == addressMask.isV4()); +} + IPAddress AuthAllowedSubnet::netmask() const { if (addressMask.isV4()) { uint32_t res = 0xffffffff ^ addressMask.toV4(); @@ -110,6 +115,30 @@ void AuthAllowedSubnet::printIP(std::string_view txt, IPAddress const& address) fmt::print("\n"); } +template +std::array AuthAllowedSubnet::createBitMask(std::array const& addr, + int netmaskWeight) { + std::array res; + res.fill((unsigned char)0xff); + int idx = netmaskWeight / 8; + if (netmaskWeight % 8 > 0) { + // 2^(netmaskWeight % 8) - 1 sets the last (netmaskWeight % 8) number of bits to 1 + // everything else will be zero. For example: 2^3 - 1 == 7 == 0b111 + unsigned char bitmask = (1 << (8 - (netmaskWeight % 8))) - ((unsigned char)1); + res[idx] ^= bitmask; + ++idx; + } + for (; idx < res.size(); ++idx) { + res[idx] = (unsigned char)0; + } + return res; +} + +template std::array AuthAllowedSubnet::createBitMask<4>(const std::array& addr, + int netmaskWeight); +template std::array AuthAllowedSubnet::createBitMask<16>(const std::array& addr, + int netmaskWeight); + // helpers for testing namespace { using boost::asio::ip::address_v4; diff --git a/fdbrpc/IPAllowList.h b/fdbrpc/IPAllowList.h index b3a44e30fc..4be11a7f52 100644 --- a/fdbrpc/IPAllowList.h +++ b/fdbrpc/IPAllowList.h @@ -29,31 +29,12 @@ struct AuthAllowedSubnet { IPAddress baseAddress; IPAddress addressMask; - AuthAllowedSubnet(IPAddress const& baseAddress, IPAddress const& addressMask) - : baseAddress(baseAddress), addressMask(addressMask) { - ASSERT(baseAddress.isV4() == addressMask.isV4()); - } + AuthAllowedSubnet(IPAddress const& baseAddress, IPAddress const& addressMask); static AuthAllowedSubnet fromString(std::string_view addressString); template - static auto createBitMask(std::array const& addr, int netmaskWeight) - -> std::array { - std::array res; - res.fill((unsigned char)0xff); - int idx = netmaskWeight / 8; - if (netmaskWeight % 8 > 0) { - // 2^(netmaskWeight % 8) - 1 sets the last (netmaskWeight % 8) number of bits to 1 - // everything else will be zero. For example: 2^3 - 1 == 7 == 0b111 - unsigned char bitmask = (1 << (8 - (netmaskWeight % 8))) - ((unsigned char)1); - res[idx] ^= bitmask; - ++idx; - } - for (; idx < res.size(); ++idx) { - res[idx] = (unsigned char)0; - } - return res; - } + static std::array createBitMask(std::array const& addr, int netmaskWeight); bool operator()(IPAddress const& address) const { if (addressMask.isV4() != address.isV4()) { diff --git a/fdbserver/workloads/ClientWorkload.actor.cpp b/fdbserver/workloads/ClientWorkload.actor.cpp index 831825ed7e..803bdf04b7 100644 --- a/fdbserver/workloads/ClientWorkload.actor.cpp +++ b/fdbserver/workloads/ClientWorkload.actor.cpp @@ -56,13 +56,12 @@ class WorkloadProcessState { } else { self->childAddress = IPAddress::parse(fmt::format("192.168.0.{}", self->clientId + 2)).get(); } - //TraceEvent("TestClientStart", id).detail("ClusterFileLocation", child->ccr->getLocation()).log(); self->processName = fmt::format("TestClient{}", self->clientId); Standalone newZoneId(deterministicRandom()->randomUniqueID().toString()); auto locality = LocalityData(Optional>(), newZoneId, newZoneId, parent->locality.dcId()); auto dataFolder = joinPath(popPath(parent->dataFolder), deterministicRandom()->randomUniqueID().toString()); platform::createDirectory(dataFolder); - TraceEvent("StartingClientForWorkload", self->id) + TraceEvent("StartingClientWorkloadProcess", self->id) .detail("Name", self->processName) .detail("Address", self->childAddress); self->childProcess = g_simulator.newProcess(self->processName.c_str(), @@ -83,7 +82,7 @@ class WorkloadProcessState { auto addr = g_simulator.getCurrentProcess()->address; futures.push_back(FlowTransport::transport().bind(addr, addr)); futures.push_back(success((self->childProcess->onShutdown()))); - TraceEvent("WorkloadProcessInitialized", self->id).log(); + TraceEvent("ClientWorkloadProcessInitialized", self->id).log(); futures.push_back(initializationDone(self, parent)); wait(waitForAny(futures)); } catch (Error& e) { @@ -96,28 +95,19 @@ class WorkloadProcessState { return Void(); } - static std::vector>& states() { - static std::vector> res; + static std::vector& states() { + static std::vector res; return res; } public: static WorkloadProcessState* instance(int clientId) { - states().resize(std::max(states().size(), size_t(clientId + 1)), std::make_pair(nullptr, 0)); + states().resize(std::max(states().size(), size_t(clientId + 1)), nullptr); auto& res = states()[clientId]; - if (res.first == nullptr) { - res = std::make_pair(new WorkloadProcessState(clientId), 0); - } - ++res.second; - return res.first; - } - static void done(WorkloadProcessState* processState) { - auto& p = states()[processState->clientId]; - ASSERT(p.first == processState); - if (--p.second == 0) { - // delete p.first; - // p.first = nullptr; + if (res == nullptr) { + res = new WorkloadProcessState(clientId); } + return res; } Future initialized() const { return init.getFuture(); } @@ -201,7 +191,7 @@ struct WorkloadProcess { res = r; } catch (Error& e) { if (e.code() == error_code_actor_cancelled) { - ASSERT(false); + ASSERT(g_simulator.getCurrentProcess() == parent); throw; } err = e; @@ -212,8 +202,6 @@ struct WorkloadProcess { } return res; } - - ~WorkloadProcess() { WorkloadProcessState::done(processState); } }; ClientWorkload::ClientWorkload(CreateWorkload const& childCreator, WorkloadContext const& wcx) diff --git a/fdbserver/workloads/PrivateEndpoints.actor.cpp b/fdbserver/workloads/PrivateEndpoints.actor.cpp index 29abb389ed..5c27adf058 100644 --- a/fdbserver/workloads/PrivateEndpoints.actor.cpp +++ b/fdbserver/workloads/PrivateEndpoints.actor.cpp @@ -59,23 +59,20 @@ struct PrivateEndpoints : TestWorkload { T t = wait(f); (void)t; ASSERT(false); - return Void(); } catch (Error& e) { if (e.code() == error_code_actor_cancelled) { throw; } else if (e.code() == error_code_unauthorized_attempt) { TraceEvent("SuccessPrivateEndpoint").log(); - return Void(); } else if (e.code() == error_code_request_maybe_delivered) { // this is also fine, because even when calling private endpoints // we might see connection failures TraceEvent("SuccessRequestMaybeDelivered").log(); - return Void(); } else { TraceEvent(SevError, "WrongErrorCode").error(e); - return Void(); } } + return Void(); } template