Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: change the way to call replication_ddl_client::set_app_envs #398

Merged
merged 8 commits into from
Sep 24, 2019
Merged

refactor: change the way to call replication_ddl_client::set_app_envs #398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3504018
table level slow query
Sep 23, 2019
5669bd5
Merge branch 'levy5307-rdsn-slow-query'
Sep 23, 2019
cad12d6
update rdsn
Sep 23, 2019
a70b060
update submodule
Sep 23, 2019
b549c36
update submodule
Sep 23, 2019
2b01bbe
Merge branch 'master' of https://github.com/XiaoMi/pegasus into rdsn-…
Sep 23, 2019
ba35dea
rdsn slow query
Sep 23, 2019
de368a8
rdsn slow query
Sep 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rdsn
Open in desktop
Submodule rdsn updated 128 files
16 changes: 12 additions & 4 deletions src/shell/commands/table_management.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,11 +744,19 @@ bool set_app_envs(command_executor *e, shell_context *sc, arguments args)
values.emplace_back(args.argv[idx++]);
}

::dsn::error_code ret = sc->ddl_client->set_app_envs(sc->current_app_name, keys, values);

if (ret != ::dsn::ERR_OK) {
fprintf(stderr, "set app env failed with err = %s\n", ret.to_string());
auto err_resp = sc->ddl_client->set_app_envs(sc->current_app_name, keys, values);
dsn::error_s err = err_resp.get_error();
std::string hint_msg;
if (err.is_ok()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

err is ok, 这个听起来太奇怪了. 建议改成err_code之类的. 而且response的error和response的信息里的error, 可以从命名上区分的更清楚些

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

要改就改 error_s 的名字。这个在 rocksdb 叫 Status::isOK，在 rust 叫 Result::is_ok，最后其实就是一个写法的问题。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

现在就专注于这里的表述就好了, 变量名改好就足够了. is_ok()这个没有什么问题, 我上面没表述清楚

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我也觉得改就改errors_s的名字最好，单纯从类型和变量名来看，err这个变量起的其实没什么问题，你上面说的err is ok奇怪说起来还是error_s::is_ok()的问题，改变量也改变不了什么

err = dsn::error_s::make(err_resp.get_value().err);
hint_msg = err_resp.get_value().hint_message;
}
if (!err.is_ok()) {
fmt::print(stderr, "set app envs failed with error {} [hint:\"{}\"]!\n", err, hint_msg);
} else {
fmt::print(stdout, "set app envs succeed\n");
}

return true;
}

Expand Down
23 changes: 13 additions & 10 deletions src/test/function_test/test_ttl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@ void set_default_ttl(int ttl)

std::string env = envs[TABLE_LEVEL_DEFAULT_TTL];
if ((env.empty() && ttl != 0) || env != std::to_string(ttl)) {
dsn::error_code ec = ddl_client->set_app_envs(
auto response = ddl_client->set_app_envs(
client->get_app_name(), {TABLE_LEVEL_DEFAULT_TTL}, {std::to_string(ttl)});
ASSERT_EQ(ERR_OK, ec);
ASSERT_EQ(true, response.is_ok());
ASSERT_EQ(ERR_OK, response.get_value().err);

// wait envs to be synced.
std::this_thread::sleep_for(std::chrono::seconds(sleep_for_envs_effect));
Expand Down Expand Up @@ -99,10 +100,11 @@ TEST(ttl, set_without_default_ttl)
ASSERT_EQ(ttl_test_value_2, value);

// trigger a manual compaction
dsn::error_code ec = ddl_client->set_app_envs(client->get_app_name(),
{MANUAL_COMPACT_ONCE_TRIGGER_TIME_KEY},
{std::to_string(time(nullptr))});
ASSERT_EQ(ERR_OK, ec);
auto response = ddl_client->set_app_envs(client->get_app_name(),
{MANUAL_COMPACT_ONCE_TRIGGER_TIME_KEY},
{std::to_string(time(nullptr))});
ASSERT_EQ(true, response.is_ok());
ASSERT_EQ(ERR_OK, response.get_value().err);

// wait envs to be synced, and manual lcompaction has been finished.
std::this_thread::sleep_for(std::chrono::seconds(sleep_for_envs_effect));
Expand Down Expand Up @@ -191,10 +193,11 @@ TEST(ttl, set_with_default_ttl)
ASSERT_EQ(ttl_test_value_2, value);

// trigger a manual compaction
dsn::error_code ec = ddl_client->set_app_envs(client->get_app_name(),
{MANUAL_COMPACT_ONCE_TRIGGER_TIME_KEY},
{std::to_string(time(nullptr))});
ASSERT_EQ(ERR_OK, ec);
auto response = ddl_client->set_app_envs(client->get_app_name(),
{MANUAL_COMPACT_ONCE_TRIGGER_TIME_KEY},
{std::to_string(time(nullptr))});
ASSERT_EQ(true, response.is_ok());
ASSERT_EQ(ERR_OK, response.get_value().err);

// wait envs to be synced, and manual compaction has been finished.
std::this_thread::sleep_for(std::chrono::seconds(sleep_for_envs_effect));
Expand Down