hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-3750) HTablePool.putTable() should call table.flushCommits()
Date Sun, 10 Apr 2011 00:55:05 GMT

    [ https://issues.apache.org/jira/browse/HBASE-3750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13018003#comment-13018003
] 

jiraposter@reviews.apache.org commented on HBASE-3750:
------------------------------------------------------



bq.  On 2011-04-09 22:05:45, Gary Helmling wrote:
bq.  > What is the motivation for calling flushCommits() automatically here?  Are we trying
to save client developers from writing buggy code?
bq.  > 
bq.  > The downside I see is that it's easy to envision a case where I use HTablePool for
batch loading, but don't want to actually flush after every cycle of: get table, perform operation,
return table.  This change would prevent me from doing that and force me either write my own
pool or somehow work around it.
bq.  > 
bq.  > In this case it both fails the obviousness test for me and limits what I can easily
do as a developer.  What is the upside?  Is it sufficient to balance out the limitations?

This addition is certainly defensive.
Consider what could happen before this patch, putTable() didn't guarantee that the table instance
would be put back into the queue (because of size limit of the queue). The user would risk
losing data.
Since putTable(tableA) followed by getTable() call doesn't guarantee that tableA would be
returned, I wonder how the user planned to finally flush all the buffered data to the underlying
table. Consider, that getTable() would always return an HTableInterface instance, he/she couldn't
just call getTable() repeatedly and flush the instance's (buffered) data.


bq.  On 2011-04-09 22:05:45, Gary Helmling wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/client/HTablePool.java, line 126
bq.  > <https://reviews.apache.org/r/573/diff/1/?file=15585#file15585line126>
bq.  >
bq.  >     This seems dangerous and unexpected from a client code point of view.  I wouldn't
expect returning the table to a pool to throw a RuntimeException that could potentially cause
my client application to exit.

Disclaimer: I didn't invent this piece of code. I got it from HTableFactory.releaseHTableInterface()
If we think that Lars' suggestion is good, we should accept this piece of code.


- Ted


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/573/#review409
-----------------------------------------------------------


On 2011-04-09 19:48:31, Ted Yu wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/573/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-04-09 19:48:31)
bq.  
bq.  
bq.  Review request for hbase and Lars George.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Currently HTablePool.putTable() doesn't call table.flushCommits()
bq.  If AutoFlush is disabled for table instance, we should call table.flushCommits().
bq.  
bq.  When HTable instance is discarded in putTable(), we should call tableFactory.releaseHTableInterface().
bq.  
bq.  
bq.  This addresses bug HBASE-3750.
bq.      https://issues.apache.org/jira/browse/HBASE-3750
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    /src/main/java/org/apache/hadoop/hbase/client/HTablePool.java 1090500 
bq.  
bq.  Diff: https://reviews.apache.org/r/573/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  TestHTablePool passes.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Ted
bq.  
bq.



> HTablePool.putTable() should call table.flushCommits()
> ------------------------------------------------------
>
>                 Key: HBASE-3750
>                 URL: https://issues.apache.org/jira/browse/HBASE-3750
>             Project: HBase
>          Issue Type: Bug
>          Components: client
>    Affects Versions: 0.90.1
>            Reporter: Ted Yu
>            Assignee: Ted Yu
>         Attachments: 3750.txt
>
>
> Currently HTablePool.putTable() doesn't call table.flushCommits()
> This may turn out to be surprise for users
> When HTable instance is discarded in putTable(), we should call tableFactory.releaseHTableInterface().

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message