fix(python): read consumer metadata without the consumer lock by ethanlin01x · Pull Request #3888 · apache/iggy

ethanlin01x · 2026-08-15T09:09:39Z

Which issue does this PR address?

Relates to #3776 (Found while reviewing)

Rationale

The synchronous getters on the Python IggyConsumer took the consumer mutex with blocking_lock() while holding the GIL, and consume_messages holds that mutex for the whole consumption run. Reading an attribute during consumption hung the interpreter; reading one from a callback panicked inside the Tokio runtime. Neither is recoverable from Python.

consume = consumer.consume_messages(handle, shutdown_event)
print(consumer.name())   # never returns

What changed?

None of those getters need exclusive access.

name, stream and topic never change after construction, so the Python wrapper snapshots them. The partition id and offsets do change, but already live behind Arcs in the Rust SDK, which now hands them out as IggyConsumerState via IggyConsumer::state(). The wrapper keeps a clone and reads them as atomic loads, never taking the lock.

Additive on the Rust side. On the Python side stream() and topic() now return the identifier directly instead of a PyResult.

Local Execution

Passed
Pre-commit hooks ran

AI Usage

Which tools? Claude
Scope of usage? help implement and write PR description
How did you verify the generated code works correctly? Ran the Python test suite against a real server
Can you explain every line of the code if asked? Yes, all the changes are checked by the human.

codecov · 2026-08-15T09:11:46Z

Codecov Report

❌ Patch coverage is 79.22705% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.03%. Comparing base (37d7eee) to head (9dcc2b7).

Files with missing lines	Patch %	Lines
core/sdk/src/clients/consumer.rs	76.10%	35 Missing and 3 partials ⚠️
foreign/python/src/consumer.rs	87.50%	5 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3888      +/-   ##
============================================
- Coverage     84.03%   84.03%   -0.01%     
  Complexity     1358     1358              
============================================
  Files          1215     1215              
  Lines        170820   170847      +27     
  Branches     138576   138603      +27     
============================================
+ Hits         143547   143566      +19     
+ Misses        23411    23398      -13     
- Partials       3862     3883      +21

Components	Coverage Δ
Rust Core	`84.92% <76.10%> (-0.02%)`	⬇️
Java SDK	`66.67% <ø> (ø)`
C# SDK	`74.96% <ø> (ø)`
Python SDK	`90.06% <89.58%> (-0.04%)`	⬇️
PHP SDK	`85.65% <ø> (ø)`
Node SDK	`95.90% <ø> (+0.09%)`	⬆️
Go SDK	`68.29% <ø> (ø)`

Files with missing lines	Coverage Δ
foreign/python/src/client.rs	`99.85% <100.00%> (+<0.01%)`	⬆️
foreign/python/src/consumer.rs	`81.34% <87.50%> (-0.90%)`	⬇️
core/sdk/src/clients/consumer.rs	`72.52% <76.10%> (-0.36%)`	⬇️

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ethanlin01x · 2026-08-15T10:39:14Z

/request-review @hubcio

ethanlin01x · 2026-08-15T10:39:34Z

/ready

The synchronous getters on IggyConsumer took the consumer mutex with blocking_lock() while holding the GIL, and consume_messages holds that mutex for the whole consumption run. Reading an attribute during consumption hung the interpreter; reading one from a callback panicked inside the Tokio runtime. None of those getters need exclusive access. The name, stream and topic are fixed at construction, and the partition id and offsets live behind Arcs that IggyConsumerState now exposes as a cloneable view. IggyConsumer owns that state and delegates to it, so the Python wrapper reads metadata without the lock. Found while reviewing apache#3776.

hubcio

fix is good, some polishing is needed tho.

three that don't fit on a diff line:

foreign/python/src/consumer.rs:410-411: the lock_owned() guard is shadowed, not dropped, so it spans the whole consume() including the callback await. harmless today, but self.task_locals.lock().await.clone() drops it.
foreign/python/src/consumer.rs:202: tokio::join! waits on the shutdown task even when consume already exited, and that task waits forever on Event.wait(). the new test is only safe because it sets shutdown_event in finally.
nothing regenerates and diffs apache_iggy.pyi in ci - php has that gate, python doesn't. no impact here, separate ticket.

slbotbm · 2026-08-25T09:38:46Z

@hubcio do we want to check the pyi file in ci? i ask because this check will require a complete recompilation of the sdk, and our ci runs are very heavy as it is.

maybe we could instead redesign the ci for ffi sdks instead?

hubcio · 2026-08-25T09:56:47Z

@slbotbm like we discussed on discord, please propose something via issues or discussions.

The flush loops held a DashMap shard read guard across the store await while the poll future can insert a new partition into the same map.

A consumption run holds the consumer mutex while it awaits the callback, so awaiting store_offset or delete_offset from inside one deadlocked. Under AutoCommit.Disabled() that left no manual commit path at all. Both take &self in the Rust SDK, so IggyConsumerState now owns them and the Python wrapper reads through it instead of taking the lock.

tokio::join! waited on a task that parks on Event.wait() forever once consuming has returned on its own, so it is now aborted instead. Signalling a receiver that is already gone is no longer an error, or it would mask the result of the run it was trying to stop. The task locals guard was shadowed rather than dropped, so it spanned the whole consume including the callback await.

IggyConsumerState is public through the prelude and had no Debug. `None` from the offset getters means the partition is untracked, not that nothing was consumed, and stream() / topic() give back an identifier rather than a name.

faulthandler_timeout and faulthandler_exit_on_timeout give the whole suite what one test body was doing by hand.

# Conflicts: # core/sdk/src/clients/consumer.rs # foreign/python/src/consumer.rs

Rust 1.98's clippy flags the enum for a large size difference between variants under -D warnings.

ethanlin01x · 2026-08-25T17:59:09Z

@hubcio Thanks for the review! Also fixed a clippy error while at it.
For the apache_iggy.pyi CI gate, I'll dig into it and open a separate issue for discussion.

ethanlin01x force-pushed the fix/python-consumer-metadata-deadlock branch from 8d0d07d to b73cf67 Compare August 15, 2026 09:28

ethanlin01x marked this pull request as ready for review August 15, 2026 09:41

github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Aug 15, 2026

github-actions Bot requested a review from hubcio August 15, 2026 10:39

ethanlin01x force-pushed the fix/python-consumer-metadata-deadlock branch from 7b7a74e to 5763540 Compare August 20, 2026 13:19

ethanlin01x force-pushed the fix/python-consumer-metadata-deadlock branch from 5763540 to e9c641e Compare August 20, 2026 13:40

hubcio and others added 3 commits August 20, 2026 16:08

Merge branch 'master' into fix/python-consumer-metadata-deadlock

004e7cb

Merge branch 'master' into fix/python-consumer-metadata-deadlock

96afc4c

Merge branch 'master' into fix/python-consumer-metadata-deadlock

b26e033

slbotbm previously approved these changes Aug 23, 2026

View reviewed changes

Merge branch 'master' into fix/python-consumer-metadata-deadlock

ad08ebc

hubcio approved these changes Aug 25, 2026

View reviewed changes

hubcio requested changes Aug 25, 2026

View reviewed changes

Merge branch 'master' into fix/python-consumer-metadata-deadlock

dfaac5d

github-actions Bot added S-waiting-on-author PR is waiting on author response and removed S-waiting-on-review PR is waiting on a reviewer labels Aug 25, 2026

ethanlin01x added 5 commits August 25, 2026 23:06

fix(sdk): snapshot consumed offsets before the store round trip

1ae5a4c

The flush loops held a DashMap shard read guard across the store await while the poll future can insert a new partition into the same map.

refactor: polish the consumer metadata surface

9844949

IggyConsumerState is public through the prelude and had no Debug. `None` from the offset getters means the partition is untracked, not that nothing was consumed, and stream() / topic() give back an identifier rather than a name.

test(python): move the faulthandler watchdog to the pytest ini

c5aa650

faulthandler_timeout and faulthandler_exit_on_timeout give the whole suite what one test body was doing by hand.

ethanlin01x dismissed slbotbm’s stale review via c5aa650 August 25, 2026 16:56

ethanlin01x added 2 commits August 26, 2026 01:30

Merge branch 'master' into fix/python-consumer-metadata-deadlock

29d2c1d

# Conflicts: # core/sdk/src/clients/consumer.rs # foreign/python/src/consumer.rs

fix(bench): box the typed benchmark consumer variants for clippy

9dcc2b7

Rust 1.98's clippy flags the enum for a large size difference between variants under -D warnings.

ethanlin01x requested review from hubcio and slbotbm August 25, 2026 17:57

Navigation Menu

Uh oh!

fix(python): read consumer metadata without the consumer lock - #3888

fix(python): read consumer metadata without the consumer lock#3888
ethanlin01x wants to merge 13 commits into
apache:masterfrom
ethanlin01x:fix/python-consumer-metadata-deadlock

ethanlin01x commented Aug 15, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Aug 15, 2026 •

edited

Loading

Uh oh!

ethanlin01x commented Aug 15, 2026

Uh oh!

ethanlin01x commented Aug 15, 2026

Uh oh!

hubcio left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

slbotbm commented Aug 25, 2026

Uh oh!

hubcio commented Aug 25, 2026

Uh oh!

ethanlin01x commented Aug 25, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ethanlin01x commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR address?

Rationale

What changed?

Local Execution

AI Usage

Uh oh!

codecov Bot commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ethanlin01x commented Aug 15, 2026

Uh oh!

ethanlin01x commented Aug 15, 2026

Uh oh!

hubcio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

slbotbm commented Aug 25, 2026

Uh oh!

hubcio commented Aug 25, 2026

Uh oh!

ethanlin01x commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ethanlin01x commented Aug 15, 2026 •

edited

Loading

codecov Bot commented Aug 15, 2026 •

edited

Loading

ethanlin01x commented Aug 25, 2026 •

edited

Loading