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.

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

feat: overload dump_write_request of replication_app_base.h #533

Merged
merged 30 commits into from
May 19, 2020

Conversation

foreverneverer
Copy link
Contributor

@foreverneverer foreverneverer commented May 12, 2020

What problem does this PR solve?

XiaoMi/rdsn#414 forbids the large write size request, but the log information is simple, XiaoMi/rdsn#458 provide the interface of dump_write_request, this pr overload it.

Check List

Tests

  • Manual test (add detailed scripts or steps below)
    • start onebox with config max_allowed_write_size = 5
    • use shell set any key-value whose size > 5
    • check the log:
[2.4@127.0.0.1:34801] client from 127.0.0.1:1 write request body size exceed threshold, request=[put:hash_key=a,sort_key=abc], request_body_size = 99, max_allowed_write_size = 5, it will be rejected!

Related changes

src/base/pegasus_utils.cpp Outdated Show resolved Hide resolved
src/server/pegasus_server_impl.h Outdated Show resolved Hide resolved
src/server/pegasus_server_impl.cpp Outdated Show resolved Hide resolved
acelyc111
acelyc111 previously approved these changes May 16, 2020
@foreverneverer foreverneverer changed the title feat: overload dwarn_write_request of replication_app_base.h feat: overload dump_write_request of replication_app_base.h May 18, 2020
@hycdong hycdong merged commit f8ef62e into apache:master May 19, 2020
@@ -2618,5 +2618,57 @@ void pegasus_server_impl::release_db()
_db = nullptr;
}

std::string pegasus_server_impl::dump_write_request(dsn::message_ex *request)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
std::string pegasus_server_impl::dump_write_request(dsn::message_ex *request)
static std::string dump_write_request(dsn::message_ex *request)

I see no private member of pegasus_server_impl is used, so make it a static function.

Comment on lines +2628 to +2633
std::string request("put:");
request.append("hash_key=")
.append(pegasus::utils::c_escape_string(hash_key))
.append(",sort_key=")
.append(pegasus::utils::c_escape_string(sort_key));
return request;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
std::string request("put:");
request.append("hash_key=")
.append(pegasus::utils::c_escape_string(hash_key))
.append(",sort_key=")
.append(pegasus::utils::c_escape_string(sort_key));
return request;
return fmt::format("hash_key={},sort_key={}",
pegasus::utils::c_escape_string(hash_key),
pegasus::utils::c_escape_string(sort_key));

if (rpc_code == dsn::apps::RPC_RRDB_RRDB_MULTI_PUT) {
auto multi_put = multi_put_rpc::auto_reply(request).request();
std::string request("multi_put:");
request.append("hash_key=")
Copy link
Contributor

Choose a reason for hiding this comment

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

using fmtlib will make it looks simpler and more efficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants