Skip to content

Commit

Permalink
Merge pull request redpanda-data#20802 from pgellert/quotas/follow-up…
Browse files Browse the repository at this point in the history
…-changes

Quotas: address post merge code review comments
  • Loading branch information
pgellert authored Jul 2, 2024
2 parents 2d309a0 + 359e29b commit 62222c7
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 3 deletions.
4 changes: 2 additions & 2 deletions src/v/cluster/client_quota_serde.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,13 @@ struct entity_value
struct entity_value_diff
: serde::
envelope<entity_value_diff, serde::version<0>, serde::compat_version<0>> {
enum key : int8_t {
enum class key {
producer_byte_rate = 0,
consumer_byte_rate,
controller_mutation_rate,
};

enum operation : int8_t {
enum class operation {
upsert = 0,
remove,
};
Expand Down
2 changes: 1 addition & 1 deletion src/v/cluster/tests/client_quota_store_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ BOOST_AUTO_TEST_CASE(quota_store_snapshot_delta) {
.key = key1,
.diff = {
.entries = {
entity_value_diff::entry(entity_value_diff::remove, entity_value_diff::key::producer_byte_rate, 0),
entity_value_diff::entry(entity_value_diff::operation::remove, entity_value_diff::key::producer_byte_rate, 0),
},
},
},
Expand Down
10 changes: 10 additions & 0 deletions src/v/kafka/server/quota_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ class quota_manager : public ss::peering_sharded_service<quota_manager> {
std::optional<atomic_token_bucket> pm_rate;
};

// Note: the use of std::shared_ptr<> is generally discouraged in the
// redpanda codebase because of the overhead of the atomic reference count
// and because seastar's memory model expects data to be deallocated on the
// same shard as where it was allocated and sharing std::shared_ptr<>'s
// across cores makes it easy to have the deallocation be on a different
// shard. The reason why it is acceptable to use a std::shared_ptr<> here is
// because (1) we're always allocating and deallocating the objects inside
// std::shared_ptr<> on shard 0 and (2) we need a way to keep track of a
// reference count across multiple shards, which is exactly what
// std::shared_ptr<> is meant for.
using local_map_t
= chunked_hash_map<tracker_key, std::shared_ptr<client_quota>>;

Expand Down

0 comments on commit 62222c7

Please sign in to comment.