-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
GH-35015: [Go] Fix parquet memleak #35973
Conversation
…ptyListsColumnReadWrite`
|
d4fc6a5
to
3425d41
Compare
3425d41
to
5751998
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zeroshade I've cracked the mem leaks issue.
The extra code pieces (some are dirty and some asserts are incorrect for maps) I used to find the issues were removed in a single commit (so you can take a look): 0f26525
5751998
to
7571dfe
Compare
@zeroshade I guess the value type issue is caused by #35899. Is there some place I can fix the (generated?) code to make it pass? |
Looks like you've got some tests failing with |
Yeah, I forgot to remove the extra release for mem buffer (as I moved it to the initialization place): bf0e4a9 |
@zeroshade I've fixed the extra release in bf0e4a9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM just one nitpick here
@zeroshade just a mention that everything has passed & it only waits for you to merge |
Benchmark runs are scheduled for baseline = 8b5919d and contender = 17311b6. 17311b6 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Rationale for this change
Some memory leaks resulted in partially skipped memory checks in pqarrow package.
This PR brings the checks back.
What changes are included in this PR?
Releases in proper places.
Are these changes tested?
Yes, the tests from #35015 are fully enabled now.
Are there any user-facing changes?
No.