[GitHub] [arrow] thorfour opened a new issue, #36186: [Go] Have the Allocator return an error

["thorfour (via GitHub)" <gi...@apache.org>]

thorfour opened a new issue, #36186:
URL: https://github.com/apache/arrow/issues/36186

   ### Describe the enhancement requested
   
   We're looking into the ability to limit the amount of memory that we allow a single array/record to consume at a time. 
   
   One avenue that seems like a good fit would be to change the `Allocator` interface from:
   
   https://github.com/apache/arrow/blob/f959a2e05c79351255227a91cb36d6ca39d01a3d/go/arrow/memory/allocator.go#L23-L27
   
   into 
   
   ```go
   type Allocator interface {
       Allocate(size int) ([]byte, error)
       Reallocate(size int, b []byte) []byte
       Free(b []byte)
   }
   ```
   
   Where then an Allocator implementation that performed some internal accounting of the number of bytes currently allocated and could return an error when there's not enough remaining space in the predefined size limit.
   
   ### Component(s)
   
   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@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] zeroshade commented on issue #36186: [Go] Have the Allocator return an error

["zeroshade (via GitHub)" <gi...@apache.org>]

zeroshade commented on issue #36186:
URL: https://github.com/apache/arrow/issues/36186#issuecomment-1601630785

   The only problem I can see with this idea is that there's no way for your accounting to attribute the allocated memory to a specific array or record at allocation time. For example if there are multiple goroutines active and handling building multiple arrays or records, that information isn't passed down to the allocator unless you're manually doing it in your own code in your interactions with the allocator. Which, in that case, you could just check it and throw an error at that point?
   
   That said, I'm not opposed to this change to the interface (although it will result in a very large PR to change everywhere we call Allocate to handle an error and propagate it). Though, this might cause consumers who have their own custom allocators to break which I'd be concerned for.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] zeroshade commented on issue #36186: [Go] Have the Allocator return an error

["zeroshade (via GitHub)" <gi...@apache.org>]

zeroshade commented on issue #36186:
URL: https://github.com/apache/arrow/issues/36186#issuecomment-1601844355

   @thorfour I would welcome such a PR and would review it. Like I said, I'm not opposed to the idea in principle.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] thorfour commented on issue #36186: [Go] Have the Allocator return an error

["thorfour (via GitHub)" <gi...@apache.org>]

thorfour commented on issue #36186:
URL: https://github.com/apache/arrow/issues/36186#issuecomment-1601676351

   > 
   
   In terms of the multi-goroutine issue I think that's more of a composition problem for the end user. They could choose to use a separate allocator for each logical array/record creation. Or in our case we have multiple go routines that are building multiple arrays and we want to limit the total usage of that group of go routines so it would make sense for us to limit it with a single allocator. 
   
   The PR would likely be very large you're right but hopefully it would be relatively straight-forward to review since it would be mostly boiler plate error handling. 
   
   I'm unfamiliar with any compatibility promises that versions have in this library. I think the breakage by changing the interface by addition would at least be fairly easy for consumers to address in their code since they could just amend their return statements to always return nil thus achieving the same interface they had previously.
   
   I'm happy to make the necessary change if this is something that would be accepted.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org