hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "stack (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-14501) NPE in replication with TDE
Date Mon, 28 Sep 2015 20:53:04 GMT

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

stack commented on HBASE-14501:
-------------------------------

What is TDE?

Return of null trying to parse a KV from stream looks like it has been around for a long time
moved to KeyValueUtil from KeyValue itself as part of this:

    HBASE-10800 - Use CellComparator instead of KVComparator (Ram)

The comment is:

   * @return Created KeyValue OR if we find a length of zero, we will return
   *         null which can be useful marking a stream as done.

bq. However, the contract for the Decoder.parseCell() is not clear whether returning null
is acceptable or not. 

It is totally useless. It has no doc. I would think that it should at least note that a null
could come back but  would imagine as user that you should not go in here unless something
to parse..... and throw an exception if failed to parse... that would be 'cleaner' than what
is here now.

bq. The other Decoders (CompressedKvDecoder, CellCodec, etc) do not return null while KeyValueCodec
does.

This seems like the proper behavior.

On the is.available, that came in here:

Author: Michael Stack <stack@apache.org>
Date:   Sat Feb 23 22:38:58 2013 +0000

    HBASE-7899 Cell block building tools: Cell codec and means of iterating an objects Cells

    git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1449420 13f79535-47bb-0310-9956-ffa450edef68

Can you redo your bit about is.available? There are some bits missing which makes it hard
to read. I think you are trying to say relying on is.available is dodgy... which yeah, generally
is (though I put it here I believe).

bq. What should be the interface for Decoder.parseCell()? Can it return null?

To be more correct, it should throw an exception but it will probably be highly disruptive
of current Cell/KV parsing.

bq. How to properly fix BaseDecoder.advance() to not rely on available() call.

I'm not sure how to do this. We want the Interface to be dumb and exist out in hbase-common....
IS doesn't have much in it   Could read to EOF I suppose.

This is an ugly one Enis.



> NPE in replication with TDE
> ---------------------------
>
>                 Key: HBASE-14501
>                 URL: https://issues.apache.org/jira/browse/HBASE-14501
>             Project: HBase
>          Issue Type: Bug
>            Reporter: Enis Soztutar
>
> We are seeing a NPE when replication (or in this case async wal replay for region replicas)
is run on top of an HDFS cluster with TDE configured.
> This is the stack trace:
> {code}
> java.lang.NullPointerException
>         at org.apache.hadoop.hbase.CellUtil.matchingRow(CellUtil.java:370)
>         at org.apache.hadoop.hbase.replication.regionserver.ReplicationSource.countDistinctRowKeys(ReplicationSource.java:649)
>         at org.apache.hadoop.hbase.replication.regionserver.ReplicationSource.readAllEntriesToReplicateOrNextFile(ReplicationSource.java:450)
>         at org.apache.hadoop.hbase.replication.regionserver.ReplicationSource.run(ReplicationSource.java:346)
> {code}
> This stack trace can only happen if WALEdit.getCells() returns an array containing null
entries. I believe this happens due to {{KeyValueCodec.parseCell()}} uses {{KeyValueUtil.iscreate()}}
which returns null in case of EOF at the beginning. However, the contract for the Decoder.parseCell()
is not clear whether returning null is acceptable or not. The other Decoders (CompressedKvDecoder,
CellCodec, etc) do not return null while KeyValueCodec does. 
> BaseDecoder has this code: 
> {code}
>   public boolean advance() throws IOException {
>     if (!this.hasNext) return this.hasNext;
>     if (this.in.available() == 0) {
>       this.hasNext = false;
>       return this.hasNext;
>     }
>     try {
>       this.current = parseCell();
>     } catch (IOException ioEx) {
>       rethrowEofException(ioEx);
>     }
>     return this.hasNext;
>   }
> {code}
> which is not correct since it uses {{IS.available()}} not according to the javadoc: (https://docs.oracle.com/javase/7/docs/api/java/io/InputStream.html#available()).
DFSInputStream implements {{available()}} as the remaining bytes to read from the stream,
so we do not see the issue there. {{CryptoInputStream.available()}} does a similar thing but
see the issue. 
> So two questions: 
>  - What should be the interface for Decoder.parseCell()? Can it return null? 
>  - How to properly fix  BaseDecoder.advance() to not rely on {{available()}} call. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message