Virtual connections in Kafka server #16658

mmaslankaprv · 2024-02-21T13:48:20Z

Implemented virtualization of Kafka connections on top of one physical
TCP connection. Requests processed in a virtual connection context are
independent from other requests in different virtual connections. This
way we prevent requests from different virtual connections from blocking
each other.

Virtual connections are only available if first requests handled by the
connection has the __redpanda_mpx client id set and Redpanda MPX
extensions are enabled.

Backports Required

Release Notes

none

dotnwat

largely looks ok to me.

I had imagined that instead of adding complexity to / generalizing connection_context, we would instead leave connection_context as the logical connection largely untounched (except for pulling out the bits that grab the requests off the wire), and virtualize above it.

src/v/kafka/server/server.h

src/v/kafka/server/connection_context.cc

dotnwat · 2024-02-22T05:09:52Z

src/v/kafka/server/connection_context.cc

+              is_first_request()
+              && h->client_id == multi_proxy_initial_client_id)) {


so what happens if the first request doesn't have this magic client id, and then later the connection starts behaving like its virtualized?

no, in this case connection is not virtualized

src/v/kafka/server/connection_context.cc

mmaslankaprv · 2024-02-22T07:57:46Z

largely looks ok to me.

I had imagined that instead of adding complexity to / generalizing connection_context, we would instead leave connection_context as the logical connection largely untounched (except for pulling out the bits that grab the requests off the wire), and virtualize above it.

Connection context has a lot of state which is valid only for the physical connection like _mtls and auth state therefore i decided to do it inside of the connection context to make the virtualized state as small as possible.

graphcareful

Nice job! I like how the diff is pretty small because the abstractions themselves implement process_request

src/v/kafka/server/connection_context.h

mmaslankaprv · 2024-02-23T07:46:19Z

/dt

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

vbotbuildovich · 2024-02-23T10:28:38Z

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45291#018dd530-bf67-46af-ab42-a83347601263

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45296#018dd5c1-3c04-426f-bc06-81d7e8e2e062

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45296#018dd5ea-0df9-4bee-b109-0f26f2b7a3ec

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45347#018de5b0-1e0c-45f7-89bb-ea4d39cca195

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45387#018de9a6-14d0-47cd-8bc6-1775e1961223

mmaslankaprv · 2024-02-23T10:43:19Z

/dt

mmaslankaprv · 2024-02-26T12:32:00Z

/dt

mmaslankaprv · 2024-02-27T07:01:50Z

/ci-repeat 1

michael-redpanda

Looks nice, just a couple of questions

src/v/kafka/server/connection_context.cc

michael-redpanda · 2024-02-27T16:09:42Z

src/v/kafka/server/connection_context.cc

+bytes parse_virtual_connection_id(const ss::temporary_buffer<char>& buffer) {
+    // TODO: should we use vcluster_id here ?
+    return bytes{
+      reinterpret_cast<const uint8_t*>(buffer.begin()), buffer.size()};
+}


question: So once we get the 'magic' mpx value, each subsequent request will have the virtual connection ID in the client_id. Is there any format to that value? Does it need any additional validation or once we know we're a virtual connection, any value for client ID is accepted?

we haven't yet agreed with the team about the format, i made it to accept any string for now, a plan is to add schema to it in future PR

michael-redpanda · 2024-02-27T16:13:32Z

src/v/kafka/server/connection_context.cc

+    auto v_connection_id = parse_virtual_connection_id(
+      rctx.header().client_id_buffer);


question: Is there any requirement/need to handle authentication messages differently than other requests? Like, only handle authn requests if it has the magic mpx header value? As RP now supports re-authentication should that be disallowed in a virtual connection? How does the MPX system authenticate with Redpanda? Is it via SASL/SCRAM or will it use mTLS?

Maybe worth noting that reauth is performed automagically (transparently) by modern enough Kafka clients. If the MPX system needs to mediate requests in some way to conform to the virtual connection protocol (and if reauthentication is desirable), this might require additional handling/care.

i went over the code and it seems that there is no additional handling required for authentication/reauth requests. They do not have any special handling of client id. I think in this case we should relay on client to order the requests correctly.

They do not have any special handling of client id

The question I was trying to pose was whether or not authentication for virtual connections should be handled differently based on client ID (e.g. only honor auth requests from the mpx magic client for example).

I think in this case we should relay on client to order the requests correctly.

I think that's fine. I'm just posing the question as any change of authentication will effect all virtual connections.

Implemented virtualization of Kafka connections on top of one physical TCP connection. Requests processed in a virtual connection context are independent from other requests in different virtual connections. This way we prevent requests from different virtual connections from blocking each other. Virtual connections are only available if first requests handled by the connection has the `__redpanda_mpx` client id set and Redpanda MPX extensions are enabled. Signed-off-by: Michal Maslanka <michal@redpanda.com>

Added a test that validates if requests executed in different connection contexts do not block each other. The test is using a `kafka-python` client. Unfortunately we need to access the client internal to change client ids and expected response sequences but we are able to execute the virtual connection handling code without pulling in MPX into ducktape. Signed-off-by: Michal Maslanka <michal@redpanda.com>

michael-redpanda

LGTM

github-actions bot added the area/redpanda label Feb 21, 2024

mmaslankaprv force-pushed the multiplexing-connections branch from ec87b64 to 6a3ceb8 Compare February 21, 2024 14:09

dotnwat reviewed Feb 22, 2024

View reviewed changes

mmaslankaprv force-pushed the multiplexing-connections branch from 6a3ceb8 to d4445b9 Compare February 22, 2024 08:36

mmaslankaprv requested review from bharathv, graphcareful and dotnwat February 22, 2024 08:36

mmaslankaprv marked this pull request as draft February 22, 2024 10:52

graphcareful reviewed Feb 22, 2024

View reviewed changes

src/v/kafka/server/connection_context.h Outdated Show resolved Hide resolved

mmaslankaprv force-pushed the multiplexing-connections branch from d4445b9 to 7b7af45 Compare February 22, 2024 17:24

mmaslankaprv added 2 commits February 23, 2024 10:53

configuration: added property controlling mpx_extensions

28f5272

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

k/server: expose enable_mpx_extensions method

93299cd

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

mmaslankaprv force-pushed the multiplexing-connections branch from 7b7af45 to e5eb0e4 Compare February 23, 2024 10:40

redpanda-data deleted a comment from vbotbuildovich Feb 23, 2024

mmaslankaprv force-pushed the multiplexing-connections branch from e5eb0e4 to 3ba460d Compare February 26, 2024 08:51

mmaslankaprv marked this pull request as ready for review February 26, 2024 16:49

mmaslankaprv requested a review from graphcareful February 26, 2024 16:49

mmaslankaprv requested review from oleiman and michael-redpanda February 27, 2024 11:56

michael-redpanda reviewed Feb 27, 2024

View reviewed changes

mmaslankaprv added 2 commits February 28, 2024 08:47

mmaslankaprv force-pushed the multiplexing-connections branch from 3ba460d to dd777b0 Compare February 28, 2024 08:33

mmaslankaprv requested a review from michael-redpanda February 28, 2024 08:48

michael-redpanda approved these changes Feb 29, 2024

View reviewed changes

mmaslankaprv merged commit 3839e4c into redpanda-data:dev Mar 1, 2024
17 checks passed

mmaslankaprv deleted the multiplexing-connections branch March 1, 2024 10:15

pgellert mentioned this pull request May 20, 2024

[v23.3.x] CORE-2752 - Fix Kafka quota throttling delay enforcement #18575

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Virtual connections in Kafka server #16658

Virtual connections in Kafka server #16658

mmaslankaprv commented Feb 21, 2024 •

edited

dotnwat left a comment •

edited

dotnwat Feb 22, 2024

mmaslankaprv Feb 22, 2024

mmaslankaprv commented Feb 22, 2024

graphcareful left a comment

mmaslankaprv commented Feb 23, 2024

vbotbuildovich commented Feb 23, 2024 •

edited

mmaslankaprv commented Feb 23, 2024

mmaslankaprv commented Feb 26, 2024

mmaslankaprv commented Feb 27, 2024

michael-redpanda left a comment

michael-redpanda Feb 27, 2024

mmaslankaprv Feb 28, 2024

michael-redpanda Feb 27, 2024

oleiman Feb 27, 2024

mmaslankaprv Feb 28, 2024

michael-redpanda Feb 28, 2024

michael-redpanda left a comment

		is_first_request()
		&& h->client_id == multi_proxy_initial_client_id)) {

		auto v_connection_id = parse_virtual_connection_id(
		rctx.header().client_id_buffer);

Virtual connections in Kafka server #16658

Virtual connections in Kafka server #16658

Conversation

mmaslankaprv commented Feb 21, 2024 • edited

Backports Required

Release Notes

dotnwat left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmaslankaprv commented Feb 22, 2024

graphcareful left a comment

Choose a reason for hiding this comment

mmaslankaprv commented Feb 23, 2024

vbotbuildovich commented Feb 23, 2024 • edited

mmaslankaprv commented Feb 23, 2024

mmaslankaprv commented Feb 26, 2024

mmaslankaprv commented Feb 27, 2024

michael-redpanda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michael-redpanda left a comment

Choose a reason for hiding this comment

mmaslankaprv commented Feb 21, 2024 •

edited

dotnwat left a comment •

edited

vbotbuildovich commented Feb 23, 2024 •

edited