Skip to content

Commit

Permalink
Migrate openr to std::optional (from folly::Optional)
Browse files Browse the repository at this point in the history 
Summary:
- switch all internal data structures to std::optional
- add converter utilities for external data types (used by thrift and socket dependencies)
- update constant references where necessary (std::nullopt instead of folly::none)

Reviewed By: saifhhasan

Differential Revision: D20413295

fbshipit-source-id: e15bbd4d90fcae296192d94253cc17631e40fc5f
  • Loading branch information
@facebook-github-bot
Alex Boyko authored and facebook-github-bot committed Mar 31, 2020
1 parent 1525230 commit 4f2111b
Show file tree
Hide file tree
Showing 63 changed files with 886 additions and 801 deletions.
2 changes: 1 addition & 1 deletion examples/KvStoreAgent.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ KvStoreAgent::KvStoreAgent(std::string nodeId, KvStore* kvStore)
// care about
kvStoreClient_->setKvCallback(
[this, nodeId](
const std::string& key, const folly::Optional<thrift::Value>& value) {
const std::string& key, const std::optional<thrift::Value>& value) {
if (0 == key.find(agentKeyPrefix) &&
value.value().originatorId != nodeId && value.value().value) {
// Lets check out what some other node's value is
Expand Down
4 changes: 2 additions & 2 deletions examples/KvStorePoller.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ KvStorePoller::KvStorePoller(std::vector<folly::SocketAddress>& sockAddrs)
: sockAddrs_(sockAddrs) {}

std::pair<
folly::Optional<std::unordered_map<std::string, thrift::AdjacencyDatabase>>,
std::optional<std::unordered_map<std::string, thrift::AdjacencyDatabase>>,
std::vector<fbzmq::SocketUrl> /* unreached url */>
KvStorePoller::getAdjacencyDatabases(std::chrono::milliseconds pollTimeout) {
return openr::dumpAllWithPrefixMultipleAndParse<thrift::AdjacencyDatabase>(
Expand All @@ -27,7 +27,7 @@ KvStorePoller::getAdjacencyDatabases(std::chrono::milliseconds pollTimeout) {
}

std::pair<
folly::Optional<std::unordered_map<std::string, thrift::PrefixDatabase>>,
std::optional<std::unordered_map<std::string, thrift::PrefixDatabase>>,
std::vector<fbzmq::SocketUrl> /* unreached url */>
KvStorePoller::getPrefixDatabases(std::chrono::milliseconds pollTimeout) {
return openr::dumpAllWithPrefixMultipleAndParse<thrift::PrefixDatabase>(
Expand Down
5 changes: 2 additions & 3 deletions examples/KvStorePoller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,12 @@ class KvStorePoller {
~KvStorePoller() {}

std::pair<
folly::Optional<
std::unordered_map<std::string, thrift::AdjacencyDatabase>>,
std::optional<std::unordered_map<std::string, thrift::AdjacencyDatabase>>,
std::vector<fbzmq::SocketUrl> /* unreached url */>
getAdjacencyDatabases(std::chrono::milliseconds pollTimeout);

std::pair<
folly::Optional<std::unordered_map<std::string, thrift::PrefixDatabase>>,
std::optional<std::unordered_map<std::string, thrift::PrefixDatabase>>,
std::vector<fbzmq::SocketUrl> /* unreached url */>
getPrefixDatabases(std::chrono::milliseconds pollTimeout);

Expand Down
4 changes: 2 additions & 2 deletions openr/Main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ main(int argc, char** argv) {
<< "Overlapping global/local segment routing label space.";

// Prepare IP-TOS value from flag and do sanity checks
folly::Optional<int> maybeIpTos{0};
std::optional<int> maybeIpTos{0};
if (FLAGS_ip_tos != 0) {
CHECK_LE(0, FLAGS_ip_tos) << "ip_tos must be greater than 0";
CHECK_GE(256, FLAGS_ip_tos) << "ip_tos must be less than 256";
Expand Down Expand Up @@ -712,7 +712,7 @@ main(int argc, char** argv) {
// SPF in Decision module. This is to make sure the Decision module
// receives itself as one of the nodes before running the spf.

folly::Optional<std::chrono::seconds> decisionGRWindow{folly::none};
std::optional<std::chrono::seconds> decisionGRWindow{std::nullopt};
if (FLAGS_decision_graceful_restart_window_s >= 0) {
decisionGRWindow =
std::chrono::seconds(FLAGS_decision_graceful_restart_window_s);
Expand Down
63 changes: 31 additions & 32 deletions openr/allocators/PrefixAllocator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ PrefixAllocator::operator()(PrefixAllocatorModeStatic const&) {
// subscribe for incremental updates of static prefix allocation key
kvStoreClient_->subscribeKey(
Constants::kStaticPrefixAllocParamKey.toString(),
[&](std::string const& key, folly::Optional<thrift::Value> value) {
[&](std::string const& key, std::optional<thrift::Value> value) {
CHECK_EQ(Constants::kStaticPrefixAllocParamKey.toString(), key);
if (value.has_value()) {
processStaticPrefixAllocUpdate(value.value());
Expand Down Expand Up @@ -128,7 +128,7 @@ PrefixAllocator::operator()(PrefixAllocatorModeStatic const&) {
LOG(WARNING)
<< "Clearing previous prefix allocation state on failure to load "
<< "allocation parameters from disk as well as KvStore.";
applyState_ = std::make_pair(true, folly::none);
applyState_ = std::make_pair(true, std::nullopt);
applyMyPrefix();
return;
}
Expand All @@ -141,7 +141,7 @@ PrefixAllocator::operator()(PrefixAllocatorModeSeeded const&) {
// subscribe for incremental updates of seed prefix
kvStoreClient_->subscribeKey(
Constants::kSeedPrefixAllocParamKey.toString(),
[&](std::string const& key, folly::Optional<thrift::Value> value) {
[&](std::string const& key, std::optional<thrift::Value> value) {
CHECK_EQ(Constants::kSeedPrefixAllocParamKey.toString(), key);
if (value.has_value()) {
processAllocParamUpdate(value.value());
Expand All @@ -152,7 +152,7 @@ PrefixAllocator::operator()(PrefixAllocatorModeSeeded const&) {

kvStoreClient_->subscribeKey(
Constants::kStaticPrefixAllocParamKey.toString(),
[&](std::string const& key, folly::Optional<thrift::Value> value) {
[&](std::string const& key, std::optional<thrift::Value> value) {
CHECK_EQ(Constants::kStaticPrefixAllocParamKey.toString(), key);
if (value.has_value()) {
processNetworkAllocationsUpdate(value.value());
Expand Down Expand Up @@ -201,7 +201,7 @@ PrefixAllocator::operator()(PrefixAllocatorModeSeeded const&) {
LOG(WARNING)
<< "Clearing previous prefix allocation state on failure to load "
<< "allocation parameters from disk as well as KvStore.";
applyState_ = std::make_pair(true, folly::none);
applyState_ = std::make_pair(true, std::nullopt);
applyMyPrefix();
return;
}
Expand All @@ -224,14 +224,14 @@ PrefixAllocator::operator()(PrefixAllocatorParams const& allocParams) {
initTimer_->scheduleTimeout(0ms);
}

folly::Optional<uint32_t>
std::optional<uint32_t>
PrefixAllocator::getMyPrefixIndex() {
if (getEvb()->isInEventBaseThread()) {
return myPrefixIndex_;
}

// Otherwise enqueue request in eventloop and wait for result to be populated
folly::Promise<folly::Optional<uint32_t>> promise;
folly::Promise<std::optional<uint32_t>> promise;
auto future = promise.getFuture();
runInEventBaseThread([this, promise = std::move(promise)]() mutable {
promise.setValue(myPrefixIndex_);
Expand Down Expand Up @@ -288,8 +288,8 @@ PrefixAllocator::processStaticPrefixAllocUpdate(thrift::Value const& value) {
LOG(ERROR) << "Error parsing static prefix allocation value. Error: "
<< folly::exceptionStr(e);
if (allocParams_) {
applyMyPrefixIndex(folly::none);
allocParams_ = folly::none;
applyMyPrefixIndex(std::nullopt);
allocParams_ = std::nullopt;
}
return;
}
Expand All @@ -301,9 +301,9 @@ PrefixAllocator::processStaticPrefixAllocUpdate(thrift::Value const& value) {
// Withdraw my prefix if not found
if (myPrefixIt == staticAlloc.nodePrefixes.end() and allocParams_) {
VLOG(2) << "Lost prefix";
applyState_ = std::make_pair(true, folly::none);
applyMyPrefixIndex(folly::none);
allocParams_ = folly::none;
applyState_ = std::make_pair(true, std::nullopt);
applyMyPrefixIndex(std::nullopt);
allocParams_ = std::nullopt;
}

// Advertise my prefix if found
Expand All @@ -315,7 +315,7 @@ PrefixAllocator::processStaticPrefixAllocUpdate(thrift::Value const& value) {
LOG(INFO) << "New and old params are same. Skipping";
} else if (allocParams_.has_value()) {
// withdraw old prefix
applyMyPrefixIndex(folly::none);
applyMyPrefixIndex(std::nullopt);
}
// create alloc params so that we share same workflow as of SEEDED mode
// seedPrefix length is same as alloc prefix length and my prefix index is 0
Expand All @@ -330,7 +330,7 @@ PrefixAllocator::processAllocParamUpdate(thrift::Value const& value) {
auto maybeParams = parseParamsStr(value.value.value());
if (maybeParams.hasError()) {
LOG(ERROR) << "Malformed prefix-allocator params. " << maybeParams.error();
startAllocation(folly::none);
startAllocation(std::nullopt);
} else {
startAllocation(maybeParams.value());
}
Expand Down Expand Up @@ -398,7 +398,7 @@ PrefixAllocator::getPrefixCount(
return (1 << std::min(31, allocPrefixLen - seedPrefix.second));
}

folly::Optional<uint32_t>
std::optional<uint32_t>
PrefixAllocator::loadPrefixIndexFromKvStore() {
VLOG(4) << "See if I am already allocated a prefix in kvstore";

Expand All @@ -407,18 +407,18 @@ PrefixAllocator::loadPrefixIndexFromKvStore() {
return rangeAllocator_->getValueFromKvStore();
}

folly::Optional<uint32_t>
std::optional<uint32_t>
PrefixAllocator::loadPrefixIndexFromDisk() {
auto maybeThriftAllocPrefix =
configStore_->loadThriftObj<thrift::AllocPrefix>(kConfigKey).get();
if (maybeThriftAllocPrefix.hasError()) {
return folly::none;
return std::nullopt;
}
return static_cast<uint32_t>(maybeThriftAllocPrefix->allocPrefixIndex);
}

void
PrefixAllocator::savePrefixIndexToDisk(folly::Optional<uint32_t> prefixIndex) {
PrefixAllocator::savePrefixIndexToDisk(std::optional<uint32_t> prefixIndex) {
CHECK(allocParams_.has_value());

if (!prefixIndex) {
Expand Down Expand Up @@ -476,8 +476,7 @@ PrefixAllocator::getInitPrefixIndex() {

void
PrefixAllocator::startAllocation(
folly::Optional<PrefixAllocatorParams> const& allocParams,
bool checkParams) {
std::optional<PrefixAllocatorParams> const& allocParams, bool checkParams) {
// Some informative logging
if (allocParams_.has_value() and allocParams.has_value()) {
if (checkParams and allocParams_ == allocParams) {
Expand Down Expand Up @@ -506,15 +505,15 @@ PrefixAllocator::startAllocation(
}
logPrefixEvent(
"ALLOC_PARAMS_UPDATE",
folly::none,
folly::none,
std::nullopt,
std::nullopt,
allocParams_ /* old params */,
allocParams /* new params */);

// Update local state
rangeAllocator_.reset();
if (allocParams_) {
applyMyPrefixIndex(folly::none); // Clear local state
applyMyPrefixIndex(std::nullopt); // Clear local state
}
CHECK(!myPrefixIndex_.has_value());
allocParams_ = allocParams;
Expand All @@ -528,7 +527,7 @@ PrefixAllocator::startAllocation(
myNodeName_,
allocPrefixMarker_,
kvStoreClient_.get(),
[this](folly::Optional<uint32_t> newPrefixIndex) noexcept {
[this](std::optional<uint32_t> newPrefixIndex) noexcept {
applyMyPrefixIndex(newPrefixIndex);
},
syncInterval_,
Expand Down Expand Up @@ -563,7 +562,7 @@ PrefixAllocator::startAllocation(
}

void
PrefixAllocator::applyMyPrefixIndex(folly::Optional<uint32_t> prefixIndex) {
PrefixAllocator::applyMyPrefixIndex(std::optional<uint32_t> prefixIndex) {
// Silently return if nothing changed. For e.g.
if (myPrefixIndex_ == prefixIndex) {
return;
Expand All @@ -576,21 +575,21 @@ PrefixAllocator::applyMyPrefixIndex(folly::Optional<uint32_t> prefixIndex) {

if (prefixIndex and !myPrefixIndex_) {
LOG(INFO) << "Elected new prefixIndex " << *prefixIndex;
logPrefixEvent("PREFIX_ELECTED", folly::none, prefixIndex);
logPrefixEvent("PREFIX_ELECTED", std::nullopt, prefixIndex);
} else if (prefixIndex and myPrefixIndex_) {
LOG(INFO) << "Updating prefixIndex to " << *prefixIndex << " from "
<< *myPrefixIndex_;
logPrefixEvent("PREFIX_UPDATED", myPrefixIndex_, prefixIndex);
} else if (myPrefixIndex_) {
LOG(INFO) << "Lost previously allocated prefixIndex " << *myPrefixIndex_;
logPrefixEvent("PREFIX_LOST", myPrefixIndex_, folly::none);
logPrefixEvent("PREFIX_LOST", myPrefixIndex_, std::nullopt);
}

// Set local state
myPrefixIndex_ = prefixIndex;

// Create network prefix to announce and loopback address to assign
folly::Optional<folly::CIDRNetwork> prefix = folly::none;
std::optional<folly::CIDRNetwork> prefix = std::nullopt;
if (prefixIndex) {
auto const& seedPrefix = allocParams_->first;
auto const& allocPrefixLen = allocParams_->second;
Expand Down Expand Up @@ -789,10 +788,10 @@ PrefixAllocator::delIfaceAddr(
void
PrefixAllocator::logPrefixEvent(
std::string event,
folly::Optional<uint32_t> oldPrefix,
folly::Optional<uint32_t> newPrefix,
folly::Optional<PrefixAllocatorParams> const& oldAllocParams,
folly::Optional<PrefixAllocatorParams> const& newAllocParams) {
std::optional<uint32_t> oldPrefix,
std::optional<uint32_t> newPrefix,
std::optional<PrefixAllocatorParams> const& oldAllocParams,
std::optional<PrefixAllocatorParams> const& newAllocParams) {
fbzmq::LogSample sample{};

sample.addString("event", event);
Expand Down
28 changes: 14 additions & 14 deletions openr/allocators/PrefixAllocator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class PrefixAllocator : public OpenrEventBase {
void operator()(PrefixAllocatorParams const&);

// Thread safe API for testing only
folly::Optional<uint32_t> getMyPrefixIndex();
std::optional<uint32_t> getMyPrefixIndex();

// Static function to parse string representation of allocation params to
// strong types.
Expand Down Expand Up @@ -124,26 +124,26 @@ class PrefixAllocator : public OpenrEventBase {
bool checkE2eAllocIndex(uint32_t index);

// get my existing prefix index from kvstore if it's present
folly::Optional<uint32_t> loadPrefixIndexFromKvStore();
std::optional<uint32_t> loadPrefixIndexFromKvStore();

// load prefix index from disk
folly::Optional<uint32_t> loadPrefixIndexFromDisk();
std::optional<uint32_t> loadPrefixIndexFromDisk();

// save newly elected prefix index to disk
void savePrefixIndexToDisk(folly::Optional<uint32_t> prefixIndex);
void savePrefixIndexToDisk(std::optional<uint32_t> prefixIndex);

// initialize my prefix
uint32_t getInitPrefixIndex();

// start allocating prefixes, can be called again with new prefix
// or `folly::none` if seed prefix is no longer valid to withdraw
// or `std::nullopt` if seed prefix is no longer valid to withdraw
// what we had before!
void startAllocation(
folly::Optional<PrefixAllocatorParams> const& allocParams,
std::optional<PrefixAllocatorParams> const& allocParams,
bool checkParams = true);

// use my newly allocated prefix
void applyMyPrefixIndex(folly::Optional<uint32_t> prefixIndex);
void applyMyPrefixIndex(std::optional<uint32_t> prefixIndex);
void applyMyPrefix();

// update prefix
Expand All @@ -154,10 +154,10 @@ class PrefixAllocator : public OpenrEventBase {

void logPrefixEvent(
std::string event,
folly::Optional<uint32_t> oldPrefix,
folly::Optional<uint32_t> newPrefix,
folly::Optional<PrefixAllocatorParams> const& oldParams = folly::none,
folly::Optional<PrefixAllocatorParams> const& newParams = folly::none);
std::optional<uint32_t> oldPrefix,
std::optional<uint32_t> newPrefix,
std::optional<PrefixAllocatorParams> const& oldParams = std::nullopt,
std::optional<PrefixAllocatorParams> const& newParams = std::nullopt);

void syncIfaceAddrs(
const std::string& ifName,
Expand Down Expand Up @@ -208,10 +208,10 @@ class PrefixAllocator : public OpenrEventBase {
//

// Allocation parameters e.g., fc00:cafe::/56, 64
folly::Optional<PrefixAllocatorParams> allocParams_;
std::optional<PrefixAllocatorParams> allocParams_;

// index of my currently claimed prefix within seed prefix
folly::Optional<uint32_t> myPrefixIndex_;
std::optional<uint32_t> myPrefixIndex_;

apache::thrift::CompactSerializer serializer_;

Expand Down Expand Up @@ -249,7 +249,7 @@ class PrefixAllocator : public OpenrEventBase {
* When Optional value is empty, it means cleanup addresses on the iface
* otherwise applys the Optional value to the iface
*/
std::pair<bool, folly::Optional<folly::CIDRNetwork>> applyState_;
std::pair<bool, std::optional<folly::CIDRNetwork>> applyState_;

// save alloc index from e2e-network-alllocation <value version, indices set>
std::pair<int64_t, std::unordered_set<uint32_t>> e2eAllocIndex_{-1, {}};
Expand Down
Loading

0 comments on commit 4f2111b

Please sign in to comment.