[jira] [Commented] (HBASE-19607) throw the exception if memstore is incompetent to do the deep clone for cell
[
https://issues.apache.org/jira/browse/HBASE-19607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16323841#comment-16323841
]
Chia-Ping Tsai commented on HBASE-19607:
bq. Maybe HBaseIOE?
The HBaseIOE is just another IOE. If we want to add more meaning to the
exception, we should add an new exception. Maybe UnsupportedCellException?
Think it over. Passing the incorrect cell is not a common case. The cells from
RPC layer are always of ExtendedCell. The cells from mutation#add are also
wrapped to ExtendedCell. The way which can smuggle the cell into MSLAB should
be viewed as a system bug like NPE. I feel a unchecked runtime exception is
more suitable.
> throw the exception if memstore is incompetent to do the deep clone for cell
>
>
> Key: HBASE-19607
> URL: https://issues.apache.org/jira/browse/HBASE-19607
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Yung-An He
>Priority: Trivial
>
> We must to clone the cell since the cell is backed by the reusable byte
> array. Also, we assume all cells passed to AbstractMemStore is of
> ExtendedCell. Not only is the type check unnecessary, but it also confuses
> the readers.
> {code:title=AbstractMemStore.java}
> private static Cell deepCopyIfNeeded(Cell cell) {
> if (cell instanceof ExtendedCell) {
> return ((ExtendedCell) cell).deepClone();
> }
> return cell;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19607) throw the exception if memstore is incompetent to do the deep clone for cell
[
https://issues.apache.org/jira/browse/HBASE-19607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16323816#comment-16323816
]
Yung-An He commented on HBASE-19607:
I found the add() method in MemStore interface defined without throws any
exception.
If we want to throw the exception when memstore is incompetent to do the deep
clone, we can't add exception to signature.
Should we refactor the MemStore interface to add exception to signature?
Or reference the method AbstractMultiFileWriter.java, log the error via
LoggerFactory.getLogger?
{code:title=AbstractMultiFileWriter.java}
public List abortWriters() {
List paths = new ArrayList<>();
for (StoreFileWriter writer : writers()) {
try {
if (writer != null) {
paths.add(writer.getPath());
writer.close();
}
} catch (Exception ex) {
LOG.error("Failed to close the writer after an unfinished compaction.",
ex);
}
}
return paths;
}
{code}
> throw the exception if memstore is incompetent to do the deep clone for cell
>
>
> Key: HBASE-19607
> URL: https://issues.apache.org/jira/browse/HBASE-19607
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Yung-An He
>Priority: Trivial
>
> We must to clone the cell since the cell is backed by the reusable byte
> array. Also, we assume all cells passed to AbstractMemStore is of
> ExtendedCell. Not only is the type check unnecessary, but it also confuses
> the readers.
> {code:title=AbstractMemStore.java}
> private static Cell deepCopyIfNeeded(Cell cell) {
> if (cell instanceof ExtendedCell) {
> return ((ExtendedCell) cell).deepClone();
> }
> return cell;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19607) throw the exception if memstore is incompetent to do the deep clone for cell
[
https://issues.apache.org/jira/browse/HBASE-19607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16323572#comment-16323572
]
Yung-An He commented on HBASE-19607:
HBaseIOE +1 .
I would use HBaseIOE instead of IOE.
Is it necessary to create a new issue to fix setSequenceId and change the
Exception type from IOException to HBaseIOE?
> throw the exception if memstore is incompetent to do the deep clone for cell
>
>
> Key: HBASE-19607
> URL: https://issues.apache.org/jira/browse/HBASE-19607
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Yung-An He
>Priority: Trivial
>
> We must to clone the cell since the cell is backed by the reusable byte
> array. Also, we assume all cells passed to AbstractMemStore is of
> ExtendedCell. Not only is the type check unnecessary, but it also confuses
> the readers.
> {code:title=AbstractMemStore.java}
> private static Cell deepCopyIfNeeded(Cell cell) {
> if (cell instanceof ExtendedCell) {
> return ((ExtendedCell) cell).deepClone();
> }
> return cell;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19607) throw the exception if memstore is incompetent to do the deep clone for cell
[
https://issues.apache.org/jira/browse/HBASE-19607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16323556#comment-16323556
]
stack commented on HBASE-19607:
---
Maybe HBaseIOE?
http://blog.tsunanet.net/2012/04/apache-hadoop-abuse-ioexception.html
> throw the exception if memstore is incompetent to do the deep clone for cell
>
>
> Key: HBASE-19607
> URL: https://issues.apache.org/jira/browse/HBASE-19607
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Yung-An He
>Priority: Trivial
>
> We must to clone the cell since the cell is backed by the reusable byte
> array. Also, we assume all cells passed to AbstractMemStore is of
> ExtendedCell. Not only is the type check unnecessary, but it also confuses
> the readers.
> {code:title=AbstractMemStore.java}
> private static Cell deepCopyIfNeeded(Cell cell) {
> if (cell instanceof ExtendedCell) {
> return ((ExtendedCell) cell).deepClone();
> }
> return cell;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19607) throw the exception if memstore is incompetent to do the deep clone for cell
[
https://issues.apache.org/jira/browse/HBASE-19607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16323514#comment-16323514
]
Chia-Ping Tsai commented on HBASE-19607:
Similar to setSeq? IOE is ok to me.
{code}
public static void setSequenceId(Cell cell, long seqId) throws IOException {
if (cell instanceof ExtendedCell) {
((ExtendedCell) cell).setSequenceId(seqId);
} else {
throw new IOException(new UnsupportedOperationException(
"Cell is not of type " + ExtendedCell.class.getName()));
}
}
{code}
> throw the exception if memstore is incompetent to do the deep clone for cell
>
>
> Key: HBASE-19607
> URL: https://issues.apache.org/jira/browse/HBASE-19607
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Yung-An He
>Priority: Trivial
>
> We must to clone the cell since the cell is backed by the reusable byte
> array. Also, we assume all cells passed to AbstractMemStore is of
> ExtendedCell. Not only is the type check unnecessary, but it also confuses
> the readers.
> {code:title=AbstractMemStore.java}
> private static Cell deepCopyIfNeeded(Cell cell) {
> if (cell instanceof ExtendedCell) {
> return ((ExtendedCell) cell).deepClone();
> }
> return cell;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19607) throw the exception if memstore is incompetent to do the deep clone for cell
[
https://issues.apache.org/jira/browse/HBASE-19607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16323499#comment-16323499
]
Yung-An He commented on HBASE-19607:
Sorry for replying too late,[~chia7712]
I am working on this issue and got some question.
What kind of exception should be thrown if memstore is incapable of doing the
deep clone? HBaseException?
Or just return the result of deepclone() as below?
{code:java}
private static Cell deepCopyIfNeeded(Cell cell) {
return ((ExtendedCell) cell).deepClone();
}
{code}
> throw the exception if memstore is incompetent to do the deep clone for cell
>
>
> Key: HBASE-19607
> URL: https://issues.apache.org/jira/browse/HBASE-19607
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Yung-An He
>Priority: Trivial
>
> We must to clone the cell since the cell is backed by the reusable byte
> array. Also, we assume all cells passed to AbstractMemStore is of
> ExtendedCell. Not only is the type check unnecessary, but it also confuses
> the readers.
> {code:title=AbstractMemStore.java}
> private static Cell deepCopyIfNeeded(Cell cell) {
> if (cell instanceof ExtendedCell) {
> return ((ExtendedCell) cell).deepClone();
> }
> return cell;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19607) throw the exception if memstore is incompetent to do the deep clone for cell
[
https://issues.apache.org/jira/browse/HBASE-19607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16318107#comment-16318107
]
Chia-Ping Tsai commented on HBASE-19607:
[~stana] Any updates?
> throw the exception if memstore is incompetent to do the deep clone for cell
>
>
> Key: HBASE-19607
> URL: https://issues.apache.org/jira/browse/HBASE-19607
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Yung-An He
>Priority: Trivial
>
> We must to clone the cell since the cell is backed by the reusable byte
> array. Also, we assume all cells passed to AbstractMemStore is of
> ExtendedCell. Not only is the type check unnecessary, but it also confuses
> the readers.
> {code:title=AbstractMemStore.java}
> private static Cell deepCopyIfNeeded(Cell cell) {
> if (cell instanceof ExtendedCell) {
> return ((ExtendedCell) cell).deepClone();
> }
> return cell;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19607) throw the exception if memstore is incompetent to do the deep clone for cell
[
https://issues.apache.org/jira/browse/HBASE-19607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16303723#comment-16303723
]
Chia-Ping Tsai commented on HBASE-19607:
sure
> throw the exception if memstore is incompetent to do the deep clone for cell
>
>
> Key: HBASE-19607
> URL: https://issues.apache.org/jira/browse/HBASE-19607
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Priority: Trivial
>
> We must to clone the cell since the cell is backed by the reusable byte
> array. Also, we assume all cells passed to AbstractMemStore is of
> ExtendedCell. Not only is the type check unnecessary, but it also confuses
> the readers.
> {code:title=AbstractMemStore.java}
> private static Cell deepCopyIfNeeded(Cell cell) {
> if (cell instanceof ExtendedCell) {
> return ((ExtendedCell) cell).deepClone();
> }
> return cell;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19607) throw the exception if memstore is incompetent to do the deep clone for cell
[
https://issues.apache.org/jira/browse/HBASE-19607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16303717#comment-16303717
]
Yung-An He commented on HBASE-19607:
Hi [~chia7712],
Did you start to work on this?
If not, could you assign to me? Thanks.
> throw the exception if memstore is incompetent to do the deep clone for cell
>
>
> Key: HBASE-19607
> URL: https://issues.apache.org/jira/browse/HBASE-19607
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Priority: Trivial
>
> We must to clone the cell since the cell is backed by the reusable byte
> array. Also, we assume all cells passed to AbstractMemStore is of
> ExtendedCell. Not only is the type check unnecessary, but it also confuses
> the readers.
> {code:title=AbstractMemStore.java}
> private static Cell deepCopyIfNeeded(Cell cell) {
> if (cell instanceof ExtendedCell) {
> return ((ExtendedCell) cell).deepClone();
> }
> return cell;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
