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

[Python] MetadataRecordBatchReader does not release GIL in to_reader() method #34731

Closed
lupko opened this issue Mar 26, 2023 · 0 comments · Fixed by #34732
Closed

[Python] MetadataRecordBatchReader does not release GIL in to_reader() method #34731

lupko opened this issue Mar 26, 2023 · 0 comments · Fixed by #34732

Comments

@lupko
Copy link
Contributor

lupko commented Mar 26, 2023

Describe the bug, including details regarding any error messages, version, and platform.

MetadataRecordBatchReader.to_reader() does not release GIL as it creates the RecordBatchReader via call to MakeRecordBatchReader.

MakeRecordBatchReader waits for first batch of data to arrive before it returns the RBR => interpreter will essentially 'stop' until the data is available. This can have 'interesting' consequences especially if used on server...

Workaround: first call stream.schema - this waits for the first batch of data while not holding GIL, then follow with stream.to_reader()

Component(s)

FlightRPC, Python

@lupko lupko changed the title [Python] FlightStreamReader does not release GIL in to_reader() method [Python] MetadataRecordBatchReader does not release GIL in to_reader() method Mar 26, 2023
@lidavidm lidavidm added this to the 12.0.0 milestone Mar 27, 2023
lidavidm pushed a commit that referenced this issue Mar 27, 2023
### Rationale for this change

See #34731

### What changes are included in this PR?

Wrap existing call that creates RBR in `with nogil:` context

### Are these changes tested?

No new tests. 

### Are there any user-facing changes?

No

* Closes: #34731

Authored-by: lupko <lubomir.slivka@gooddata.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
ArgusLi pushed a commit to Bit-Quill/arrow that referenced this issue May 15, 2023
…pache#34732)

### Rationale for this change

See apache#34731

### What changes are included in this PR?

Wrap existing call that creates RBR in `with nogil:` context

### Are these changes tested?

No new tests. 

### Are there any user-facing changes?

No

* Closes: apache#34731

Authored-by: lupko <lubomir.slivka@gooddata.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants