[
https://issues.apache.org/jira/browse/HBASE-3417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13126764#comment-13126764
]
[email protected] commented on HBASE-3417:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2379/#review2555
-----------------------------------------------------------
/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
<https://reviews.apache.org/r/2379/#comment5731>
Can we keep these final and CacheConfig immutable, and create a separate
instance of CacheConfig during testing instead?
/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
<https://reviews.apache.org/r/2379/#comment5732>
The same as above.
/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
<https://reviews.apache.org/r/2379/#comment5736>
Using the approach above we can avoid creating new setters.
If it's not feasible, then appending "ForTesting" to these method names
will probably be enough to discourage use of them in production.
/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
<https://reviews.apache.org/r/2379/#comment5733>
Maybe we can avoid these setters using the approach I described above? If
not, it should either be package-private, or the name should include something
like "ForTesting".
/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
<https://reviews.apache.org/r/2379/#comment5734>
The same as above.
/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
<https://reviews.apache.org/r/2379/#comment5737>
This does not return null anymore in the new version? This will probably
result in an IOException down the line, is that handled well? We could throw an
IOException right here as an alternative.
/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
<https://reviews.apache.org/r/2379/#comment5738>
@Michael: This pattern does not allow dashes in the UUID, it only allows
0-9 and a-f.
/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
<https://reviews.apache.org/r/2379/#comment5739>
suffix.length() <= 0 is unnecessary.
/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
<https://reviews.apache.org/r/2379/#comment5740>
Nice test! This goes all the way from client side to HFile, unlike
TestCacheOnWrite.
- Mikhail
On 2011-10-13 06:31:13, Jonathan Gray wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2379/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-13 06:31:13)
bq.
bq.
bq. Review request for hbase, Dhruba Borthakur, Michael Stack, and Mikhail
Bautin.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Adds a new test that fails w/o this fix and changes the naming scheme for
storefiles to use UUID instead of random longs as ascii.
bq.
bq. The big change is that the name of the tmp file used when flushing and
compacting is the same name (but different dir) when moved in place. This
makes it so block names are consistent for COW.
bq.
bq.
bq. This addresses bug HBASE-3417.
bq. https://issues.apache.org/jira/browse/HBASE-3417
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java
1182679
bq. /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java 1182679
bq. /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1182679
bq. /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
1182679
bq. /src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
1182679
bq.
bq. Diff: https://reviews.apache.org/r/2379/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. TestFromClientSide and TestCacheOnWrite both working
bq.
bq.
bq. Thanks,
bq.
bq. Jonathan
bq.
bq.
> CacheOnWrite is using the temporary output path for block names, need to use
> a more consistent block naming scheme
> ------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3417
> URL: https://issues.apache.org/jira/browse/HBASE-3417
> Project: HBase
> Issue Type: Bug
> Components: io, regionserver
> Affects Versions: 0.92.0
> Reporter: Jonathan Gray
> Assignee: Jonathan Gray
> Priority: Critical
> Fix For: 0.92.0
>
> Attachments: HBASE-3417-redux-v1.patch, HBASE-3417-v1.patch,
> HBASE-3417-v2.patch, HBASE-3417-v5.patch
>
>
> Currently the block names used in the block cache are built using the
> filesystem path. However, for cache on write, the path is a temporary output
> file.
> The original COW patch actually made some modifications to block naming stuff
> to make it more consistent but did not do enough. Should add a separate
> method somewhere for generating block names using some more easily mocked
> scheme (rather than just raw path as we generate a random unique file name
> twice, once for tmp and then again when moved into place).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
