You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "deepak-2605 (via GitHub)" <gi...@apache.org> on 2026/08/13 15:18:18 UTC
[I] fix(table): memory leak in RollingDataWriter cancellation path (position-delete fanout) [iceberg-go]
deepak-2605 opened a new issue, #1825:
URL: https://github.com/apache/iceberg-go/issues/1825
### Apache Iceberg version
main (development)
### Please describe the bug 🐞
TestPositionDeletePartitionedFanoutWriterEarlyStopCancelsRecordProduction (table/pos_delete_partitioned_fanout_writer_test.go) fails consistently on main. Arrow's CheckedAllocator catches unreleased allocations when a caller breaks out of positionDeletePartitionedFanoutWriter.Write() early.
**Expected**: breaking out of Write()'s result iterator after the first data file should (a) stop the upstream record producer promptly, and (b) release every Arrow allocation made along the way — mem.CurrentAlloc() should be 0.
**Actual**: the checked allocator reports stranded allocations every time.
Steps to reproduce:
`git clone https://github.com/apache/iceberg-go.git
cd iceberg-go
go test ./table/ -run TestPositionDeletePartitionedFanoutWriterEarlyStopCancelsRecordProduction -count=5 -v`
Fails 5/5 runs. The leaked-byte total differs on every run (108160, 104640, 184320, 291840, 32320 bytes across five local runs), which points to a timing race rather than a deterministic leak , the amount stranded depends on how many batches happen to be in flight at the moment cancellation fires.
**Stack trace (representative, from one run):**
checked_allocator.go:186: LEAK of 64 bytes FROM
github.com/apache/arrow-go/v18/arrow/array.(*bufferBuilder).resize+b0
.../arrow-go/v18@v18.7.0/arrow/array/bufferbuilder.go:92
checked_allocator.go:186: LEAK of 64 bytes FROM
github.com/apache/arrow-go/v18/arrow/array.(*builder).init+c0
.../arrow-go/v18@v18.7.0/arrow/array/builder.go:145
checked_allocator.go:186: LEAK of 64 bytes FROM
github.com/apache/arrow-go/v18/arrow/array.(*Int64Builder).newData+2f
.../arrow-go/v18@v18.7.0/arrow/array/numericbuilder.gen.go:183
panic.go:694: invalid memory size exp=0, got=40320
--- FAIL: TestPositionDeletePartitionedFanoutWriterEarlyStopCancelsRecordProduction (0.02s)
**Suspected area**: [#1595](https://github.com/apache/iceberg-go/pull/1595) hardened the shared startRecordFeeder/yieldDataFiles plumbing in partitioned_fanout_writer.go against this exact class of bug (retaining/releasing batches left in inputRecordsCh on early cancellation) and added this test as a regression guard. The leak appears to live downstream of that fix, in the position-delete-specific path ,either processBatch's handoff into RollingDataWriter.Add (pos_delete_partitioned_fanout_writer.go:126), or in how RollingDataWriter.stream's per-writer recordCh and bootstrap buffer are torn down when abortAll() (cancellation) rather than closeAll() (clean finish) stops the pipeline. That path is structurally separate from the channel [#1595](https://github.com/apache/iceberg-go/pull/1595) fixed, so it looks like it was never covered by the earlier hardening.
Will be working on a fix for this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] fix(table): memory leak in RollingDataWriter cancellation path (position-delete fanout) [iceberg-go]
Posted by "badalprasadsingh (via GitHub)" <gi...@apache.org>.
badalprasadsingh commented on issue #1825:
URL: https://github.com/apache/iceberg-go/issues/1825#issuecomment-5330728555
I confirmed your path finding on my `macOS`.
Right now, `LoadLocationProvider("C:\...")` makes `NewDataLocation`, `NewMetadataLocation` and `NewTableMetadataFileLocation` all silently return the bare root, `err = nil`. Similar at the catalog layer: `url.JoinPath("C:\wh", "ns.db", "tbl")` returns `c:\wh`, so every table in every namespace collapses onto one path.
Before you file, please check #1262. It already tracks this ("data and metadata dirs never nest") and names `url.Parse` drive letters as the suspected cause; it's why windows-latest is commented out in CI. It's, right now, unassigned with no comments, so post your root-cause detail there as a comment rather than opening a duplicate. Also see PR #1644, adding portable path normalization in `internal/fileuri` is likely reusable, so coordinate rather than write a third normalizer.
But, for this issue: narrow the title / body to the `CompareAndDelete`/`abortAll` leak and go ahead with that PR now. It reproduces without Windows, any mid-stream write error strands the batches buffered in `recordCh`.
cc: @laskoviymishka
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] fix(table): memory leak in RollingDataWriter cancellation path (position-delete fanout) [iceberg-go]
Posted by "badalprasadsingh (via GitHub)" <gi...@apache.org>.
badalprasadsingh commented on issue #1825:
URL: https://github.com/apache/iceberg-go/issues/1825#issuecomment-5301520039
This detailed report is very useful - thanks for this.
I tried reproducing it - but could **not** reproduce - passes `200x`, and under -race, on main (`02b3fe9`).
But you found a real leak on a different trigger: when `stream` errors early, its `CompareAndDelete` deregisters the writer, so `abortAll` never drains `recordCh`. I measured around `2.4-5KB` stranded per run.
Would be great if you could share your version of go and along with arrow-go.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] fix(table): memory leak in RollingDataWriter cancellation path (position-delete fanout) [iceberg-go]
Posted by "deepak-2605 (via GitHub)" <gi...@apache.org>.
deepak-2605 commented on issue #1825:
URL: https://github.com/apache/iceberg-go/issues/1825#issuecomment-5326684669
@badalprasadsingh Did you get a chance to take a look at it?
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] RollingDataWriter: batches queued in recordCh are stranded when stream exits on a mid-stream write error [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade closed issue #1825: RollingDataWriter: batches queued in recordCh are stranded when stream exits on a mid-stream write error
URL: https://github.com/apache/iceberg-go/issues/1825
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] fix(table): memory leak in RollingDataWriter cancellation path (position-delete fanout) [iceberg-go]
Posted by "deepak-2605 (via GitHub)" <gi...@apache.org>.
deepak-2605 commented on issue #1825:
URL: https://github.com/apache/iceberg-go/issues/1825#issuecomment-5302036936
Versions: Go 1.26.3 on windows/amd64, github.com/apache/arrow-go/v18 at v18.7.0.
I dug further and found the actual trigger on my end, and it turns out it's not a timing race at all, it's fully deterministic. require.NoError(t, writeErr) was failing on the very first data file, every single run, with:
`open c:\Users\...\TestPositionDeletePartitionedFanoutWriterEarlyStopCancelsRecordP760127849\001: is a directory`
Here's why. t.TempDir() returns a raw Windows path (that trailing \001 is just Go's own temp-dir sequence suffix, nothing to do with iceberg-go). That raw path gets passed straight into url.Parse() in newWriterFactory/LoadLocationProvider (table/locations.go) with no file:// prefix. Turns out Go's URL parser reads C: as a scheme when you do that, and dumps everything else into .Opaque instead of .Path:
`u, _ := url.Parse(`C:\Users\deepa\...\TestXxx...`)
// Scheme="c" Opaque="\Users\deepa\...\TestXxx..." Path=""`
Every location provider builds its paths through .JoinPath(...), which only ever touches .Path, and url.URL.String() ignores .Path completely once .Opaque is set. So every .JoinPath() call silently does nothing, and NewDataLocation(fileName) just keeps returning the bare warehouse root, no matter what filename or partition you pass it. os.Create then collides with that directory, since it's literally the temp dir itself, and fails every time on Windows.
And that failure is exactly what causes the leak you found. It drives RollingDataWriter.stream() into the error-return path, which deregisters itself via CompareAndDelete before abortAll() ever runs, which is the same gap you already spotted. I traced it directly and got this:
`deregister ... selfDeregistered=true recordChLenAtDeregister=15
fanoutWorkers.Wait done err="...is a directory"
calling abortAll
abortAll writerCount=0`
So really there are two bugs tangled together here. Your CompareAndDelete/abortAll gap is real and is the actual leak mechanism in both our runs. But there's also this separate url.Parse issue on Windows paths, which is what makes that gap fire on literally every attempt on my end instead of rarely. And it's not just a test problem either, any real user pointing a local or Hadoop-catalog warehouse at a Windows path without a file:// prefix would hit this and silently write every file to the wrong location.
Given how different those two things are, would it make sense to split the path bug into its own issue, and narrow this one down to just the CompareAndDelete/abortAll fix? Happy to file it if so and work on both.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org