[
https://issues.apache.org/jira/browse/HBASE-11567?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14080421#comment-14080421
]
ryan rawson commented on HBASE-11567:
-------------------------------------
Good generally speaking.
The bulk load tests should really verify more behavior. In the 'successful'
cases, what kind of behavior has changed about the HRegion that we could check?
(if anything - it might not be feasible since HRegion isn't SOLID)
I also could imagine a series of integration tests that verify that the data in
the bulk loaded file is readable. There is also some stress tests that involve
loading during concurrent operations and what should happen in those cases.
That is a separate integration test.
commented on 154f10b
hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java:L2748:
I want to see a space between ){
commented on 154f10b
hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java:L2750:
what's the line length max? Are we on Java7, can we use diamonds now?
commented on 154f10b
hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java:L2751:
spacing here, between the : and the ){
commented on 154f10b
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEdit.java:L322:
im a little vaugely unhappy how creating all these structures is spread about,
but that is a greater issue, and we cant fix it now.
commented on 154f10b
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:L3667:
spacing here:
if (log != null) {
commented on 154f10b
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:L3672
:
so the log doesnt really know how to write business logicy entries, so we have
a utility class that does it. So who is really responsible for doing these
things? The obvious design solution is a wrapper around HLog which allows
pluggable log under it, we should file a new HBase JIRA.
one thing that bothers me about this line is we are hiding a pretty heavy duty
IO event, and its not really apparent, maybe change the name to
HLogUtil.writeBulkLoadMarkerAndSync() so its apparent
commented on 154f10b
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBulkLoad.java:L46
:
writes to files = not unit test
commented on 154f10b
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBulkLoad.java:L64:
are these constants? if so, final them, upcaps them
commented on 154f10b
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBulkLoad.java:L76:
Isn't there already a constant that has this in it somewhere else?
the answer is yes, a package protected one in WALEdit - lets not copy magic
constants about, DRY that up
commented on 154f10b
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBulkLoad.java:L121:
hide this (and others like this) behind a static function
commented on 154f10b
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBulkLoad.java:L144
:
space
commented on 154f10b
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBulkLoad.java:L168:
maybe call this
bulkLogWalEditType -- it might read better when it's being used.
> Write bulk load events to WAL
> -----------------------------
>
> Key: HBASE-11567
> URL: https://issues.apache.org/jira/browse/HBASE-11567
> Project: HBase
> Issue Type: Sub-task
> Reporter: Enis Soztutar
> Assignee: Alex Newman
> Attachments: HBASE-11567-v1.patch
>
>
> Similar to writing flush (HBASE-11511), compaction(HBASE-2231) to WAL and
> region open/close (HBASE-11512) , we should persist bulk load events to WAL.
> This is especially important for secondary region replicas, since we can use
> this information to pick up primary regions' files from secondary replicas.
> A design doc for secondary replica replication can be found at HBASE-11183.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
