Skip to content

Commit

Permalink
Bug#35019415 forwarding resultsets with lots of rows is slow
Browse files Browse the repository at this point in the history 
Profiling queries with many short rows showed that the large part of the
execution time is spent encrypting rows.

Currently, the rows of a resultset are processed in a loop of:

a. read frame from server plain buffer
b. encrypt frame to client socket buffer
c. repeat until client buffer full
d. send client socket buffer
e. repeat with read frame until no more frames

Moving the encrypt step out of the inner loop and encrypting only once:

a. read frame from server plain buffer
b. move frame to client plain buffer
c. repeat until client plain buffer full
d. encrypt client plain buffer to client socket buffer
e. send client socket buffer
f. repeat with read frame until no more frames

shows a performances improvement, both in profiling and in benchmark.

For a resultset with many (100k) short rows (~10 bytes):

- throughput:  +520% (PREFERRED__AS_CLIENT)
- latency:      -83%

- throughput: +1200% (PREFERRED__DISABLED)
- latency:      -93%

Before:

[ RUN      ] Spec/Benchmark.classic_protocol/many_short_rows
name                      | query      | fetch      | throughput
------------------ no-ssl | ---------- | ---------- | -----------
DIRECT_DISABLED           |    6.63 us |   13.36 ms |  34.90 MB/s
DISABLED__DISABLED        |   10.30 us |   24.06 ms |  19.38 MB/s
DISABLED__REQUIRED        |   12.14 us |   57.73 ms |   8.08 MB/s
--------------------- ssl | ---------- | ---------- | -----------
DIRECT_PREFERRED          |    8.19 us |   19.17 ms |  24.33 MB/s
PASSTHROUGH__AS_CLIENT    |    8.02 us |   19.52 ms |  23.88 MB/s
PREFERRED__DISABLED       |    6.03 us |  314.28 ms |   1.48 MB/s
PREFERRED__AS_CLIENT      |    6.15 us |  313.33 ms |   1.49 MB/s
PREFERRED__PREFERRED      |    6.29 us |  316.07 ms |   1.48 MB/s
[       OK ] Spec/Benchmark.classic_protocol/many_short_rows (2188 ms)

After:

[ RUN      ] Spec/Benchmark.classic_protocol/many_short_rows
name                      | query      | fetch      | throughput
------------------ no-ssl | ---------- | ---------- | -----------
DIRECT_DISABLED           |    6.57 us |   13.82 ms |  33.74 MB/s
DISABLED__DISABLED        |    7.59 us |   23.44 ms |  19.89 MB/s
DISABLED__REQUIRED        |    9.10 us |   49.63 ms |   9.40 MB/s
--------------------- ssl | ---------- | ---------- | -----------
DIRECT_PREFERRED          |    8.35 us |   19.34 ms |  24.10 MB/s
PASSTHROUGH__AS_CLIENT    |    8.77 us |   19.50 ms |  23.91 MB/s
PREFERRED__DISABLED       |    8.60 us |   23.55 ms |  19.80 MB/s
PREFERRED__AS_CLIENT      |    7.92 us |   50.36 ms |   9.26 MB/s
PREFERRED__PREFERRED      |    8.82 us |   50.18 ms |   9.29 MB/s
[       OK ] Spec/Benchmark.classic_protocol/many_short_rows (1198 ms)

Change
------

- added a send_plain_buffer()
- moved SSL_write() from Channel::write_plain() to
  Channel::flush_to_send_buf() to encrypt only once.
- removed unused Channel::write_encrypted() and
  Channel::read_encrypted()

Change-Id: I08f105cd1d6aac2d3f96877337d3a10005a1f2a5
  • Loading branch information
@weigon
weigon committed Jan 30, 2023
1 parent 6ef0067 commit 1ab4790
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 142 deletions.
179 changes: 81 additions & 98 deletions router/src/routing/src/channel.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA

#include <cassert>

#include <openssl/bio.h>
#include <openssl/ssl.h>

#include "mysql/harness/net_ts/buffer.h"
Expand Down Expand Up @@ -71,75 +72,13 @@ stdx::expected<bool, std::error_code> Channel::tls_shutdown() {

stdx::expected<size_t, std::error_code> Channel::write_plain(
const net::const_buffer &b) {
if (ssl_) {
const auto res = SSL_write(ssl_.get(), b.data(), b.size());
if (res <= 0) {
return stdx::make_unexpected(make_tls_ssl_error(ssl_.get(), res));
}
// append to write_buffer
auto dyn_buf = net::dynamic_buffer(send_plain_buffer());

return res;
} else {
// append to write_buffer
auto dyn_buf = net::dynamic_buffer(this->send_buffer());

auto orig_size = dyn_buf.size();
dyn_buf.grow(b.size());
auto orig_size = dyn_buf.size();
dyn_buf.grow(b.size());

return net::buffer_copy(dyn_buf.data(orig_size, b.size()), b);
}
}

stdx::expected<size_t, std::error_code> Channel::read_encrypted(
const net::mutable_buffer &b) {
if (ssl_) {
auto wbio = SSL_get_wbio(ssl_.get());

const auto res = BIO_read(wbio, b.data(), b.size());
if (res < 0) {
if (BIO_should_retry(wbio)) {
return stdx::make_unexpected(
make_error_code(std::errc::operation_would_block));
} else {
return stdx::make_unexpected(
make_error_code(std::errc::invalid_argument));
}
}

return res;
} else {
// read from send_buffer
auto dyn_buf = net::dynamic_buffer(this->send_buffer());

auto orig_size = dyn_buf.size();
dyn_buf.grow(b.size());

net::buffer_copy(dyn_buf.data(orig_size, b.size()), b);

return b.size();
}
}

stdx::expected<size_t, std::error_code> Channel::write_encrypted(
const net::const_buffer &b) {
if (ssl_) {
auto rbio = SSL_get_rbio(ssl_.get());

const auto res = BIO_write(rbio, b.data(), b.size());
if (res < 0) {
return stdx::make_unexpected(
make_error_code(std::errc::operation_would_block));
}

return res;
} else {
// append to recv-buffer
auto dyn_buf = net::dynamic_buffer(this->recv_buffer());

auto orig_size = dyn_buf.size();
dyn_buf.grow(b.size());

return net::buffer_copy(dyn_buf.data(orig_size, b.size()), b);
}
return net::buffer_copy(dyn_buf.data(orig_size, b.size()), b);
}

stdx::expected<size_t, std::error_code> Channel::read_plain(
Expand Down Expand Up @@ -172,23 +111,27 @@ stdx::expected<size_t, std::error_code> Channel::flush_from_recv_buf() {

view_discard_raw();

auto rbio = SSL_get_rbio(ssl_.get());

size_t transferred{};

auto dyn_buf = net::dynamic_buffer(recv_buf);
while (dyn_buf.size() != 0) {
const auto orig_size = dyn_buf.size();
const auto res = write_encrypted(dyn_buf.data(0, orig_size));

if (!res) {
if (res.error() == std::errc::operation_would_block &&
transferred != 0) {
return transferred;
}
return res;

auto buf = dyn_buf.data(0, orig_size);

const auto bio_res = BIO_write(rbio, buf.data(), buf.size());
if (bio_res < 0) {
if (transferred != 0) return transferred;

return stdx::make_unexpected(
make_error_code(std::errc::operation_would_block));
}
dyn_buf.consume(res.value());

transferred += res.value();
dyn_buf.consume(bio_res);

transferred += bio_res;

// recv-buffer changed, update the view.
view_sync_raw();
Expand All @@ -201,34 +144,70 @@ stdx::expected<size_t, std::error_code> Channel::flush_from_recv_buf() {
}

stdx::expected<size_t, std::error_code> Channel::flush_to_send_buf() {
if (ssl_) {
// if this is non-ssl channel, no bytes get copied from send_plain_buffer() to
// send_buffer()
if (!ssl_) return 0;

//
// if there is plaintext data, encrypt it ...
//

if (!this->send_plain_buffer_.empty()) {
// encode the plaintext data
auto &buf = this->send_plain_buffer_;

const auto spn = net::buffer(buf);

const auto res = SSL_write(ssl_.get(), spn.data(), spn.size());
if (res <= 0) {
return stdx::make_unexpected(make_tls_ssl_error(ssl_.get(), res));
}

// remove the data that has been sent.
net::dynamic_buffer(buf).consume(res);
}

//
// ... and if there is encrypted data, move it to the socket's send-buffer.
//

auto *wbio = SSL_get_wbio(ssl_.get());

size_t transferred{};

// check if there is encrypted data waiting.
while (const auto pending = BIO_pending(wbio)) {
auto dyn_buf = net::dynamic_buffer(this->send_buffer());
const auto orig_size = dyn_buf.size();
const auto grow_size = pending;

size_t transferred{};
// append encrypted data to the send-buffer.
dyn_buf.grow(grow_size);
auto buf = dyn_buf.data(orig_size, grow_size);

while (true) {
const auto orig_size = dyn_buf.size();
const auto grow_size = 16 * 1024; // a TLS frame

dyn_buf.grow(grow_size);

const auto res = read_encrypted(dyn_buf.data(orig_size, grow_size));
if (!res) {
dyn_buf.shrink(grow_size);
if ((res.error() ==
make_error_condition(std::errc::operation_would_block)) &&
transferred != 0) {
return transferred;
}
return res;
const auto bio_res = BIO_read(wbio, buf.data(), buf.size());
if (bio_res < 0) {
dyn_buf.shrink(grow_size);

if (!BIO_should_retry(wbio)) {
return stdx::make_unexpected(
make_error_code(std::errc::invalid_argument));
}
dyn_buf.shrink(grow_size - res.value());

transferred += res.value();
if (transferred != 0) return transferred;

return stdx::make_unexpected(
make_error_code(std::errc::operation_would_block));
}
} else {
return this->send_buffer().size();

assert(bio_res != 0);

dyn_buf.shrink(grow_size - bio_res);

transferred += bio_res;
}

return transferred;
}

stdx::expected<size_t, std::error_code> Channel::read_to_plain(size_t sz) {
Expand Down Expand Up @@ -280,6 +259,10 @@ stdx::expected<size_t, std::error_code> Channel::read_to_plain(size_t sz) {
return bytes_read;
}

Channel::recv_buffer_type &Channel::send_plain_buffer() {
return ssl_ ? send_plain_buffer_ : send_buffer_;
}

const Channel::recv_view_type &Channel::recv_view() const { return recv_view_; }

const Channel::recv_view_type &Channel::recv_plain_view() const {
Expand Down
32 changes: 12 additions & 20 deletions router/src/routing/src/channel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,27 +164,17 @@ class Channel {

stdx::expected<size_t, std::error_code> read_to_plain(size_t sz);

/**
* write raw data from a net::const_buffer to the channel.
*/
stdx::expected<size_t, std::error_code> write_encrypted(
const net::const_buffer &b);

/**
* write unencrypted data from a net::const_buffer to the channel.
*
* if the channel has an ssl session it transparently encrypts before
* the data is appended to the send_buf()
* call flush_to_send_buf() ensure data is written to the
* send-buffers for the socket.
*
* @see flush_to_send_buf()
*/
stdx::expected<size_t, std::error_code> write_plain(
const net::const_buffer &b);

/**
* read raw data from recv_buffer() into b.
*/
stdx::expected<size_t, std::error_code> read_encrypted(
const net::mutable_buffer &b);

/**
* read plaintext data from recv_plain_buffer() into b.
*/
Expand Down Expand Up @@ -229,22 +219,24 @@ class Channel {

/**
* buffer of data that was received from the socket.
*
* written into by write(), write_plain(), write_encrypted().
*/
recv_buffer_type &recv_buffer() { return recv_buffer_; }

/**
* buffer of data to be sent to the socket.
*
* written into by write(), write_plain(), write_encrypted().
* written into by write(), write_plain(), flush_to_send_buf().
*/
recv_buffer_type &send_buffer() { return send_buffer_; }

/**
* unencrypted data to be sent to the socket.
*/
recv_buffer_type &send_plain_buffer();

/**
* buffer of data that was received from the socket.
*
* written into by write(), write_plain(), write_encrypted().
*/
const recv_buffer_type &recv_buffer() const { return recv_buffer_; }

Expand All @@ -255,8 +247,6 @@ class Channel {

/**
* buffer of data to be sent to the socket.
*
* written into by write(), write_plain(), write_encrypted().
*/
const recv_buffer_type &send_buffer() const { return send_buffer_; }

Expand Down Expand Up @@ -321,6 +311,8 @@ class Channel {
recv_view_type recv_view_;
recv_buffer_type recv_plain_buffer_;
recv_view_type recv_plain_view_;

recv_buffer_type send_plain_buffer_;
recv_buffer_type send_buffer_;

bool is_tls_{false};
Expand Down
Loading

0 comments on commit 1ab4790

Please sign in to comment.