Shard producer state manager with `v_cluster_id` #16992

mmaslankaprv · 2024-03-11T09:31:05Z

Change implementation of producer_state_manager to keep separate producer state LRU cache for each v_cluster_id. This approach isolates each virtual cluster queue and guarantees fair distribution of producer state slots among them.

Backports Required

Release Notes

none

vbotbuildovich · 2024-03-12T11:51:55Z

new failures in https://buildkite.com/redpanda/redpanda/builds/46046#018e3236-bc93-42f2-8f65-14656265c541:

"rptest.tests.random_node_operations_test.RandomNodeOperationsTest.test_node_operations.enable_failures=True.num_to_upgrade=0.with_tiered_storage=True"

new failures in https://buildkite.com/redpanda/redpanda/builds/46614#018e6653-d8b7-462b-88a8-e1af5b154ae7:

"rptest.tests.raft_availability_test.RaftAvailabilityTest.test_two_nodes_down"

vbotbuildovich · 2024-03-15T17:46:10Z

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/46264#018e42ef-8a29-4ac7-bea9-3e312dbca478

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/46614#018e6653-d8b2-461c-9c45-79487c5e56b6

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/46721#018e7723-5907-457d-bbe4-aaf24924ab55

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/46721#018e7737-0973-4bbd-8128-4b4cb93014e4

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/46875#018e801c-4769-434c-a195-35aacbef34c8

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/46894#018e80a6-6ee7-4a7e-abbe-238fc4e39ff3

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/46894#018e80b8-ae52-4923-9cfb-52a570454cb8

bharathv

lgtm overall, mostly minor comments.

src/v/cluster/namespaced_cache.h

bharathv · 2024-03-18T20:08:04Z

src/v/cluster/producer_state.h

@@ -223,38 +219,24 @@ class producer_state {
    void update_current_txn_start_offset(std::optional<kafka::offset> offset) {
        _current_txn_start_offset = offset;
    }
+    safe_intrusive_list_hook _hook;


why is this public? seems a bit risky, perhaps befriend the other class?

it is a common pattern across the source to have the hook public, i will try in a follow up to change this.

bharathv · 2024-03-18T20:11:01Z

src/v/cluster/producer_state.h

-                      });
-                });
+        if (_evicted) {
+            throw ss::gate_closed_exception();


nit: gate_closed_exception without a gate instance seems a bit confusing.

yeah, agreed. I wanted to be consistent with previous behavior where a gate was part of the producer_state. Producer eviction was signalled by gate_closed_exception

bharathv · 2024-03-18T20:53:44Z

src/v/config/configuration.cc

+  , virtual_cluster_min_producer_ids(
+      *this,
+      "virtual_cluster_min_producer_ids",
+      "Minium number of active producers per virtual cluster",


typo: minimum

A little more color might be helpful. Why a minimum is needed.

Also on naming, should we prefix all mpx stuff with "mpx_", mpx_min_producers_per_cluder_id or something?

i didn't want the mpx_ prefix to be epidemic

src/v/cluster/rm_stm.cc

bharathv

lgtm.

Producer state manager is a cluster module service Raft layer should not depend on any cluster related services. Signed-off-by: Michal Maslanka <michal@redpanda.com>

Added time based eviction driven by an external timer to the `cluster::namespaced_cache` the method allows to evict entries older than the requested time. Signed-off-by: Michal Maslanka <michal@redpanda.com>

Using namespaced cache to isolate producer id queues for each virtual cluster. Each virtual cluster has its own guaranteed number of producer ids. If not all producer id slots are used virtual clusters will use more than minimum number of producer ids until they are required to provide min number of producer ids for other virtual clusters. Signed-off-by: Michal Maslanka <michal@redpanda.com>

Signed-off-by: Michal Maslanka <michal@redpanda.com>

Passing partition virtual cluster id to rm_stm to be able to add a virtual cluster context to producer state that is managed in producer state manager. Signed-off-by: Michal Maslanka <michal@redpanda.com>

Made producer state manager configurable with minimal number of producers per virtual cluster. Signed-off-by: Michal Maslanka <michal@redpanda.com>

Signed-off-by: Michal Maslanka <michal@redpanda.com>

When the number of allowed virtual clusters maintained in producer state manager cache is exceeded a cache throws an exception. The exception is translated to the error code indicating that the cache size limit was exceeded. The error code is finally translated to kafka policy violation error. This error is not retryable and will indicate issue with number of producers. Signed-off-by: Michal Maslanka <michal@redpanda.com>

Signed-off-by: Michal Maslanka <michal@redpanda.com>

github-actions bot added the area/redpanda label Mar 11, 2024

mmaslankaprv force-pushed the sharded-producer-state-mgr branch 2 times, most recently from 4e4412d to ce29dea Compare March 12, 2024 08:50

redpanda-data deleted a comment from vbotbuildovich Mar 12, 2024

mmaslankaprv force-pushed the sharded-producer-state-mgr branch from ce29dea to 7f376dd Compare March 15, 2024 11:36

mmaslankaprv requested review from bharathv and ztlpn March 15, 2024 12:40

bharathv reviewed Mar 18, 2024

View reviewed changes

mmaslankaprv force-pushed the sharded-producer-state-mgr branch from 7f376dd to 6a40f42 Compare March 22, 2024 12:10

mmaslankaprv requested a review from bharathv March 22, 2024 16:05

mmaslankaprv force-pushed the sharded-producer-state-mgr branch 3 times, most recently from 66f34b7 to 94db988 Compare March 25, 2024 18:25

bharathv previously approved these changes Mar 27, 2024

View reviewed changes

mmaslankaprv dismissed bharathv’s stale review via 1b6f59d March 27, 2024 08:13

mmaslankaprv force-pushed the sharded-producer-state-mgr branch from 94db988 to 1b6f59d Compare March 27, 2024 08:13

bharathv previously approved these changes Mar 27, 2024

View reviewed changes

mmaslankaprv dismissed bharathv’s stale review via c45d879 March 27, 2024 12:35

mmaslankaprv force-pushed the sharded-producer-state-mgr branch from 1b6f59d to c45d879 Compare March 27, 2024 12:35

mmaslankaprv added 9 commits March 27, 2024 16:10

tests: moved producer state manager to rm_stm fixture

4a130cc

Producer state manager is a cluster module service Raft layer should not depend on any cluster related services. Signed-off-by: Michal Maslanka <michal@redpanda.com>

c/namespaced_cache: added time based eviction to namespaced cache

9539724

Added time based eviction driven by an external timer to the `cluster::namespaced_cache` the method allows to evict entries older than the requested time. Signed-off-by: Michal Maslanka <michal@redpanda.com>

configuration: added property to set number of producers per vcluster

68a73fd

Signed-off-by: Michal Maslanka <michal@redpanda.com>

c/rm_stm: pass vcluster id to rm_stm

cad6d58

Passing partition virtual cluster id to rm_stm to be able to add a virtual cluster context to producer state that is managed in producer state manager. Signed-off-by: Michal Maslanka <michal@redpanda.com>

c/psm: wire min number of producers per virtual cluster

0316f59

Made producer state manager configurable with minimal number of producers per virtual cluster. Signed-off-by: Michal Maslanka <michal@redpanda.com>

c/errc: added error code for exceeded number of virtual clusters

14fbade

Signed-off-by: Michal Maslanka <michal@redpanda.com>

tests: added basic test validating maintaining per tenant producer cache

0ba1d36

Signed-off-by: Michal Maslanka <michal@redpanda.com>

mmaslankaprv force-pushed the sharded-producer-state-mgr branch from c45d879 to 0ba1d36 Compare March 27, 2024 15:11

mmaslankaprv requested a review from bharathv March 27, 2024 18:08

bharathv approved these changes Mar 27, 2024

View reviewed changes

bharathv merged commit 8916706 into redpanda-data:dev Mar 27, 2024
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Shard producer state manager with `v_cluster_id` #16992

Shard producer state manager with `v_cluster_id` #16992

mmaslankaprv commented Mar 11, 2024 •

edited

vbotbuildovich commented Mar 12, 2024 •

edited

vbotbuildovich commented Mar 15, 2024 •

edited

bharathv left a comment

bharathv Mar 18, 2024

mmaslankaprv Mar 22, 2024

bharathv Mar 18, 2024

mmaslankaprv Mar 22, 2024

bharathv Mar 18, 2024

mmaslankaprv Mar 22, 2024

bharathv left a comment

Shard producer state manager with v_cluster_id #16992

Shard producer state manager with v_cluster_id #16992

Conversation

mmaslankaprv commented Mar 11, 2024 • edited

Backports Required

Release Notes

vbotbuildovich commented Mar 12, 2024 • edited

vbotbuildovich commented Mar 15, 2024 • edited

bharathv left a comment

Choose a reason for hiding this comment

bharathv Mar 18, 2024

Choose a reason for hiding this comment

mmaslankaprv Mar 22, 2024

Choose a reason for hiding this comment

bharathv Mar 18, 2024

Choose a reason for hiding this comment

mmaslankaprv Mar 22, 2024

Choose a reason for hiding this comment

bharathv Mar 18, 2024

Choose a reason for hiding this comment

mmaslankaprv Mar 22, 2024

Choose a reason for hiding this comment

bharathv left a comment

Choose a reason for hiding this comment

Shard producer state manager with `v_cluster_id` #16992

Shard producer state manager with `v_cluster_id` #16992

mmaslankaprv commented Mar 11, 2024 •

edited

vbotbuildovich commented Mar 12, 2024 •

edited

vbotbuildovich commented Mar 15, 2024 •

edited