Skip to content

Commit

Permalink
fair_queue: disallow zeroed shares.
Browse files Browse the repository at this point in the history
The value of 0 is an invalid value for shares. It is also dangerously
invalid, in the sense that due to way we calculate the cost of requests
it will add a NaN to the accumulator that will stay there forever - even
if the shares happen to be adjusted later.

Although we could just document it and expect the caller to verify that,
or even throw an exception, we can just as easily enforce in the
fair_queue that the minimum shares should be 1 instead of 0.

Classes tend to be well behaved with respect to its shares, which is why
we have never really felt the need for it. But once we start adjusting
shares that is an easy mistake to make.

Signed-off-by: Glauber Costa <[email protected]>
Message-Id: <[email protected]>
  • Loading branch information
Glauber Costa authored and avikivity committed Nov 25, 2017
1 parent 87b759d commit 80330bb
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions core/fair_queue.hh
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ class priority_class {
bool _queued = false;

friend struct shared_ptr_no_esft<priority_class>;
explicit priority_class(uint32_t shares) : _shares(shares) {}
explicit priority_class(uint32_t shares) : _shares(std::max(shares, 1u)) {}

void update_shares(uint32_t shares) {
_shares = (std::max(shares, 1u));
}
};
/// \endcond

Expand Down Expand Up @@ -225,7 +229,7 @@ public:
///
/// \param new_shares the new number of shares for this priority class
static void update_shares(priority_class_ptr pc, uint32_t new_shares) {
pc->_shares = new_shares;
pc->update_shares(new_shares);
}
};
/// @}
Expand Down

0 comments on commit 80330bb

Please sign in to comment.