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

[Go][Parquet] Memory leaks in parquet/pqarrow #35015

Closed
2 tasks
disq opened this issue Apr 10, 2023 · 0 comments · Fixed by #35973
Closed
2 tasks

[Go][Parquet] Memory leaks in parquet/pqarrow #35015

disq opened this issue Apr 10, 2023 · 0 comments · Fixed by #35973

Comments

@disq
Copy link
Contributor

disq commented Apr 10, 2023

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

Some memory leaks in pqarrow became apparent after adding CheckedAllocator to tests in GH-34330

  • Some in ParquetIOTestSuite.roundTripTable: So many leaks, mostly around: LEAK of 64 bytes FROM github.com/apache/arrow/go/v12/parquet/file.(*primitiveRecordReader).ReserveValues line 231
  • ParquetIOTestSuite.TestSingleEmptyListsColumnReadWrite: This seems to be a whack-a-mole leak, attempting to fix it (at least superficially) breaks other tests: checked_allocator.go:127: LEAK of 448 bytes FROM github.com/apache/arrow/go/v12/parquet/pqarrow.(*listReader).BuildArray line 383

Both have their defer mem.AssertSize(ps.T(), 0) lines commented out so that the checks are disabled.

Component(s)

Go
Parquet

@disq disq added the Type: bug label Apr 10, 2023
@disq disq changed the title [Go] Memory leaks in parquet/pqarrow [Go][Parquet]: Memory leaks in parquet/pqarrow Apr 10, 2023
@zeroshade zeroshade changed the title [Go][Parquet]: Memory leaks in parquet/pqarrow [Go][Parquet] Memory leaks in parquet/pqarrow Apr 10, 2023
zeroshade added a commit that referenced this issue Jun 8, 2023
### 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.

* Closes: #35015

Lead-authored-by: candiduslynx <candiduslynx@gmail.com>
Co-authored-by: Alex Shcherbakov <candiduslynx@users.noreply.github.com>
Co-authored-by: Matt Topol <zotthewizard@gmail.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
@zeroshade zeroshade added this to the 13.0.0 milestone Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment