[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14378681#comment-14378681 ] Sean Busbey commented on HBASE-13273: - I suggested that above, other folks liked the field better and I didn't feel strongly. (the v5 patch is in master, branch-1, and branch-1.0 right now. I'm just waiting on Andrew's decision to close.) Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14378701#comment-14378701 ] Enis Soztutar commented on HBASE-13273: --- bq. I suggested that above, other folks liked the field better and I didn't feel strongly. (the v5 patch is in master, branch-1, and branch-1.0 right now. I'm just waiting on Andrew's decision to close.) Sorry I skimmed through the comments in the jira, went directly to the patch. It seems what we want is the Result object being immutable, but also having a Result.Builder to build immutable Results. Main argument was that we do not want to pay the price (in heap) for all the Result object which are normally not read only. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14378714#comment-14378714 ] Mikhail Antonov commented on HBASE-13273: - [~enis] yeah, this object is sure on the hot code path, just looking at the Result class structure (fields) and taking into account common object overhead (header, word alignment) I kind of doubt adding one boolean field to this class provides for measurable difference is object size.. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14378833#comment-14378833 ] Mikhail Antonov commented on HBASE-13273: - Thanks for review and comments guys! Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.12 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14378779#comment-14378779 ] Hudson commented on HBASE-13273: SUCCESS: Integrated in HBase-TRUNK #6298 (See [https://builds.apache.org/job/HBase-TRUNK/6298/]) HBASE-13273 Make Result.EMPTY_RESULT read-only; currently it can be modified (busbey: rev 8cb4f89c013b254c6d3e9bee7b63137a391bcaba) * hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java * hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14378793#comment-14378793 ] Hudson commented on HBASE-13273: SUCCESS: Integrated in HBase-1.0 #824 (See [https://builds.apache.org/job/HBase-1.0/824/]) HBASE-13273 Make Result.EMPTY_RESULT read-only; currently it can be modified (busbey: rev 04f7122087d8814bcfb26098cd7e2016b8e3f0bf) * hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java * hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14378798#comment-14378798 ] Hudson commented on HBASE-13273: FAILURE: Integrated in HBase-1.1 #317 (See [https://builds.apache.org/job/HBase-1.1/317/]) HBASE-13273 Make Result.EMPTY_RESULT read-only; currently it can be modified (busbey: rev ac6fd86d266e06696e31282de0844b897ef73864) * hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java * hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14378695#comment-14378695 ] Andrew Purtell commented on HBASE-13273: +1 for 0.98, any time, I'm holding the RC for HBASE-13326 looks like anyway. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14379119#comment-14379119 ] Hudson commented on HBASE-13273: FAILURE: Integrated in HBase-0.98 #918 (See [https://builds.apache.org/job/HBase-0.98/918/]) HBASE-13273 Make Result.EMPTY_RESULT read-only; currently it can be modified (busbey: rev 1c500017123a6f74b4ae53c960f2c433ac309919) * hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java * hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.12 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14379256#comment-14379256 ] Hudson commented on HBASE-13273: FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #872 (See [https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/872/]) HBASE-13273 Make Result.EMPTY_RESULT read-only; currently it can be modified (busbey: rev 1c500017123a6f74b4ae53c960f2c433ac309919) * hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java * hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.12 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14378547#comment-14378547 ] Sean Busbey commented on HBASE-13273: - [~apurtell] am I fine to push this into 0.98 now, or would you prefer after the 0.98.12 RC2 is tagged? Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14378660#comment-14378660 ] Enis Soztutar commented on HBASE-13273: --- Should we add a sub class to Result instead of adding 1 field for every Result object? Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14376462#comment-14376462 ] Mikhail Antonov commented on HBASE-13273: - [~larsgeorge], [~busbey] what do you guys think of last version? Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14375652#comment-14375652 ] Hadoop QA commented on HBASE-13273: --- {color:green}+1 overall{color}. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12706468/HBASE-13273-v5.patch against master branch at commit 845f5de121e92a99b41b30dc86cb3f2898e0254f. ATTACHMENT ID: 12706468 {color:green}+1 @author{color}. The patch does not contain any @author tags. {color:green}+1 tests included{color}. The patch appears to include 4 new or modified tests. {color:green}+1 hadoop versions{color}. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0) {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 javadoc{color}. The javadoc tool did not generate any warning messages. {color:green}+1 checkstyle{color}. The applied patch does not increase the total number of checkstyle errors {color:green}+1 findbugs{color}. The patch does not introduce any new Findbugs (version 2.0.3) warnings. {color:green}+1 release audit{color}. The applied patch does not increase the total number of release audit warnings. {color:green}+1 lineLengths{color}. The patch does not introduce lines longer than 100 {color:green}+1 site{color}. The mvn site goal succeeds with this patch. {color:green}+1 core tests{color}. The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13365//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13365//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13365//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13365//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13365//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13365//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13365//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13365//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13365//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13365//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13365//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13365//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13365//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13365//console This message is automatically generated. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14376836#comment-14376836 ] Mikhail Antonov commented on HBASE-13273: - Thanks [~busbey] Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14376795#comment-14376795 ] Sean Busbey commented on HBASE-13273: - +1 LGTM. I think it meets all of Lars' stated preferences. I'll push it in a bit unless someone has an objection. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273-v5.patch, HBASE-13273-v5.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14371091#comment-14371091 ] Hadoop QA commented on HBASE-13273: --- {color:red}-1 overall{color}. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12705865/HBASE-13273.patch against master branch at commit 0d766544166fc9630bb00ae14a4a34a69d93f127. ATTACHMENT ID: 12705865 {color:green}+1 @author{color}. The patch does not contain any @author tags. {color:green}+1 tests included{color}. The patch appears to include 3 new or modified tests. {color:green}+1 hadoop versions{color}. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0) {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 javadoc{color}. The javadoc tool did not generate any warning messages. {color:green}+1 checkstyle{color}. The applied patch does not increase the total number of checkstyle errors {color:red}-1 findbugs{color}. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings. {color:green}+1 release audit{color}. The applied patch does not increase the total number of release audit warnings. {color:green}+1 lineLengths{color}. The patch does not introduce lines longer than 100 {color:green}+1 site{color}. The mvn site goal succeeds with this patch. {color:green}+1 core tests{color}. The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13327//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13327//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13327//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13327//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13327//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13327//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13327//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13327//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13327//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13327//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13327//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13327//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13327//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13327//console This message is automatically generated. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14370995#comment-14370995 ] Lars George commented on HBASE-13273: - I am +1 on that. :) Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14371019#comment-14371019 ] Mikhail Antonov commented on HBASE-13273: - Thanks for taking a look Lars :) Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14371686#comment-14371686 ] Sean Busbey commented on HBASE-13273: - Please also ensure we can't mutate state in EMPTY_RESULT via the {{setExists(Boolean)}} method. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14371668#comment-14371668 ] Ashish Singhi commented on HBASE-13273: --- Please format the code. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14371689#comment-14371689 ] Sean Busbey commented on HBASE-13273: - Thanks for the heads up! Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14371605#comment-14371605 ] Sean Busbey commented on HBASE-13273: - Can someone tell me why TestResult is in hbase-server? {quote} -1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings. {quote} I'm not seeing what in hbase-client generated a new warning. I'll chase it down in a bit. {code} + private void checkReadonly() { +if (this.readonly == true) { + throw new UnsupportedOperationException(Attempting to modify readonly EMPTY_RESULT!); +} + } {code} Please add a javadoc about how all methods that mutate state in a result *must* call this method. {code} + private void checkReadonly() { +if (this.readonly == true) { + throw new UnsupportedOperationException(Attempting to modify readonly EMPTY_RESULT!); +} + } {code} just use {{if(readonly)}} directly. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14371644#comment-14371644 ] Sean Busbey commented on HBASE-13273: - As a nit, please follow the guidelines for [submitting patches|http://hbase.apache.org/book.html#submitting.patches]. Specifically, please use {{git format-patch}} with a good commit message and version your patches. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14371623#comment-14371623 ] Ashish Singhi commented on HBASE-13273: --- {quote} bq. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings. I'm not seeing what in hbase-client generated a new warning. I'll chase it down in a bit. {quote} This is not related to this patch. It is from HBASE-13199 Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14368583#comment-14368583 ] stack commented on HBASE-13273: --- +1 from me. I like the @sean suggestion above about inner subclass that throws on copyFrom but this will do. Thanks Mikhail. Will wait on Sean's ok before commit. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14368565#comment-14368565 ] Hadoop QA commented on HBASE-13273: --- {color:red}-1 overall{color}. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12705505/HBASE-13273.patch against master branch at commit 014b8121031d6268f73f901d54c32a9d880bb12f. ATTACHMENT ID: 12705505 {color:green}+1 @author{color}. The patch does not contain any @author tags. {color:green}+1 tests included{color}. The patch appears to include 3 new or modified tests. {color:green}+1 hadoop versions{color}. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0) {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 javadoc{color}. The javadoc tool did not generate any warning messages. {color:green}+1 checkstyle{color}. The applied patch does not increase the total number of checkstyle errors {color:red}-1 findbugs{color}. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings. {color:green}+1 release audit{color}. The applied patch does not increase the total number of release audit warnings. {color:green}+1 lineLengths{color}. The patch does not introduce lines longer than 100 {color:green}+1 site{color}. The mvn site goal succeeds with this patch. {color:green}+1 core tests{color}. The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13308//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13308//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13308//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13308//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13308//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13308//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13308//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13308//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13308//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13308//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13308//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13308//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13308//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13308//console This message is automatically generated. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14368579#comment-14368579 ] Mikhail Antonov commented on HBASE-13273: - As i see in warnings to client module, additional warning isn't related. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14368881#comment-14368881 ] Lars George commented on HBASE-13273: - I do not think this is enough. You can still call {{addResults()}} (or {{addStats()}} soon), and {{setExists()}}, modifying the EMPTY_RESULT. What we really need is either the interface idea suggested by [~ndimiduk] or some more coding to make a Result read-only. I mean, we could have an package local constructor that sets a {{boolean readonly}} to {{true}} and then all setters respecting that. This will not break the signature and is better than doing the {{copyfrom()}} kludge. So I am -1 until convinced otherwise. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14370118#comment-14370118 ] Mikhail Antonov commented on HBASE-13273: - [~larsgeorge] oops, you're right. I'm thinking 'readonly' field and package local (or in fact, private would do) constructor could be a bit less, kind of, intrusive change? I'll fix the patch. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14367841#comment-14367841 ] Mikhail Antonov commented on HBASE-13273: - 2 options I can see is to make EMPTY_RESULT object private and provide method getEmptyResult(), returning new cloned empty Result, or make Result object immutable (which means copyFrom() method should be removed)? Doesn't look like backward-compatible either? Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.12 Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14368178#comment-14368178 ] Andrew Purtell commented on HBASE-13273: I'm rolling the 0.98.12 RC tonight. Did you want to get this in ahead of that? Or I can move it out. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.12 Attachments: HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14368396#comment-14368396 ] Mikhail Antonov commented on HBASE-13273: - bq. 0 failures (±0) , 24 skipped (+17) I guess not related. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14368346#comment-14368346 ] Hadoop QA commented on HBASE-13273: --- {color:red}-1 overall{color}. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12705471/HBASE-13273.patch against master branch at commit f9a17edc252a88c5a1a2c7764e3f9f65623e0ced. ATTACHMENT ID: 12705471 {color:green}+1 @author{color}. The patch does not contain any @author tags. {color:green}+1 tests included{color}. The patch appears to include 3 new or modified tests. {color:green}+1 hadoop versions{color}. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0) {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 javadoc{color}. The javadoc tool did not generate any warning messages. {color:green}+1 checkstyle{color}. The applied patch does not increase the total number of checkstyle errors {color:green}+1 findbugs{color}. The patch does not introduce any new Findbugs (version 2.0.3) warnings. {color:green}+1 release audit{color}. The applied patch does not increase the total number of release audit warnings. {color:green}+1 lineLengths{color}. The patch does not introduce lines longer than 100 {color:green}+1 site{color}. The mvn site goal succeeds with this patch. {color:red}-1 core tests{color}. The patch failed these unit tests: Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13303//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13303//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13303//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13303//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13303//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13303//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13303//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13303//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13303//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13303//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13303//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13303//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13303//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13303//console This message is automatically generated. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14368063#comment-14368063 ] Nick Dimiduk commented on HBASE-13273: -- Or Result should have been made an interface in our 1.0 API revamp, which EMPTY_RESULT being an implementation with mutation methods that throw. Comment on copyFrom says bq. Copy another Result into this one. Needed for the old Mapred framework Indeed, the use case is instance-resuse, Writable-style. Copy constructor doesn't help this scenario. Probably best to do the copyFrom instance check [~stack] mentioned. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.12 Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14368094#comment-14368094 ] Mikhail Antonov commented on HBASE-13273: - Yeah, will do that. Also as I understand, this is an issue for branch 1.0, so removing this method (public?) or adding another constructor doesn't look like a solution (can do that for 2.0 later in separate jira). Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.12 Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14368098#comment-14368098 ] Mikhail Antonov commented on HBASE-13273: - This method is only used from TableRecordReaderImpl and TableSnapshotInputFormat, yeah, part of old mapred -related API. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.12 Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14368424#comment-14368424 ] Sean Busbey commented on HBASE-13273: - {code} + throw new IllegalStateException(Invoking #copyFrom() on EMPTY_RESULT isn't allowed!); {code} use UnsupportedOperationException please. {code} + /** + * Verifies that one can't call copyFrom on instance of EMPTY_RESULT. + */ + public void testCopyFromOnEmptyResult() { +try { + Result emptyResult = Result.EMPTY_RESULT; + Result otherResult = new Result(); + emptyResult.copyFrom(otherResult); + fail(IllegalStateException should have been thrown when trying to + +call copyFrom on EMPTY_RESULT!); +} catch (RuntimeException ex) { + assertTrue(ex instanceof IllegalStateException); +} + } {code} we can't use the {{@Test(expected=}} notation because this is still using the old TestCase form, right? I'd just like to confirm. {code} +} catch (RuntimeException ex) { + assertTrue(ex instanceof IllegalStateException); +} {code} catch the desired exception directly and just log a debug statement about how everything is fine. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14368423#comment-14368423 ] Sean Busbey commented on HBASE-13273: - Why not make the instance of EMPTY_RESULT an anonymous subclass that always throws when copyFrom is called? Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14368445#comment-14368445 ] Mikhail Antonov commented on HBASE-13273: - I actually noted above - bq. patch (testing on object identity in fact, as Result doesn't implement equals(), which is what we need in this case) So I explicitly wanted to use reference (identity) comparison, isnce equals/hashCode isn't implemented. So yeah, let me modify it to use == :) much cleaner. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14368432#comment-14368432 ] Sean Busbey commented on HBASE-13273: - turns out it doesn't prevent it, because Result doesn't implement equals/hashCode. If we do an equality test in copyFrom, we should still use reference equality so it's clear that what we mean is _exactly_ the EMPTY_RESULT instance. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14368429#comment-14368429 ] Sean Busbey commented on HBASE-13273: - Specifically, we should not do a {{.equals}} check because it keeps us from doing object reuse on an Result that starts empty (from the no-args constructor) that isn't the singleton EMPTY_RESULT. We suggest exactly such reuse in the javadocs. I think the anonymous subclass will be cleaner. If we go with doing a check in the copyFrom method, then reference equality should be used. Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Assignee: Mikhail Antonov Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.13 Attachments: HBASE-13273.patch, HBASE-13273.patch Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-13273) Make Result.EMPTY_RESULT read-only; currently it can be modified
[ https://issues.apache.org/jira/browse/HBASE-13273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14367887#comment-14367887 ] stack commented on HBASE-13273: --- Can we test copyFrom is the EMPTY_RESULT instance -- object equality -- and throw if we are trying to copy into it? If copyFrom used anywhere? Should do a constructor that takes another result in favor of this copyFrom if that is all that is preventing us be immutable. Thanks [~mantonov] Make Result.EMPTY_RESULT read-only; currently it can be modified Key: HBASE-13273 URL: https://issues.apache.org/jira/browse/HBASE-13273 Project: HBase Issue Type: Bug Affects Versions: 0.98.0, 1.0.0 Reporter: stack Labels: beginner Fix For: 2.0.0, 1.0.1, 1.1.0, 0.98.12 Again from [~larsgeorge] Result result2 = Result.EMPTY_RESULT; System.out.println(result2); result2.copyFrom(result1); System.out.println(result2); What do you think happens when result1 has cells? Yep, you just modified the shared public EMPTY_RESULT to be not empty anymore. Fix. Result should be non-modifiable post-construction. -- This message was sent by Atlassian JIRA (v6.3.4#6332)