Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v24.1.x] audit: clamp audit client max parallelism #24149

Merged

Conversation

pgellert
Copy link
Contributor

@pgellert pgellert commented Nov 15, 2024

Backport of PR #24137

Fixes #24143

Fixes: CORE-8259

Conflict was in tests only

This fixes a bug in the audit client where if the cluster config value
`kafka_batch_max_bytes` is greater than `audit_client_max_buffer_size`,
the audit client ends up not producing any messages and fills up the
audit log buffers.

The problem is the division here could lead to `max_concurrency=0`. In
debug mode this is caught by an assert inside
`ss::max_concurrent_for_each` but in release mode stdlibrary `assert`s
are disabled and the behaviour ends up being that the background fibre
just blocks waiting for a semaphore unit to be available, but it never
will become available because it was initialized to 0 and no tasks will
ever release any units to it.

(cherry picked from commit add24d9)
@pgellert pgellert added this to the v24.1.x-next milestone Nov 15, 2024
@pgellert pgellert added the kind/backport PRs targeting a stable branch label Nov 15, 2024
@pgellert pgellert self-assigned this Nov 15, 2024
@pgellert pgellert marked this pull request as ready for review November 15, 2024 18:37
@pgellert pgellert requested review from michael-redpanda and oleiman and removed request for michael-redpanda November 15, 2024 18:37
@pgellert pgellert enabled auto-merge November 15, 2024 18:38
Copy link
Member

@oleiman oleiman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

conflict was in tests only

you mean surrounding code right? the test in the PR didn't actually change?

@pgellert
Copy link
Contributor Author

you mean surrounding code right? the test in the PR didn't actually change?

Apart from that I also changed skip_fips_mode to ok_to_fail_fips to keep it the same as the rest of the file, but that's it.

@vbotbuildovich
Copy link
Collaborator

the below tests from https://buildkite.com/redpanda/redpanda/builds/58131#01933120-7e74-4079-afc7-e9e4a98168a7 have failed and will be retried

partition_balancer_simulator_test_rpunit

@pgellert pgellert merged commit 907c5f9 into redpanda-data:v24.1.x Nov 15, 2024
20 checks passed
@BenPope BenPope modified the milestones: v24.1.x-next, v24.1.18 Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/redpanda kind/backport PRs targeting a stable branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants