Re: [I] flightsql.BaseServer does not exhaust channel in case of an error writing StreamChunks in DoGet [arrow-go]

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

zeroshade commented on issue #435:
URL: https://github.com/apache/arrow-go/issues/435#issuecomment-3063889620

   That's a good catch!
   
   Rather than adding an explicit function like that, we can handle this with a defer:
   
   ```go
   defer func() {
           for chunk := range cc {
                   if chunk.Data != nil {
                         chunk.Data.Release()
                  }
           }
   }()
   
   wr := flight.NewRecordWriter(stream, ipc.WithSchema(sc))
   defer wr.Close()
   
   for chunk := range cc {
        ....
   }
   ```
   
   that should ensure we always drain it regardless of which condition occurs. 
   
   > I can take care of the PR if need be.
   
   That would be fantastic! I should automatically get tagged for review, but feel free to manually tag me if it doesn't do that.
   
   > I've been testing that manually. Do you think I can test the behaviour rigorously?
   
   It should hopefully not be too difficult to use the `testServer` framework in `server_test.go` to manually create a client that makes a DoGet request and then closes the connection such that a `CheckedAllocator` in the server could detect that some records weren't released (failing without the above fix). If you need help setting it up reach out on the PR and I'll try to find some time to help put something together.


-- 
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