FIX: fix reader not getting closed. #1474
Conversation
this lead to s3a connection pool timeout
|
Looks like there is no need to create another reader. I can just pass in existing reader @rdblue do you know if this is better fix ? |
|
Thanks, @sudssf! I think we should just close the file for now, and improve the way this works in a follow-up. |
This reverts commit 0c47ba8.
|
|
||
| try(ParquetFileReader fileReader = newReader(this.file, ParquetReadOptions.builder().build())) { |
There was a problem hiding this comment.
Nit: missing space between try and (. This will probably fail validation.
There was a problem hiding this comment.
While you're changing this, can you remove this. from the variable reference? We use this. to distinguish setting instance fields, not getting instance fields.
There was a problem hiding this comment.
yup I will revert to code you posted above :)
sudssf
force-pushed
the
fix-reader-leak
branch
2 times, most recently
from
aa8f42a to
2b257db
Compare
sudssf
force-pushed
the
fix-reader-leak
branch
from
2b257db to
0e7444c
Compare
|
Thanks for the quick fix, @sudssf! I've merged this to master. |
thanks I am happy to add tests to catch this kind of issues in future. we build end to end integration tests using local s3a which help capture some of these bugs. let me know if you think it will add value. |
|
Sure, more tests are always welcome. What do you have in mind to catch unclosed files? |
this lead to s3a connection pool timeout