fix: BigtableSession is never closed by Reader causing "ManagedChanne…#2782
Conversation
…l allocation site" exceptions
google-cla
Bot
added
the
cla: yes
product-auto-label
Bot
added
the
api: bigtable
|
|
||
| @VisibleForTesting | ||
| protected void setSession(BigtableSession session) { | ||
| void setSession(BigtableSession session) { |
There was a problem hiding this comment.
why are we changing the visibility of these methods?
There was a problem hiding this comment.
As I can see, these methods are introduced for the sake of testability, they are not supposed to be invoked not by tests,
so there is no reason for them to belong to the public API.
There was a problem hiding this comment.
Got it - since this is not required for this fix, let's revert these and maybe revisit the public surface in a different issue/PR just in case this would inadvertently break something :)
There was a problem hiding this comment.
Yes, I agree, fixing multiple issues in the same commit is not a good practice.
Restored, please take a look when you have time
|
|
||
| @VisibleForTesting | ||
| protected void setSession(BigtableSession session) { | ||
| void setSession(BigtableSession session) { |
There was a problem hiding this comment.
Got it - since this is not required for this fix, let's revert these and maybe revisit the public surface in a different issue/PR just in case this would inadvertently break something :)
…l allocation site" exceptions
|
Opened #2785 to track this for 1.x branch as well |
googleapis#2782) * fix: BigtableSession is never closed by Reader causing "ManagedChannel allocation site" exceptions * fix: BigtableSession is never closed by Reader causing "ManagedChannel allocation site" exceptions
googleapis#2782) * fix: BigtableSession is never closed by Reader causing "ManagedChannel allocation site" exceptions * fix: BigtableSession is never closed by Reader causing "ManagedChannel allocation site" exceptions (cherry picked from commit 5340db5)
…g "ManagedChanne… (googleapis#2782)" This reverts commit 5340db5
* Reader.close(): fixes googleapis#2658 where original fix googleapis#2782 was rolled back by googleapis#2871 and googleapis#2873. As we don’t use “session” here anymore, I believe we should close “connection” here. * AbstractCloudBigtableTableDoFn.tearDown(): If an exception happened in ProcessElement, FinishBundle is not executed. We should clean up resources in TearDown. You can see the similar code with BigQueryIO apache/beam#14949.
2 participants
Fixes #2658