hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sergey Shelukhin (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-8497) Protobuf WAL also needs a trailer
Date Thu, 09 May 2013 00:55:15 GMT

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

Sergey Shelukhin commented on HBASE-8497:
-----------------------------------------

{code}
+message WALTrailer {
+}
{code}
Nit: please put above the custom messages section.

bq. +     * returns the WALTrailer of the current HLog. It may be null in case of legacy WAL
files.
Or failures?


bq. +    LOG.info("After reading the trailer,: "+this.positionToRead +", "+this.fileLength);
Nit: spaces; ",:"

bq. +    if (trailerPresent && this.inputStream.getPos() >= this.positionToRead)
return false;
It would be very suspicious if it was ">", I think we need to error out in this case.
In fact probably error out at the end of the read if we read into trailer?

bq. +      entry.getKey().readFieldsFromPb(walKey, this.byteStringUncompressor);
Spurious whitespace change.

bq. +  private boolean trailerWritten;
Nit: initialize explicitly?
 
bq. +  WALTrailer trailer;
private

{code}
+  private void writeWALTrailer() throws IOException {
+    if (trailer == null) trailer = WALTrailer.newBuilder().build();
+    trailer.writeTo(output);
{code}
This usage pattern is not very clean, either external user can set the trailer or writer can
create it.
Perhaps it should be mandatory and passed as parameter in ctor? That way HLog stuff can pass
in trailer with necessary values, and writer can augment it with more fields and write it
out.
{code}
+    this.output.writeInt(trailer.getSerializedSize());
{code}
Are you sure bytebuffer's readInt and this writeInt will always be compatible with regard
to format and endian-ness? Just checking, I have no idea. Perhaps Bytes stuff can be used
in both places.


{code}
+        out.write(corrupted_bytes, 0, fileSize
+          - (32 + ProtobufLogReader.PB_WAL_COMPLETE_MAGIC.length + Bytes.SIZEOF_INT));
{code}
Is another test for corrupt trailer needed?

                
> Protobuf WAL also needs a trailer 
> ----------------------------------
>
>                 Key: HBASE-8497
>                 URL: https://issues.apache.org/jira/browse/HBASE-8497
>             Project: HBase
>          Issue Type: Sub-task
>          Components: Protobufs, wal
>    Affects Versions: 0.95.1
>            Reporter: Enis Soztutar
>            Assignee: Himanshu Vashishtha
>             Fix For: 0.98.0, 0.95.1
>
>         Attachments: HBASE-8497-v0.patch
>
>
> New Protobuf WAL has a header, but we will probably need a trailer as well, reserved
for later usage. 
> Right now, we can we just serialize an empty trailer, but putting more metadata there,
like range of sequence_id's, region names, table names etc might be needed in the future.


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message