[jira] [Commented] (HBASE-18554) Append#add doesn't check the row of passed cell
[
https://issues.apache.org/jira/browse/HBASE-18554?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16200680#comment-16200680
]
Chia-Ping Tsai commented on HBASE-18554:
bq. Fix versions have been set for various 1.x. We can't accept compat breaking
changes to this public API there so please make the branch-1 patches
accordingly.
ok. Perhaps the 3.0 is an ideal target.
> Append#add doesn't check the row of passed cell
> ---
>
> Key: HBASE-18554
> URL: https://issues.apache.org/jira/browse/HBASE-18554
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Reid Chan
> Labels: beginner
> Fix For: 3.0.0
>
> Attachments: HBASE-18554.master.001.patch,
> HBASE-18554.master.002.patch
>
>
> {code}
> @SuppressWarnings("unchecked")
> public Append add(final Cell cell) {
> // Presume it is KeyValue for now.
> byte [] family = CellUtil.cloneFamily(cell);
> List list = this.familyMap.get(family);
> if (list == null) {
> list = new ArrayList<>(1);
> }
> // find where the new entry should be placed in the List
> list.add(cell);
> this.familyMap.put(family, list);
> return this;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-18554) Append#add doesn't check the row of passed cell
[
https://issues.apache.org/jira/browse/HBASE-18554?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16200666#comment-16200666
]
Andrew Purtell commented on HBASE-18554:
Fix versions have been set for various 1.x. We can't accept compat breaking
changes to this public API there so please make the branch-1 patches
accordingly. For branch-2 and master a breaking change would be ok, but not
ideal.
> Append#add doesn't check the row of passed cell
> ---
>
> Key: HBASE-18554
> URL: https://issues.apache.org/jira/browse/HBASE-18554
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Reid Chan
> Labels: beginner
> Fix For: 2.0.0, 1.4.0, 1.3.2, 1.5.0, 1.2.7
>
> Attachments: HBASE-18554.master.001.patch,
> HBASE-18554.master.002.patch
>
>
> {code}
> @SuppressWarnings("unchecked")
> public Append add(final Cell cell) {
> // Presume it is KeyValue for now.
> byte [] family = CellUtil.cloneFamily(cell);
> List list = this.familyMap.get(family);
> if (list == null) {
> list = new ArrayList<>(1);
> }
> // find where the new entry should be placed in the List
> list.add(cell);
> this.familyMap.put(family, list);
> return this;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-18554) Append#add doesn't check the row of passed cell
[
https://issues.apache.org/jira/browse/HBASE-18554?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16200601#comment-16200601
]
Chia-Ping Tsai commented on HBASE-18554:
{code}
@Test
public void testWrongRowInAppend() throws Exception {
TableName tableName = TableName.valueOf("testDiffRowAppend");
HTableDescriptor desc = new HTableDescriptor(tableName)
.addFamily(new HColumnDescriptor(FAMILY));
byte[][] splits = new byte[][]{Bytes.toBytes("3")};
TEST_UTIL.getHBaseAdmin().createTable(desc, splits);
Append append = new Append(Bytes.toBytes("1"));
append.add(CellUtil.createCell(Bytes.toBytes("1"), FAMILY, null,
System.currentTimeMillis(),
KeyValue.Type.Put.getCode(), Bytes.toBytes("value")));
append.add(CellUtil.createCell(Bytes.toBytes("5"), FAMILY, null,
System.currentTimeMillis(),
KeyValue.Type.Put.getCode(), Bytes.toBytes("value")));
try (Table t = TEST_UTIL.getConnection().getTable(tableName)) {
t.batch(Arrays.asList(append));
Get g = new Get(Bytes.toBytes("5"));
Result r = t.get(g);
assertFalse(r.isEmpty());
}
}
{code}
User can add a invalid row of cell to the region via {{Append#add}}. Not sure
how dangerous the invalid cell is, but we should protect user from the time
bomb.
> Append#add doesn't check the row of passed cell
> ---
>
> Key: HBASE-18554
> URL: https://issues.apache.org/jira/browse/HBASE-18554
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Reid Chan
> Labels: beginner
> Attachments: HBASE-18554.master.001.patch,
> HBASE-18554.master.002.patch
>
>
> {code}
> @SuppressWarnings("unchecked")
> public Append add(final Cell cell) {
> // Presume it is KeyValue for now.
> byte [] family = CellUtil.cloneFamily(cell);
> List list = this.familyMap.get(family);
> if (list == null) {
> list = new ArrayList<>(1);
> }
> // find where the new entry should be placed in the List
> list.add(cell);
> this.familyMap.put(family, list);
> return this;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-18554) Append#add doesn't check the row of passed cell
[
https://issues.apache.org/jira/browse/HBASE-18554?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16142860#comment-16142860
]
Chia-Ping Tsai commented on HBASE-18554:
HRegion lock the row according to the Mutation's row, so the cells having
different rows will be processed without any locks. The data will be corrupted,
and user may be unaware of this problem because no error occur.
We need a workaround for keeping our compatibility.
# (2.0) Deprecate Append#add; Introduce an new method Append#addCell which
throw IOException. Also, the Append#add() will throw an unchecked exception.
# (3.0) Specify the IOException thrown by Append#add; Remove the Deprecated
from Append#add; Deprecate Append#addCell
# (4.0) Remove the Append#addCell
This issue cannot be resolved totally until we release 4.0.:P
> Append#add doesn't check the row of passed cell
> ---
>
> Key: HBASE-18554
> URL: https://issues.apache.org/jira/browse/HBASE-18554
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Reid Chan
> Labels: beginner
> Attachments: HBASE-18554.master.001.patch,
> HBASE-18554.master.002.patch
>
>
> {code}
> @SuppressWarnings("unchecked")
> public Append add(final Cell cell) {
> // Presume it is KeyValue for now.
> byte [] family = CellUtil.cloneFamily(cell);
> List list = this.familyMap.get(family);
> if (list == null) {
> list = new ArrayList<>(1);
> }
> // find where the new entry should be placed in the List
> list.add(cell);
> this.familyMap.put(family, list);
> return this;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-18554) Append#add doesn't check the row of passed cell
[
https://issues.apache.org/jira/browse/HBASE-18554?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16124998#comment-16124998
]
Anoop Sam John commented on HBASE-18554:
So for backward compatibility, we dont throw checked Exception now. Ideally
when we deprecate some thing in a version, there should be alternate way in
that version itself. Then only we can make sure users use the new way and we
can remove in next version. Now when we add the checked Exception in 3.0,
there also it will be a src compatibility break. (Though it will be binary
compat)
Am not sure what we do here is correct
> Append#add doesn't check the row of passed cell
> ---
>
> Key: HBASE-18554
> URL: https://issues.apache.org/jira/browse/HBASE-18554
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Reid Chan
> Labels: beginner
> Attachments: HBASE-18554.master.001.patch,
> HBASE-18554.master.002.patch
>
>
> {code}
> @SuppressWarnings("unchecked")
> public Append add(final Cell cell) {
> // Presume it is KeyValue for now.
> byte [] family = CellUtil.cloneFamily(cell);
> List list = this.familyMap.get(family);
> if (list == null) {
> list = new ArrayList<>(1);
> }
> // find where the new entry should be placed in the List
> list.add(cell);
> this.familyMap.put(family, list);
> return this;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-18554) Append#add doesn't check the row of passed cell
[
https://issues.apache.org/jira/browse/HBASE-18554?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16124625#comment-16124625
]
Chia-Ping Tsai commented on HBASE-18554:
LGTM.
[~anoop.hbase] Any suggestions?
> Append#add doesn't check the row of passed cell
> ---
>
> Key: HBASE-18554
> URL: https://issues.apache.org/jira/browse/HBASE-18554
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Reid Chan
> Labels: beginner
> Attachments: HBASE-18554.master.001.patch,
> HBASE-18554.master.002.patch
>
>
> {code}
> @SuppressWarnings("unchecked")
> public Append add(final Cell cell) {
> // Presume it is KeyValue for now.
> byte [] family = CellUtil.cloneFamily(cell);
> List list = this.familyMap.get(family);
> if (list == null) {
> list = new ArrayList<>(1);
> }
> // find where the new entry should be placed in the List
> list.add(cell);
> this.familyMap.put(family, list);
> return this;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-18554) Append#add doesn't check the row of passed cell
[
https://issues.apache.org/jira/browse/HBASE-18554?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16124585#comment-16124585
]
Reid Chan commented on HBASE-18554:
---
Other mutations: Delete, Put and Increment all throw
IOException(WrongRowIOException).
> Append#add doesn't check the row of passed cell
> ---
>
> Key: HBASE-18554
> URL: https://issues.apache.org/jira/browse/HBASE-18554
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Reid Chan
> Labels: beginner
> Attachments: HBASE-18554.master.001.patch
>
>
> {code}
> @SuppressWarnings("unchecked")
> public Append add(final Cell cell) {
> // Presume it is KeyValue for now.
> byte [] family = CellUtil.cloneFamily(cell);
> List list = this.familyMap.get(family);
> if (list == null) {
> list = new ArrayList<>(1);
> }
> // find where the new entry should be placed in the List
> list.add(cell);
> this.familyMap.put(family, list);
> return this;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-18554) Append#add doesn't check the row of passed cell
[
https://issues.apache.org/jira/browse/HBASE-18554?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16124555#comment-16124555
]
Reid Chan commented on HBASE-18554:
---
Ya, whether to throw exception made me confused.
So, which one should be chosen, making behavioral change, or not throwing
exception until 3.0, and throwing runtime exception temporarily to keep api
compatibility?
Btw, there is a WrongRowIOException.
> Append#add doesn't check the row of passed cell
> ---
>
> Key: HBASE-18554
> URL: https://issues.apache.org/jira/browse/HBASE-18554
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Reid Chan
> Labels: beginner
> Attachments: HBASE-18554.master.001.patch
>
>
> {code}
> @SuppressWarnings("unchecked")
> public Append add(final Cell cell) {
> // Presume it is KeyValue for now.
> byte [] family = CellUtil.cloneFamily(cell);
> List list = this.familyMap.get(family);
> if (list == null) {
> list = new ArrayList<>(1);
> }
> // find where the new entry should be placed in the List
> list.add(cell);
> this.familyMap.put(family, list);
> return this;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-18554) Append#add doesn't check the row of passed cell
[
https://issues.apache.org/jira/browse/HBASE-18554?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16124549#comment-16124549
]
Chia-Ping Tsai commented on HBASE-18554:
I'm not a fan of eating exception. We can make behavioral change (throw
unchecked exception?).Also make it as Deprecated with comment saying "Will
throw IOException in 3.0"
BTW, why not throw illegalargumentexception directly? The IOE can't represent
the incorrect data.
> Append#add doesn't check the row of passed cell
> ---
>
> Key: HBASE-18554
> URL: https://issues.apache.org/jira/browse/HBASE-18554
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Reid Chan
> Labels: beginner
> Attachments: HBASE-18554.master.001.patch
>
>
> {code}
> @SuppressWarnings("unchecked")
> public Append add(final Cell cell) {
> // Presume it is KeyValue for now.
> byte [] family = CellUtil.cloneFamily(cell);
> List list = this.familyMap.get(family);
> if (list == null) {
> list = new ArrayList<>(1);
> }
> // find where the new entry should be placed in the List
> list.add(cell);
> this.familyMap.put(family, list);
> return this;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-18554) Append#add doesn't check the row of passed cell
[
https://issues.apache.org/jira/browse/HBASE-18554?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16124544#comment-16124544
]
Hadoop QA commented on HBASE-18554:
---
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
18s{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green} 0m
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 3m
23s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
17s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
24s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
9s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
50s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
18s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
24s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
9s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}
30m 13s{color} | {color:green} Patch does not cause any errors with Hadoop
2.6.1 2.6.2 2.6.3 2.6.4 2.6.5 2.7.1 2.7.2 2.7.3 or 3.0.0-alpha4. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
57s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 2m
28s{color} | {color:green} hbase-client in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
7s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 41m 14s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=1.11.2 Server=1.11.2 Image:yetus/hbase:bdc94b1 |
| JIRA Issue | HBASE-18554 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12881607/HBASE-18554.master.001.patch
|
| Optional Tests | asflicense javac javadoc unit findbugs hadoopcheck
hbaseanti checkstyle compile |
| uname | Linux cd4aee98ba64 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31
14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality |
/home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh
|
| git revision | master / 173dce7 |
| Default Java | 1.8.0_144 |
| findbugs | v3.1.0-RC3 |
| Test Results |
https://builds.apache.org/job/PreCommit-HBASE-Build/8057/testReport/ |
| modules | C: hbase-client U: hbase-client |
| Console output |
https://builds.apache.org/job/PreCommit-HBASE-Build/8057/console |
| Powered by | Apache Yetus 0.4.0 http://yetus.apache.org |
This message was automatically generated.
> Append#add doesn't check the row of passed cell
> ---
>
> Key: HBASE-18554
> URL: https://issues.apache.org/jira/browse/HBASE-18554
> Project: HBase
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Reid Ch
