Improve ConfigurationResult and CoordinatorsResult type safety

This commit is contained in:
sfc-gh-tclinkenbeard 2020-09-27 11:52:18 -07:00
parent 8d9e7adac7
commit 6c726ba8dd
7 changed files with 83 additions and 61 deletions

View File

@ -1807,7 +1807,7 @@ ACTOR Future<Void> commitTransaction( Reference<ReadYourWritesTransaction> tr )
}
ACTOR Future<bool> configure( Database db, std::vector<StringRef> tokens, Reference<ClusterConnectionFile> ccf, LineNoise* linenoise, Future<Void> warn ) {
state ConfigurationResult::Type result;
state ConfigurationResult result;
state int startToken = 1;
state bool force = false;
if (tokens.size() < 2)
@ -1888,7 +1888,8 @@ ACTOR Future<bool> configure( Database db, std::vector<StringRef> tokens, Refere
}
}
ConfigurationResult::Type r = wait( makeInterruptable( changeConfig( db, std::vector<StringRef>(tokens.begin()+startToken,tokens.end()), conf, force) ) );
ConfigurationResult r = wait(makeInterruptable(
changeConfig(db, std::vector<StringRef>(tokens.begin() + startToken, tokens.end()), conf, force)));
result = r;
}
@ -2014,7 +2015,7 @@ ACTOR Future<bool> fileConfigure(Database db, std::string filePath, bool isNewDa
return true;
}
}
ConfigurationResult::Type result = wait( makeInterruptable( changeConfig(db, configString, force) ) );
ConfigurationResult result = wait(makeInterruptable(changeConfig(db, configString, force)));
// Real errors get thrown from makeInterruptable and printed by the catch block in cli(), but
// there are various results specific to changeConfig() that we need to report:
bool ret;
@ -2145,7 +2146,7 @@ ACTOR Future<bool> coordinators( Database db, std::vector<StringRef> tokens, boo
}
if(setName.size()) change = nameQuorumChange( setName.toString(), change );
CoordinatorsResult::Type r = wait( makeInterruptable( changeQuorum( db, change ) ) );
CoordinatorsResult r = wait(makeInterruptable(changeQuorum(db, change)));
// Real errors get thrown from makeInterruptable and printed by the catch block in cli(), but
// there are various results specific to changeConfig() that we need to report:

View File

@ -259,7 +259,8 @@ std::map<std::string, std::string> configForToken( std::string const& mode ) {
return out;
}
ConfigurationResult::Type buildConfiguration( std::vector<StringRef> const& modeTokens, std::map<std::string, std::string>& outConf ) {
ConfigurationResult buildConfiguration(std::vector<StringRef> const& modeTokens,
std::map<std::string, std::string>& outConf) {
for(auto it : modeTokens) {
std::string mode = it.toString();
auto m = configForToken( mode );
@ -295,7 +296,7 @@ ConfigurationResult::Type buildConfiguration( std::vector<StringRef> const& mode
return ConfigurationResult::SUCCESS;
}
ConfigurationResult::Type buildConfiguration( std::string const& configMode, std::map<std::string, std::string>& outConf ) {
ConfigurationResult buildConfiguration(std::string const& configMode, std::map<std::string, std::string>& outConf) {
std::vector<StringRef> modes;
int p = 0;
@ -335,7 +336,7 @@ ACTOR Future<DatabaseConfiguration> getDatabaseConfiguration( Database cx ) {
}
}
ACTOR Future<ConfigurationResult::Type> changeConfig( Database cx, std::map<std::string, std::string> m, bool force ) {
ACTOR Future<ConfigurationResult> changeConfig(Database cx, std::map<std::string, std::string> m, bool force) {
state StringRef initIdKey = LiteralStringRef( "\xff/init_id" );
state Transaction tr(cx);
@ -852,7 +853,7 @@ ConfigureAutoResult parseConfig( StatusObject const& status ) {
return result;
}
ACTOR Future<ConfigurationResult::Type> autoConfig( Database cx, ConfigureAutoResult conf ) {
ACTOR Future<ConfigurationResult> autoConfig(Database cx, ConfigureAutoResult conf) {
state Transaction tr(cx);
state Key versionKey = BinaryWriter::toValue(deterministicRandom()->randomUniqueID(),Unversioned());
@ -919,7 +920,8 @@ ACTOR Future<ConfigurationResult::Type> autoConfig( Database cx, ConfigureAutoRe
}
}
Future<ConfigurationResult::Type> changeConfig( Database const& cx, std::vector<StringRef> const& modes, Optional<ConfigureAutoResult> const& conf, bool force ) {
Future<ConfigurationResult> changeConfig(Database const& cx, std::vector<StringRef> const& modes,
Optional<ConfigureAutoResult> const& conf, bool force) {
if( modes.size() && modes[0] == LiteralStringRef("auto") && conf.present() ) {
return autoConfig(cx, conf.get());
}
@ -931,7 +933,7 @@ Future<ConfigurationResult::Type> changeConfig( Database const& cx, std::vector<
return changeConfig(cx, m, force);
}
Future<ConfigurationResult::Type> changeConfig( Database const& cx, std::string const& modes, bool force ) {
Future<ConfigurationResult> changeConfig(Database const& cx, std::string const& modes, bool force) {
TraceEvent("ChangeConfig").detail("Mode", modes);
std::map<std::string,std::string> m;
auto r = buildConfiguration( modes, m );
@ -1000,7 +1002,7 @@ ACTOR Future<std::vector<NetworkAddress>> getCoordinators( Database cx ) {
}
}
ACTOR Future<CoordinatorsResult::Type> changeQuorum( Database cx, Reference<IQuorumChange> change ) {
ACTOR Future<CoordinatorsResult> changeQuorum(Database cx, Reference<IQuorumChange> change) {
state Transaction tr(cx);
state int retries = 0;
state std::vector<NetworkAddress> desiredCoordinators;
@ -1020,7 +1022,7 @@ ACTOR Future<CoordinatorsResult::Type> changeQuorum( Database cx, Reference<IQuo
if ( cx->getConnectionFile() && old.clusterKeyName().toString() != cx->getConnectionFile()->getConnectionString().clusterKeyName() )
return CoordinatorsResult::BAD_DATABASE_STATE; // Someone changed the "name" of the database??
state CoordinatorsResult::Type result = CoordinatorsResult::SUCCESS;
state CoordinatorsResult result = CoordinatorsResult::SUCCESS;
if(!desiredCoordinators.size()) {
std::vector<NetworkAddress> _desiredCoordinators = wait( change->getDesiredCoordinators( &tr, old.coordinators(), Reference<ClusterConnectionFile>(new ClusterConnectionFile(old)), result ) );
desiredCoordinators = _desiredCoordinators;
@ -1090,14 +1092,20 @@ ACTOR Future<CoordinatorsResult::Type> changeQuorum( Database cx, Reference<IQuo
struct SpecifiedQuorumChange : IQuorumChange {
vector<NetworkAddress> desired;
explicit SpecifiedQuorumChange( vector<NetworkAddress> const& desired ) : desired(desired) {}
virtual Future<vector<NetworkAddress>> getDesiredCoordinators( Transaction* tr, vector<NetworkAddress> oldCoordinators, Reference<ClusterConnectionFile>, CoordinatorsResult::Type& ) {
virtual Future<vector<NetworkAddress>> getDesiredCoordinators(Transaction* tr,
vector<NetworkAddress> oldCoordinators,
Reference<ClusterConnectionFile>,
CoordinatorsResult&) {
return desired;
}
};
Reference<IQuorumChange> specifiedQuorumChange(vector<NetworkAddress> const& addresses) { return Reference<IQuorumChange>(new SpecifiedQuorumChange(addresses)); }
struct NoQuorumChange : IQuorumChange {
virtual Future<vector<NetworkAddress>> getDesiredCoordinators( Transaction* tr, vector<NetworkAddress> oldCoordinators, Reference<ClusterConnectionFile>, CoordinatorsResult::Type& ) {
virtual Future<vector<NetworkAddress>> getDesiredCoordinators(Transaction* tr,
vector<NetworkAddress> oldCoordinators,
Reference<ClusterConnectionFile>,
CoordinatorsResult&) {
return oldCoordinators;
}
};
@ -1107,7 +1115,10 @@ struct NameQuorumChange : IQuorumChange {
std::string newName;
Reference<IQuorumChange> otherChange;
explicit NameQuorumChange( std::string const& newName, Reference<IQuorumChange> const& otherChange ) : newName(newName), otherChange(otherChange) {}
virtual Future<vector<NetworkAddress>> getDesiredCoordinators( Transaction* tr, vector<NetworkAddress> oldCoordinators, Reference<ClusterConnectionFile> cf, CoordinatorsResult::Type& t ) {
virtual Future<vector<NetworkAddress>> getDesiredCoordinators(Transaction* tr,
vector<NetworkAddress> oldCoordinators,
Reference<ClusterConnectionFile> cf,
CoordinatorsResult& t) {
return otherChange->getDesiredCoordinators(tr, oldCoordinators, cf, t);
}
virtual std::string getDesiredClusterKeyName() {
@ -1122,7 +1133,10 @@ struct AutoQuorumChange : IQuorumChange {
int desired;
explicit AutoQuorumChange( int desired ) : desired(desired) {}
virtual Future<vector<NetworkAddress>> getDesiredCoordinators( Transaction* tr, vector<NetworkAddress> oldCoordinators, Reference<ClusterConnectionFile> ccf, CoordinatorsResult::Type& err ) {
virtual Future<vector<NetworkAddress>> getDesiredCoordinators(Transaction* tr,
vector<NetworkAddress> oldCoordinators,
Reference<ClusterConnectionFile> ccf,
CoordinatorsResult& err) {
return getDesired( this, tr, oldCoordinators, ccf, &err );
}
@ -1174,7 +1188,10 @@ struct AutoQuorumChange : IQuorumChange {
return true; // The status quo seems fine
}
ACTOR static Future<vector<NetworkAddress>> getDesired( AutoQuorumChange* self, Transaction* tr, vector<NetworkAddress> oldCoordinators, Reference<ClusterConnectionFile> ccf, CoordinatorsResult::Type* err ) {
ACTOR static Future<vector<NetworkAddress>> getDesired(AutoQuorumChange* self, Transaction* tr,
vector<NetworkAddress> oldCoordinators,
Reference<ClusterConnectionFile> ccf,
CoordinatorsResult* err) {
state int desiredCount = self->desired;
if(desiredCount == -1) {

View File

@ -43,41 +43,35 @@ standard API and some knowledge of the contents of the system key space.
// ConfigurationResult enumerates normal outcomes of changeConfig() and various error
// conditions specific to it. changeConfig may also throw an Error to report other problems.
class ConfigurationResult {
public:
enum Type {
NO_OPTIONS_PROVIDED,
CONFLICTING_OPTIONS,
UNKNOWN_OPTION,
INCOMPLETE_CONFIGURATION,
INVALID_CONFIGURATION,
DATABASE_ALREADY_CREATED,
DATABASE_CREATED,
DATABASE_UNAVAILABLE,
STORAGE_IN_UNKNOWN_DCID,
REGION_NOT_FULLY_REPLICATED,
MULTIPLE_ACTIVE_REGIONS,
REGIONS_CHANGED,
NOT_ENOUGH_WORKERS,
REGION_REPLICATION_MISMATCH,
DCID_MISSING,
LOCKED_NOT_NEW,
SUCCESS,
};
enum class ConfigurationResult {
NO_OPTIONS_PROVIDED,
CONFLICTING_OPTIONS,
UNKNOWN_OPTION,
INCOMPLETE_CONFIGURATION,
INVALID_CONFIGURATION,
DATABASE_ALREADY_CREATED,
DATABASE_CREATED,
DATABASE_UNAVAILABLE,
STORAGE_IN_UNKNOWN_DCID,
REGION_NOT_FULLY_REPLICATED,
MULTIPLE_ACTIVE_REGIONS,
REGIONS_CHANGED,
NOT_ENOUGH_WORKERS,
REGION_REPLICATION_MISMATCH,
DCID_MISSING,
LOCKED_NOT_NEW,
SUCCESS,
};
class CoordinatorsResult {
public:
enum Type {
INVALID_NETWORK_ADDRESSES,
SAME_NETWORK_ADDRESSES,
NOT_COORDINATORS, //FIXME: not detected
DATABASE_UNREACHABLE, //FIXME: not detected
BAD_DATABASE_STATE,
COORDINATOR_UNREACHABLE,
NOT_ENOUGH_MACHINES,
SUCCESS
};
enum class CoordinatorsResult {
INVALID_NETWORK_ADDRESSES,
SAME_NETWORK_ADDRESSES,
NOT_COORDINATORS, // FIXME: not detected
DATABASE_UNREACHABLE, // FIXME: not detected
BAD_DATABASE_STATE,
COORDINATOR_UNREACHABLE,
NOT_ENOUGH_MACHINES,
SUCCESS
};
struct ConfigureAutoResult {
@ -116,17 +110,24 @@ struct ConfigureAutoResult {
bool isValid() const { return processes != -1; }
};
ConfigurationResult::Type buildConfiguration( std::vector<StringRef> const& modeTokens, std::map<std::string, std::string>& outConf ); // Accepts a vector of configuration tokens
ConfigurationResult::Type buildConfiguration( std::string const& modeString, std::map<std::string, std::string>& outConf ); // Accepts tokens separated by spaces in a single string
ConfigurationResult buildConfiguration(
std::vector<StringRef> const& modeTokens,
std::map<std::string, std::string>& outConf); // Accepts a vector of configuration tokens
ConfigurationResult buildConfiguration(
std::string const& modeString,
std::map<std::string, std::string>& outConf); // Accepts tokens separated by spaces in a single string
bool isCompleteConfiguration( std::map<std::string, std::string> const& options );
// All versions of changeConfig apply the given set of configuration tokens to the database, and return a ConfigurationResult (or error).
Future<ConfigurationResult::Type> changeConfig( Database const& cx, std::string const& configMode, bool force ); // Accepts tokens separated by spaces in a single string
Future<ConfigurationResult> changeConfig(Database const& cx, std::string const& configMode,
bool force); // Accepts tokens separated by spaces in a single string
ConfigureAutoResult parseConfig( StatusObject const& status );
Future<ConfigurationResult::Type> changeConfig( Database const& cx, std::vector<StringRef> const& modes, Optional<ConfigureAutoResult> const& conf, bool force ); // Accepts a vector of configuration tokens
ACTOR Future<ConfigurationResult::Type> changeConfig(
Future<ConfigurationResult> changeConfig(Database const& cx, std::vector<StringRef> const& modes,
Optional<ConfigureAutoResult> const& conf,
bool force); // Accepts a vector of configuration tokens
ACTOR Future<ConfigurationResult> changeConfig(
Database cx, std::map<std::string, std::string> m,
bool force); // Accepts a full configuration in key/value format (from buildConfiguration)
@ -135,12 +136,15 @@ ACTOR Future<Void> waitForFullReplication(Database cx);
struct IQuorumChange : ReferenceCounted<IQuorumChange> {
virtual ~IQuorumChange() {}
virtual Future<vector<NetworkAddress>> getDesiredCoordinators( Transaction* tr, vector<NetworkAddress> oldCoordinators, Reference<ClusterConnectionFile>, CoordinatorsResult::Type& ) = 0;
virtual Future<vector<NetworkAddress>> getDesiredCoordinators(Transaction* tr,
vector<NetworkAddress> oldCoordinators,
Reference<ClusterConnectionFile>,
CoordinatorsResult&) = 0;
virtual std::string getDesiredClusterKeyName() { return std::string(); }
};
// Change to use the given set of coordination servers
ACTOR Future<CoordinatorsResult::Type> changeQuorum(Database cx, Reference<IQuorumChange> change);
ACTOR Future<CoordinatorsResult> changeQuorum(Database cx, Reference<IQuorumChange> change);
Reference<IQuorumChange> autoQuorumChange(int desired = -1);
Reference<IQuorumChange> noQuorumChange();
Reference<IQuorumChange> specifiedQuorumChange(vector<NetworkAddress> const&);

View File

@ -715,7 +715,7 @@ void SimulationConfig::set_config(std::string config) {
// The only mechanism we have for turning "single" into what single means
// is buildConfiguration()... :/
std::map<std::string, std::string> hack_map;
ASSERT( buildConfiguration(config, hack_map) );
ASSERT(buildConfiguration(config, hack_map) != ConfigurationResult::NO_OPTIONS_PROVIDED);
for(auto kv : hack_map) db.set( kv.first, kv.second );
}

View File

@ -243,7 +243,7 @@ struct ConfigureDatabaseWorkload : TestWorkload {
return StringRef(format("DestroyDB%d", dbIndex));
}
static Future<ConfigurationResult::Type> IssueConfigurationChange( Database cx, const std::string& config, bool force ) {
static Future<ConfigurationResult> IssueConfigurationChange(Database cx, const std::string& config, bool force) {
printf("Issuing configuration change: %s\n", config.c_str());
return changeConfig(cx, config, force);
}

View File

@ -549,7 +549,7 @@ struct RemoveServersSafelyWorkload : TestWorkload {
while (true) {
cycle ++;
nQuorum = ((g_simulator.desiredCoordinators+1)/2)*2-1;
CoordinatorsResult::Type result = wait( changeQuorum( cx, autoQuorumChange(nQuorum) ) );
CoordinatorsResult result = wait(changeQuorum(cx, autoQuorumChange(nQuorum)));
TraceEvent(result==CoordinatorsResult::SUCCESS || result==CoordinatorsResult::SAME_NETWORK_ADDRESSES ? SevInfo : SevWarn, "RemoveAndKillQuorumChangeResult").detail("Step", "coordinators auto").detail("Result", (int)result).detail("Attempt", cycle).detail("Quorum", nQuorum).detail("DesiredCoordinators", g_simulator.desiredCoordinators);
if (result==CoordinatorsResult::SUCCESS || result==CoordinatorsResult::SAME_NETWORK_ADDRESSES)
break;

View File

@ -71,7 +71,7 @@ struct TriggerRecoveryLoopWorkload : TestWorkload {
state StringRef configStr(format("resolvers=%d", numResolversToSet));
loop {
Optional<ConfigureAutoResult> conf;
ConfigurationResult::Type r = wait(changeConfig(cx, { configStr }, conf, true));
ConfigurationResult r = wait(changeConfig(cx, { configStr }, conf, true));
if (r == ConfigurationResult::SUCCESS) {
self->currentNumOfResolvers = numResolversToSet;
TraceEvent(SevInfo, "TriggerRecoveryLoop_ChangeResolverConfigSuccess").detail("NumOfResolvers", self->currentNumOfResolvers.get());