Skip to content

Commit

Permalink
correctly configure I/O Scheduler for usage with the YAML file
Browse files Browse the repository at this point in the history
We had recently a bug with the I/O Scheduler where it was not configured
correctly for the case where --max-io-requests (legacy) was passed.

The solution for that involved testing for whether or not we have
specified a capacity for the io_queue: an io_queue with a capacity is a
legacy io_queue, and an io_queue without a capacity is configured with
the new yaml method.

The problem is that despite that being the intention, that was not what
I wrote. Instead of testing for _capacity to be the default, I tested
for cfg.capacity, which is divided by the number of I/O Queues already
As a result, we would now never take the yaml-config branch.

This patch changes that and also proactively works to avoid such problems
in the future by changing the default state to optional, instead of just
std::numeric_limits<unsigned>::max().

Signed-off-by: Glauber Costa <[email protected]>
Message-Id: <[email protected]>
  • Loading branch information
Glauber Costa authored and avikivity committed Aug 27, 2018
1 parent 1d40a5d commit 12f18ce
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions core/reactor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4036,7 +4036,7 @@ void smp::qs_deleter::operator()(smp_message_queue** qs) const {
class disk_config_params {
public:
unsigned _num_io_queues = smp::count;
unsigned _capacity = std::numeric_limits<unsigned>::max();
compat::optional<unsigned> _capacity;
std::unordered_map<dev_t, mountpoint_params> _mountpoints;
std::chrono::duration<double> _latency_goal;

Expand Down Expand Up @@ -4102,8 +4102,7 @@ class disk_config_params {
uint64_t max_bandwidth = std::max(p.read_bytes_rate, p.write_bytes_rate);
uint64_t max_iops = std::max(p.read_req_rate, p.write_req_rate);

cfg.capacity = per_io_queue(_capacity);
if (cfg.capacity == std::numeric_limits<unsigned>::max()) {
if (!_capacity) {
cfg.disk_bytes_write_to_read_multiplier = (io_queue::read_request_base_count * p.read_bytes_rate) / p.write_bytes_rate;
cfg.disk_req_write_to_read_multiplier = (io_queue::read_request_base_count * p.read_req_rate) / p.write_req_rate;
cfg.max_req_count = max_bandwidth == std::numeric_limits<uint64_t>::max()
Expand All @@ -4114,6 +4113,7 @@ class disk_config_params {
: io_queue::read_request_base_count * per_io_queue(max_bandwidth * _latency_goal.count());
cfg.mountpoint = p.mountpoint;
} else {
cfg.capacity = per_io_queue(*_capacity);
cfg.disk_bytes_write_to_read_multiplier = 1;
cfg.disk_req_write_to_read_multiplier = 1;
}
Expand Down

0 comments on commit 12f18ce

Please sign in to comment.