Address code review comments.

* Fixed memory corruption with SystemData key constants
* Removed duplication in ClusterController
* Reworked fdbcli actions to better represent explicit vs default assignments
This commit is contained in:
Alex Miller 2017-10-03 20:57:39 -07:00
parent 80fa597422
commit e55cc447d2
3 changed files with 44 additions and 43 deletions

View File

@ -18,6 +18,7 @@
* limitations under the License.
*/
#include "boost/lexical_cast.hpp"
#include "fdbclient/NativeAPI.h"
#include "fdbclient/Status.h"
#include "fdbclient/StatusClient.h"
@ -551,9 +552,9 @@ void initHelp() {
"attempts to kill one or more processes in the cluster",
"If no addresses are specified, populates the list of processes which can be killed. Processes cannot be killed before this list has been populated.\n\nIf `all' is specified, attempts to kill all known processes.\n\nIf `list' is specified, displays all known processes. This is only useful when the database is unresponsive.\n\nFor each IP:port pair in <ADDRESS>*, attempt to kill the specified process.");
helpMap["profile"] = CommandHelp(
"<type> <on|off|status> <ARGS>",
"<type> <action> <ARGS>",
"toggles the profiling state for a form of profiling.",
"Supported types are: client.\n`on` and `off` enable and disable the selected type of profiling, and may require additional arguments. `status` shows if the selected profiling type is currently active.\n\nprofile client on <RATE> <SIZE>\n\tRATE is a double between 0 and 1 inclusive.\n\tSIZE is a unit suffixed value of bytes of samples to maintain in the database. Defaults to 100MB.\n");
"Supported types are: client. Different types support different actions. Run `profile` to get a list of types, and iteratively explore the help.\n");
hiddenCommands.insert("expensive_data_check");
hiddenCommands.insert("datadistribution");
@ -2588,42 +2589,59 @@ ACTOR Future<int> cli(CLIOptions opt, LineNoise* plinenoise) {
if (tokencmp(tokens[0], "profile")) {
if (tokens.size() == 1) {
printf("Supported profiling types are: client\n");
printf("ERROR: Usage: profile <client>\n");
is_error = true;
continue;
}
if (tokencmp(tokens[1], "client")) {
getTransaction(db, tr, options, intrans);
tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
if (tokens.size() == 2) {
printf("ERROR: on, off, or status missing.\n");
printf("ERROR: Usage: profile client <get|set>\n");
is_error = true;
continue;
}
if (tokencmp(tokens[2], "status")) {
if (tokencmp(tokens[2], "get")) {
if (tokens.size() != 3) {
printUsage(tokens[0]);
printf("ERROR: Addtional arguments to `get` are not supported.\n");
is_error = true;
continue;
}
Optional<Standalone<StringRef>> sampleRate = wait(makeInterruptable(tr->get(fdbClientInfoTxnSampleRate)));
const char* status = NULL;
if (sampleRate.present() && !std::isinf(BinaryReader::fromStringRef<double>(sampleRate.get(), Unversioned()))) {
status = "on";
} else {
status = "off";
state Future<Optional<Standalone<StringRef>>> sampleRateFuture = tr->get(fdbClientInfoTxnSampleRate);
state Future<Optional<Standalone<StringRef>>> sizeLimitFuture = tr->get(fdbClientInfoTxnSizeLimit);
Void _ = wait(makeInterruptable(success(sampleRateFuture) && success(sizeLimitFuture)));
std::string sampleRateStr = "default", sizeLimitStr = "default";
if (sampleRateFuture.get().present()) {
const double sampleRateDbl = BinaryReader::fromStringRef<double>(sampleRateFuture.get().get(), Unversioned());
if (!std::isinf(sampleRateDbl)) {
sampleRateStr = boost::lexical_cast<std::string>(sampleRateDbl);
}
}
printf("Client profiling is %s.\n", status);
if (sizeLimitFuture.get().present()) {
const int64_t sizeLimit = BinaryReader::fromStringRef<int64_t>(sizeLimitFuture.get().get(), Unversioned());
if (sizeLimit != -1) {
sizeLimitStr = boost::lexical_cast<std::string>(sizeLimitStr);
}
}
printf("Client profiling rate is set to %s and size limit is set to %s.\n", sampleRateStr.c_str(), sizeLimitStr.c_str());
continue;
}
if (tokencmp(tokens[2], "on")) {
if (tokens.size() < 4 || tokens.size() > 5) {
printUsage(tokens[0]);
if (tokencmp(tokens[2], "set")) {
if (tokens.size() != 5) {
printf("ERROR: Usage: profile client set <RATE|default> <SIZE|default>\n");
is_error = true;
continue;
}
double sampleRate;
int64_t sizeLimit = 100 * 1e6;
sampleRate = std::atof((const char*)tokens[3].begin());
if (tokens.size() == 5) {
if (tokencmp(tokens[3], "default")) {
sampleRate = std::numeric_limits<double>::infinity();
} else {
sampleRate = std::atof((const char*)tokens[3].begin());
}
int64_t sizeLimit;
if (tokencmp(tokens[4], "default")) {
sizeLimit = -1;
} else {
Optional<uint64_t> parsed = parse_with_suffix(tokens[4].toString());
if (parsed.present()) {
sizeLimit = parsed.get();
@ -2640,22 +2658,7 @@ ACTOR Future<int> cli(CLIOptions opt, LineNoise* plinenoise) {
}
continue;
}
if (tokencmp(tokens[2], "off")) {
if (tokens.size() != 3) {
printUsage(tokens[0]);
is_error = true;
continue;
}
double sampleRate = std::numeric_limits<double>::infinity();
int64_t sizeLimit = -1;
tr->set(fdbClientInfoTxnSampleRate, BinaryWriter::toValue(sampleRate, Unversioned()));
tr->set(fdbClientInfoTxnSizeLimit, BinaryWriter::toValue(sizeLimit, Unversioned()));
if (!intrans) {
Void _ = wait( commitTransaction( tr ) );
}
continue;
}
printf("ERROR: Unknown state: %s\n", printable(tokens[2]).c_str());
printf("ERROR: Unknown action: %s\n", printable(tokens[2]).c_str());
is_error = true;
continue;
}

View File

@ -306,8 +306,8 @@ const UID dataDistributionModeLock = UID(6345,3425);
// Client status info prefix
const KeyRangeRef fdbClientInfoPrefixRange(LiteralStringRef("\xff\x02/fdbClientInfo/"), LiteralStringRef("\xff\x02/fdbClientInfo0"));
const KeyRef fdbClientInfoTxnSampleRate = LiteralStringRef("client_txn_sample_rate/").withPrefix(fdbClientInfoPrefixRange.begin);
const KeyRef fdbClientInfoTxnSizeLimit = LiteralStringRef("client_txn_size_limit/").withPrefix(fdbClientInfoPrefixRange.begin);
const KeyRef fdbClientInfoTxnSampleRate = LiteralStringRef("\xff\x02/fdbClientInfo/client_txn_sample_rate/");
const KeyRef fdbClientInfoTxnSizeLimit = LiteralStringRef("\xff\x02/fdbClientInfo/client_txn_size_limit/");
// Keyspace to maintain wall clock to version map
const KeyRangeRef timeKeeperPrefixRange(LiteralStringRef("\xff\x02/timeKeeper/map/"), LiteralStringRef("\xff\x02/timeKeeper/map0"));

View File

@ -1572,16 +1572,14 @@ ACTOR Future<Void> monitorProcessClasses(ClusterControllerData *self) {
}
ACTOR Future<Void> monitorClientTxnInfoConfigs(ClusterControllerData::DBInfo* db) {
state const Key sampleRate= LiteralStringRef("client_txn_sample_rate/").withPrefix(fdbClientInfoPrefixRange.begin);
state const Key sizeLimit = LiteralStringRef("client_txn_size_limit/").withPrefix(fdbClientInfoPrefixRange.begin);
loop {
state ReadYourWritesTransaction tr(db->db);
loop {
try {
tr.setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
tr.setOption(FDBTransactionOptions::PRIORITY_SYSTEM_IMMEDIATE);
state Optional<Value> rateVal = wait(tr.get(sampleRate));
state Optional<Value> limitVal = wait(tr.get(sizeLimit));
state Optional<Value> rateVal = wait(tr.get(fdbClientInfoTxnSampleRate));
state Optional<Value> limitVal = wait(tr.get(fdbClientInfoTxnSizeLimit));
ClientDBInfo clientInfo = db->clientInfo->get();
if (rateVal.present()) {
double rate = BinaryReader::fromStringRef<double>(rateVal.get(), Unversioned());
@ -1596,8 +1594,8 @@ ACTOR Future<Void> monitorClientTxnInfoConfigs(ClusterControllerData::DBInfo* db
db->clientInfo->set(clientInfo);
}
state Future<Void> watchRateFuture = tr.watch(sampleRate);
state Future<Void> watchLimitFuture = tr.watch(sizeLimit);
state Future<Void> watchRateFuture = tr.watch(fdbClientInfoTxnSampleRate);
state Future<Void> watchLimitFuture = tr.watch(fdbClientInfoTxnSizeLimit);
Void _ = wait(tr.commit());
choose {
when(Void _ = wait(watchRateFuture)) { break; }