Review Request 15517: ACCUMULO-1892 examples.simple.RandomBatchWriter might not write the specified number of rowids
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15517/ --- Review request for accumulo. Bugs: ACCUMULO-1892 https://issues.apache.org/jira/browse/ACCUMULO-1892 Repository: accumulo Description --- ACCUMULO-1892 changes RandomBatchWriter to ensure it writes the specified number of rowids. Diffs - src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java 3206fa6bbb5e560175e9aec6a1e612185e4ca945 Diff: https://reviews.apache.org/r/15517/diff/ Testing --- unit tests pass. simple.examples.Examples functional tests pass (given fixes in ACCUMULO-1878 to actually run them). Thanks, Sean Busbey
Review Request 15518: ACCUMULO-1878 handle error conditions in example tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15518/ --- Review request for accumulo. Bugs: ACCUMULO-1878 https://issues.apache.org/jira/browse/ACCUMULO-1878 Repository: accumulo Description --- ACCUMULO-1878 handle error conditions in example tests. * ensure simple examples used in integration tests properly set non-zero exit on errors * check return code of executed commands. * fix typo in bloom filter speed comparison * fix error in arg order on helloworld examples * Makes sure paired examples for RandomBatch use the same seed. Diffs - src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchScanner.java 7801104bf5b8095066630512a0b44bc8af2a85bf src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java 3206fa6bbb5e560175e9aec6a1e612185e4ca945 test/system/auto/simple/examples.py 5fc9b4bdee7f24beba0d85f414600afffa8d0b08 Diff: https://reviews.apache.org/r/15518/diff/ Testing --- unit tests pass. functional test simple.examples.Examples passes, presuming the fixes for ACCUMULO-1891 and ACCUMULO-1892 are in place. I was get rare on-VM flakiness on the bloom filter /no bloom speed comparison. I was getting loud failure on the non-isolated scanner interference example, but it mysteriously stopped happening. I presume it was VM related. I have stack traces still, so I could file a follow on ticket if there's interest. Thanks, Sean Busbey
Re: Review Request 15482: ACCUMULO-1889 ZooKeeperInstance close method should mark instance closed.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15482/#review28871 --- Ship it! Ship It! - Bill Slacum On Nov. 13, 2013, 9:35 p.m., Sean Busbey wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15482/ --- (Updated Nov. 13, 2013, 9:35 p.m.) Review request for accumulo. Bugs: ACCUMULO-1889 https://issues.apache.org/jira/browse/ACCUMULO-1889 Repository: accumulo Description --- ACCUMULO-1889 mark ZKI as closed once close() is called. Once a given ZKI has .close() called, mark that instance closed regardless of outstanding client count. Diffs - src/core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java f657c07a28c3bf330556f0d4e02b9adcb0ea755e Diff: https://reviews.apache.org/r/15482/diff/ Testing --- unit tests pass, functional tests in progress. Thanks, Sean Busbey
Re: Review Request 15518: ACCUMULO-1878 handle error conditions in example tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15518/ --- (Updated Nov. 14, 2013, 4:52 p.m.) Review request for accumulo. Bugs: ACCUMULO-1878 https://issues.apache.org/jira/browse/ACCUMULO-1878 Repository: accumulo Description --- ACCUMULO-1878 handle error conditions in example tests. * ensure simple examples used in integration tests properly set non-zero exit on errors * check return code of executed commands. * fix typo in bloom filter speed comparison * fix error in arg order on helloworld examples * Makes sure paired examples for RandomBatch use the same seed. Diffs - src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchScanner.java 7801104bf5b8095066630512a0b44bc8af2a85bf src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java 3206fa6bbb5e560175e9aec6a1e612185e4ca945 test/system/auto/simple/examples.py 5fc9b4bdee7f24beba0d85f414600afffa8d0b08 Diff: https://reviews.apache.org/r/15518/diff/ Testing (updated) --- unit tests pass. functional test simple.examples.Examples passes, presuming the fixes for ACCUMULO-1891 and ACCUMULO-1892 are in place. I get rare on-VM flakiness on the bloom filter /no bloom speed comparison. I was getting loud failure on the non-isolated scanner interference example, but it mysteriously stopped happening. I presume it was VM related. I have stack traces still, so I could file a follow on ticket if there's interest. Thanks, Sean Busbey
Re: Review Request 15482: ACCUMULO-1889 ZooKeeperInstance close method should mark instance closed.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15482/#review28874 --- Ship it! Ship It! - Ryan Fishel On Nov. 13, 2013, 9:35 p.m., Sean Busbey wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15482/ --- (Updated Nov. 13, 2013, 9:35 p.m.) Review request for accumulo. Bugs: ACCUMULO-1889 https://issues.apache.org/jira/browse/ACCUMULO-1889 Repository: accumulo Description --- ACCUMULO-1889 mark ZKI as closed once close() is called. Once a given ZKI has .close() called, mark that instance closed regardless of outstanding client count. Diffs - src/core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java f657c07a28c3bf330556f0d4e02b9adcb0ea755e Diff: https://reviews.apache.org/r/15482/diff/ Testing --- unit tests pass, functional tests in progress. Thanks, Sean Busbey
[VOTE] Deprecate mock in 1.6.0
Should we deprecate mock accumulo for 1.6.0? This was considered [1] for 1.5.0. I started thinking about this because I never added conditional writer to mock. [1] : https://issues.apache.org/jira/browse/ACCUMULO-878
Re: [VOTE] Deprecate mock in 1.6.0
On Thu, Nov 14, 2013 at 2:41 PM, Keith Turner ke...@deenlo.com wrote: Should we deprecate mock accumulo for 1.6.0? This was considered [1] for 1.5.0. I started thinking about this because I never added conditional writer to mock. [1] : https://issues.apache.org/jira/browse/ACCUMULO-878 Would deprecating it in 1.6.0 mean it is removed in 1.7.0 or in 1.8.0? Does it impact anything else? Would we not attempt feature parity for 1.6.0 (e.g. conditional writer)? -- Sean
Re: [VOTE] Deprecate mock in 1.6.0
+1 Been bitten by goofy/half-implemented stuff in Mock too many times. I'd rather see effort placed into making MAC a reliable and fast testing mechanism than helping Mock limp along On 11/14/13, 12:41 PM, Keith Turner wrote: Should we deprecate mock accumulo for 1.6.0? This was considered [1] for 1.5.0. I started thinking about this because I never added conditional writer to mock. [1] : https://issues.apache.org/jira/browse/ACCUMULO-878
Review Request 15538: ACCUMULO-1893 keep tests from attempting to write to the root of HDFS.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15538/ --- Review request for accumulo and Eric Newton. Bugs: ACCUMULO-1893 https://issues.apache.org/jira/browse/ACCUMULO-1893 Repository: accumulo Description --- ACCUMULO-1893 keep tests from attempting to write to the root of HDFS. Reapplication of part of ACCUMULO-1563 to current branch. Author: Jon Hsieh jmhs...@apache.org Diffs - src/server/src/main/java/org/apache/accumulo/server/test/CreateMapFiles.java a57189437c6aee88af74e42254c8c6ffdec9b054 src/server/src/main/java/org/apache/accumulo/server/test/functional/BulkSplitOptimizationTest.java 3af414442e187de39a2f95895dad405c1616e1f7 Diff: https://reviews.apache.org/r/15538/diff/ Testing --- unit tests pass, function test simple.compaction.CompactionTest fails without, passes with. Thanks, Sean Busbey
Re: [VOTE] Deprecate mock in 1.6.0
+1 On Nov 14, 2013 3:42 PM, Keith Turner ke...@deenlo.com wrote: Should we deprecate mock accumulo for 1.6.0? This was considered [1] for 1.5.0. I started thinking about this because I never added conditional writer to mock. [1] : https://issues.apache.org/jira/browse/ACCUMULO-878
Re: [VOTE] Deprecate mock in 1.6.0
+1 (non-binding) Christopher's comment on -878 [1] echoes my thinking. For me, mock means unit tests, and I'd rather use a mock framework (Easymock, Mockito, whatever) than a running cluster (however lightweight) in unit tests. [1]: https://issues.apache.org/jira/browse/ACCUMULO-878?focusedCommentId=13504949page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13504949 On Thu, Nov 14, 2013 at 3:41 PM, Keith Turner ke...@deenlo.com wrote: Should we deprecate mock accumulo for 1.6.0? This was considered [1] for 1.5.0. I started thinking about this because I never added conditional writer to mock. [1] : https://issues.apache.org/jira/browse/ACCUMULO-878 -- | - - - | Bill Havanki | Solutions Architect, Cloudera Government Solutions | - - -
Re: Review Request 15538: ACCUMULO-1893 keep tests from attempting to write to the root of HDFS.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15538/#review28898 --- Ship it! Ship It! - Ryan Fishel On Nov. 14, 2013, 8:53 p.m., Sean Busbey wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15538/ --- (Updated Nov. 14, 2013, 8:53 p.m.) Review request for accumulo and Eric Newton. Bugs: ACCUMULO-1893 https://issues.apache.org/jira/browse/ACCUMULO-1893 Repository: accumulo Description --- ACCUMULO-1893 keep tests from attempting to write to the root of HDFS. Reapplication of part of ACCUMULO-1563 to current branch. Author: Jon Hsieh jmhs...@apache.org Diffs - src/server/src/main/java/org/apache/accumulo/server/test/CreateMapFiles.java a57189437c6aee88af74e42254c8c6ffdec9b054 src/server/src/main/java/org/apache/accumulo/server/test/functional/BulkSplitOptimizationTest.java 3af414442e187de39a2f95895dad405c1616e1f7 Diff: https://reviews.apache.org/r/15538/diff/ Testing --- unit tests pass, function test simple.compaction.CompactionTest fails without, passes with. Thanks, Sean Busbey
Re: Review Request 15166: ACCUMULO-802 Tablespaces
On Nov. 1, 2013, 6:19 p.m., John Vines wrote: core/src/main/java/org/apache/accumulo/core/client/TableNamespaceExistsException.java, line 24 https://reviews.apache.org/r/15166/diff/1/?file=376077#file376077line24 If we make TableNamespaceExistException extend AccumuloExtension we can get around a lot of api compatibility issues Christopher Tubbs wrote: That doesn't follow the existing pattern with TableExistsException, and the Exception does not appear to be thrown in any existing API. To what API-compatibility issues are you referring? John Vines wrote: There is an issue with TableNamespaceNotFoundException and I think all 3 of these exceptions should be in the same family, i.e. implemented by similar means. Sean Hickey wrote: Like Christopher pointed out, I was just following what Tables did with their exceptions. If you think that should be different, then we might want to change the table exceptions too. I think this can be addressed later. I think that where the rare places where a TableNamespaceNotFoundException is wrapped with a RuntimeException, it is sufficient, because namespace operations should be rare, and namespaces can generally be stable. It's not that I think it shouldn't be addressed... I just think it's not a priority. - Christopher --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15166/#review28029 --- On Oct. 31, 2013, 10:01 p.m., Christopher Tubbs wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15166/ --- (Updated Oct. 31, 2013, 10:01 p.m.) Review request for accumulo. Bugs: ACCUMULO-802 https://issues.apache.org/jira/browse/ACCUMULO-802 Repository: accumulo Description --- ACCUMULO-802 Tablespaces (Table Namespaces), work done by Sean Hickey, https://github.com/Wisellama/accumulo/tree/ACCUMULO-802, rebase'd onto latest master (including 210 commits) Diffs - core/src/main/java/org/apache/accumulo/core/Constants.java 9db0c405c5b9fbac13fa735c3ffd6433e9831051 core/src/main/java/org/apache/accumulo/core/client/Connector.java bbfa55f4b9ad8fc0e5f0c0058e2e0564685d7c85 core/src/main/java/org/apache/accumulo/core/client/TableNamespaceExistsException.java PRE-CREATION core/src/main/java/org/apache/accumulo/core/client/TableNamespaceNotEmptyException.java PRE-CREATION core/src/main/java/org/apache/accumulo/core/client/TableNamespaceNotFoundException.java PRE-CREATION core/src/main/java/org/apache/accumulo/core/client/admin/SecurityOperations.java 86a3ff271fc2e085c426da3156cfab9cdbb5c36b core/src/main/java/org/apache/accumulo/core/client/admin/SecurityOperationsImpl.java 0f0e998f334631cfb342f9a39fdd12e62aa98f13 core/src/main/java/org/apache/accumulo/core/client/admin/TableNamespaceOperations.java PRE-CREATION core/src/main/java/org/apache/accumulo/core/client/admin/TableNamespaceOperationsHelper.java PRE-CREATION core/src/main/java/org/apache/accumulo/core/client/admin/TableNamespaceOperationsImpl.java PRE-CREATION core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsImpl.java 1b652f99e23e2dfa06c7373a6f4c3c044f5cd1a3 core/src/main/java/org/apache/accumulo/core/client/impl/ConnectorImpl.java bd1156912849b13ebad8f6b974fd35d8adf18f1d core/src/main/java/org/apache/accumulo/core/client/impl/TableNamespaces.java PRE-CREATION core/src/main/java/org/apache/accumulo/core/client/impl/Tables.java 8bc725a3405a79c20c690bff3f6fdb6f713be523 core/src/main/java/org/apache/accumulo/core/client/impl/thrift/ClientService.java 488e0654720250542145e0d543ad16813f8d8b6d core/src/main/java/org/apache/accumulo/core/client/impl/thrift/SecurityErrorCode.java b706ce87bb1e8a525e585b14b7fb8563c3493c2c core/src/main/java/org/apache/accumulo/core/client/mock/MockAccumulo.java 5ee144d9eb67c8e79dad870abbd823cac64e111e core/src/main/java/org/apache/accumulo/core/client/mock/MockConnector.java 80ec514f9ef466dd7e4077230acfd2c77fb5 core/src/main/java/org/apache/accumulo/core/client/mock/MockSecurityOperations.java 765cda9cac6fb3bc7df9eb8db1c83846960e85c3 core/src/main/java/org/apache/accumulo/core/client/mock/MockTable.java 3dcab11bd50bf189ccc4cbaf97d3d4ecb9133047 core/src/main/java/org/apache/accumulo/core/client/mock/MockTableNamespace.java PRE-CREATION core/src/main/java/org/apache/accumulo/core/client/mock/MockTableNamespaceOperations.java PRE-CREATION core/src/main/java/org/apache/accumulo/core/client/mock/MockTableOperations.java a3c2043b8120e13a190eb173d8342c205700382f
Re: [VOTE] Deprecate mock in 1.6.0
+1 On Thu, Nov 14, 2013 at 3:41 PM, Keith Turner ke...@deenlo.com wrote: Should we deprecate mock accumulo for 1.6.0? This was considered [1] for 1.5.0. I started thinking about this because I never added conditional writer to mock. [1] : https://issues.apache.org/jira/browse/ACCUMULO-878
Re: Review Request 15538: ACCUMULO-1893 keep tests from attempting to write to the root of HDFS.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15538/#review28905 --- Ship it! (non-binding) The diff encompasses the changes that were missed in ACCUMULO-1563 that should have been applied to the Java files in 1.4.x. See accumulo-1536-1.4.patch under that ticket. - Bill Havanki On Nov. 14, 2013, 3:53 p.m., Sean Busbey wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15538/ --- (Updated Nov. 14, 2013, 3:53 p.m.) Review request for accumulo and Eric Newton. Bugs: ACCUMULO-1893 https://issues.apache.org/jira/browse/ACCUMULO-1893 Repository: accumulo Description --- ACCUMULO-1893 keep tests from attempting to write to the root of HDFS. Reapplication of part of ACCUMULO-1563 to current branch. Author: Jon Hsieh jmhs...@apache.org Diffs - src/server/src/main/java/org/apache/accumulo/server/test/CreateMapFiles.java a57189437c6aee88af74e42254c8c6ffdec9b054 src/server/src/main/java/org/apache/accumulo/server/test/functional/BulkSplitOptimizationTest.java 3af414442e187de39a2f95895dad405c1616e1f7 Diff: https://reviews.apache.org/r/15538/diff/ Testing --- unit tests pass, function test simple.compaction.CompactionTest fails without, passes with. Thanks, Sean Busbey
Re: Review Request 15166: ACCUMULO-802 Tablespaces
On Nov. 1, 2013, 6:19 p.m., John Vines wrote: core/src/main/java/org/apache/accumulo/core/security/TableNamespacePermission.java, line 26 https://reviews.apache.org/r/15166/diff/1/?file=376101#file376101line26 I think we also need BULK_IMPORT for bulk importing to namespace tables, DROP for dropping the entire namespace, and possibly GRANT_TABLE (though I can see that being iffy) Sean Hickey wrote: I thought about adding BULK_IMPORT, but I didn't add it because I don't think I added bulk importing for namespaces. I made DROP_NAMESPACE in SystemPermission, but I agree that it should also be in TableNamespacePermission (because TablePermission has DROP_TABLE). I was a little confused how permissions were supposed to work in general, so I don't know how well GRANT_TABLE would work as a namespace permission. There's some confusion here, because it's not clear how these permissions are supposed to be used in the policy for performing operations (specifically, what they mean, and how they are inherited, and whether inheritance changes their meaning). I think we should keep them minimal for now, pending a comprehensive policy-based model in the next release that allow users to manage permissions in a more robust way. (ACCUMULO-1632). On Nov. 1, 2013, 6:19 p.m., John Vines wrote: server/base/src/main/java/org/apache/accumulo/server/conf/TableNamespaceConfWatcher.java, line 29 https://reviews.apache.org/r/15166/diff/1/?file=376133#file376133line29 Why is this overriding the logging? Christopher Tubbs wrote: Because TableConfWatcher does. If we don't need to do that, we can create a separate ticket. John Vines wrote: Ticket opened Ticket number? On Nov. 1, 2013, 6:19 p.m., John Vines wrote: server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java, line 431 https://reviews.apache.org/r/15166/diff/1/?file=376130#file376130line431 This error could cause confusion when you have a table in the default namespace with the same name as a namespace (if that's possible?) Christopher Tubbs wrote: I don't think so, not since the why string explicitly says Could not find table namespace while getting configuration. John Vines wrote: Resorting to parsing strings to determine errors programmatically is pretty crummy for end users. Also, looking through this code path shows that TableNamespaceOperationsImpl#getProperties doesn't work right since it looks for the type NOTFOUND, which this error isn't setting. Perhaps we should make TableOperation a combination of Table and TableNamespace options in order to support backwards compatibility while maintaining a solid set of easily decipherable errors. Sean Hickey wrote: Well, if you really want to be able to easily find it programmatically, it might be best to just make a new type of exception (like ThriftNamespaceOperationException or something) because all the namespace stuff is new and won't break any backwards compatibility. I was just trying to reuse some existing stuff, especially with the thrift calls because I was brand new to thrift. I don't think users should parse error messages. But, your initial comment wasn't about being able to programmatically determine the error... it was about potential for confusion. My comment was in that regard. I don't want to create a bunch of new thrift exceptions at this point... but it's an option. For now, I can fix the NOTFOUND oversight, so a better exception is thrown in the client API. - Christopher --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15166/#review28029 --- On Oct. 31, 2013, 10:01 p.m., Christopher Tubbs wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15166/ --- (Updated Oct. 31, 2013, 10:01 p.m.) Review request for accumulo. Bugs: ACCUMULO-802 https://issues.apache.org/jira/browse/ACCUMULO-802 Repository: accumulo Description --- ACCUMULO-802 Tablespaces (Table Namespaces), work done by Sean Hickey, https://github.com/Wisellama/accumulo/tree/ACCUMULO-802, rebase'd onto latest master (including 210 commits) Diffs - core/src/main/java/org/apache/accumulo/core/Constants.java 9db0c405c5b9fbac13fa735c3ffd6433e9831051 core/src/main/java/org/apache/accumulo/core/client/Connector.java bbfa55f4b9ad8fc0e5f0c0058e2e0564685d7c85 core/src/main/java/org/apache/accumulo/core/client/TableNamespaceExistsException.java PRE-CREATION core/src/main/java/org/apache/accumulo/core/client/TableNamespaceNotEmptyException.java PRE-CREATION
Re: Review Request 15518: ACCUMULO-1878 handle error conditions in example tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15518/#review28907 --- src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchScanner.java https://reviews.apache.org/r/15518/#comment55973 Suggest renaming this method to something like `allRowsFound`, since it has a purpose now beyond merely printing stuff. src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchScanner.java https://reviews.apache.org/r/15518/#comment55975 If the cold run above fails, then this hot run won't be attempted (Boolean short-circuit). This is a difference from the old behavior, but as far as I can tell it's a beneficial one. If that's right, no issue here. Just want to bring it up. - Bill Havanki On Nov. 14, 2013, 11:52 a.m., Sean Busbey wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15518/ --- (Updated Nov. 14, 2013, 11:52 a.m.) Review request for accumulo. Bugs: ACCUMULO-1878 https://issues.apache.org/jira/browse/ACCUMULO-1878 Repository: accumulo Description --- ACCUMULO-1878 handle error conditions in example tests. * ensure simple examples used in integration tests properly set non-zero exit on errors * check return code of executed commands. * fix typo in bloom filter speed comparison * fix error in arg order on helloworld examples * Makes sure paired examples for RandomBatch use the same seed. Diffs - src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchScanner.java 7801104bf5b8095066630512a0b44bc8af2a85bf src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java 3206fa6bbb5e560175e9aec6a1e612185e4ca945 test/system/auto/simple/examples.py 5fc9b4bdee7f24beba0d85f414600afffa8d0b08 Diff: https://reviews.apache.org/r/15518/diff/ Testing --- unit tests pass. functional test simple.examples.Examples passes, presuming the fixes for ACCUMULO-1891 and ACCUMULO-1892 are in place. I get rare on-VM flakiness on the bloom filter /no bloom speed comparison. I was getting loud failure on the non-isolated scanner interference example, but it mysteriously stopped happening. I presume it was VM related. I have stack traces still, so I could file a follow on ticket if there's interest. Thanks, Sean Busbey
Re: Review Request 15518: ACCUMULO-1878 handle error conditions in example tests.
On Nov. 14, 2013, 9:55 p.m., Bill Havanki wrote: src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchScanner.java, line 242 https://reviews.apache.org/r/15518/diff/1/?file=384505#file384505line242 If the cold run above fails, then this hot run won't be attempted (Boolean short-circuit). This is a difference from the old behavior, but as far as I can tell it's a beneficial one. If that's right, no issue here. Just want to bring it up. yeah, I figured not worth it. - Sean --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15518/#review28907 --- On Nov. 14, 2013, 4:52 p.m., Sean Busbey wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15518/ --- (Updated Nov. 14, 2013, 4:52 p.m.) Review request for accumulo. Bugs: ACCUMULO-1878 https://issues.apache.org/jira/browse/ACCUMULO-1878 Repository: accumulo Description --- ACCUMULO-1878 handle error conditions in example tests. * ensure simple examples used in integration tests properly set non-zero exit on errors * check return code of executed commands. * fix typo in bloom filter speed comparison * fix error in arg order on helloworld examples * Makes sure paired examples for RandomBatch use the same seed. Diffs - src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchScanner.java 7801104bf5b8095066630512a0b44bc8af2a85bf src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java 3206fa6bbb5e560175e9aec6a1e612185e4ca945 test/system/auto/simple/examples.py 5fc9b4bdee7f24beba0d85f414600afffa8d0b08 Diff: https://reviews.apache.org/r/15518/diff/ Testing --- unit tests pass. functional test simple.examples.Examples passes, presuming the fixes for ACCUMULO-1891 and ACCUMULO-1892 are in place. I get rare on-VM flakiness on the bloom filter /no bloom speed comparison. I was getting loud failure on the non-isolated scanner interference example, but it mysteriously stopped happening. I presume it was VM related. I have stack traces still, so I could file a follow on ticket if there's interest. Thanks, Sean Busbey
Re: Review Request 15517: ACCUMULO-1892 examples.simple.RandomBatchWriter might not write the specified number of rowids
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15517/#review28912 --- src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java https://reviews.apache.org/r/15517/#comment55978 I believe this will lead to an infinite loop if max - min num. The loop will keep adding numbers in the range [min, max) but cannot terminate because the set cannot grow beyond size max - min. I think the check for this condition above should do System.exit(1). - Bill Havanki On Nov. 14, 2013, 11:29 a.m., Sean Busbey wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15517/ --- (Updated Nov. 14, 2013, 11:29 a.m.) Review request for accumulo. Bugs: ACCUMULO-1892 https://issues.apache.org/jira/browse/ACCUMULO-1892 Repository: accumulo Description --- ACCUMULO-1892 changes RandomBatchWriter to ensure it writes the specified number of rowids. Diffs - src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java 3206fa6bbb5e560175e9aec6a1e612185e4ca945 Diff: https://reviews.apache.org/r/15517/diff/ Testing --- unit tests pass. simple.examples.Examples functional tests pass (given fixes in ACCUMULO-1878 to actually run them). Thanks, Sean Busbey
Re: Review Request 15517: ACCUMULO-1892 examples.simple.RandomBatchWriter might not write the specified number of rowids
On Nov. 14, 2013, 10:03 p.m., Bill Havanki wrote: src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java, line 156 https://reviews.apache.org/r/15517/diff/1/?file=384504#file384504line156 I believe this will lead to an infinite loop if max - min num. The loop will keep adding numbers in the range [min, max) but cannot terminate because the set cannot grow beyond size max - min. I think the check for this condition above should do System.exit(1). yep! exactly why the above check was added. :/ - Sean --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15517/#review28912 --- On Nov. 14, 2013, 4:29 p.m., Sean Busbey wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15517/ --- (Updated Nov. 14, 2013, 4:29 p.m.) Review request for accumulo. Bugs: ACCUMULO-1892 https://issues.apache.org/jira/browse/ACCUMULO-1892 Repository: accumulo Description --- ACCUMULO-1892 changes RandomBatchWriter to ensure it writes the specified number of rowids. Diffs - src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java 3206fa6bbb5e560175e9aec6a1e612185e4ca945 Diff: https://reviews.apache.org/r/15517/diff/ Testing --- unit tests pass. simple.examples.Examples functional tests pass (given fixes in ACCUMULO-1878 to actually run them). Thanks, Sean Busbey
Re: Review Request 15499: ACCUMULO-1852 - NativeMapIT on OS X
On Nov. 14, 2013, 2:24 p.m., Michael Berman wrote: This does fix NativeMapIT for me on Mavericks+Java7, although it doesn't address the `make test` issue in nativeMap itself. Still, seems like an improvement, so, ship it. Bill Havanki wrote: Thanks for checking on me. You'll have to explain to me the make test issue. Again, I'm happy to help work on it. Run `mvn verify` and observe failure in the server/native module. This is from Maven invoking `make test` on server/native/src/main/resources/Makefile - Josh --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15499/#review28867 --- On Nov. 13, 2013, 10:56 p.m., Bill Havanki wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15499/ --- (Updated Nov. 13, 2013, 10:56 p.m.) Review request for accumulo. Bugs: ACCUMULO-1852 https://issues.apache.org/jira/browse/ACCUMULO-1852 Repository: accumulo Description --- Changes (possibly only initial) made to get NativeMapIT running on Mac OS X. Diffs - assemble/src/main/assemblies/component.xml 3655a22b6271e1f08c479124605df7088efb2ff6 minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb82c46a1a4fd407e36a3ee13e954aa2b40 server/native/src/main/resources/Makefile 6309e6d44514432da491fd8cf8981524e5014abe Diff: https://reviews.apache.org/r/15499/diff/ Testing --- Ran `mvn verify` to completion, both specifically targeting NativeMapIT and running all tests. Platform is OS X 10.8.5, Java 1.6.0_65. I made the effort to keep the tests working under Java 7 as well, but I have not tried it myself; someone should attempt it (under the test source tree: mvn -Dtest=NativeMapIT -DfailIfNoTests=false clean verify). Thanks, Bill Havanki
Re: Review Request 15517: ACCUMULO-1892 examples.simple.RandomBatchWriter might not write the specified number of rowids
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15517/ --- (Updated Nov. 14, 2013, 10:23 p.m.) Review request for accumulo. Changes --- Updated based on feedback from Bill. Now properly sets exit error status on all something went wrong code paths. Bugs: ACCUMULO-1892 https://issues.apache.org/jira/browse/ACCUMULO-1892 Repository: accumulo Description --- ACCUMULO-1892 changes RandomBatchWriter to ensure it writes the specified number of rowids. Diffs (updated) - src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java 3206fa6bbb5e560175e9aec6a1e612185e4ca945 Diff: https://reviews.apache.org/r/15517/diff/ Testing --- unit tests pass. simple.examples.Examples functional tests pass (given fixes in ACCUMULO-1878 to actually run them). Thanks, Sean Busbey
Re: [VOTE] Deprecate mock in 1.6.0
Sent from my phone, please pardon the typos and brevity. On Nov 14, 2013 2:49 PM, Josh Elser josh.el...@gmail.com wrote: +1 Been bitten by goofy/half-implemented stuff in Mock too many times. I'd rather see effort placed into making MAC a reliable and fast testing mechanism than helping Mock limp along -1 MAC isn't there yet and I don't think we should deprecate it without a suitable alternative in place On 11/14/13, 12:41 PM, Keith Turner wrote: Should we deprecate mock accumulo for 1.6.0? This was considered [1] for 1.5.0. I started thinking about this because I never added conditional writer to mock. [1] : https://issues.apache.org/jira/browse/ACCUMULO-878
Re: [VOTE] Deprecate mock in 1.6.0
On 11/14/13, 2:41 PM, John Vines wrote: Sent from my phone, please pardon the typos and brevity. On Nov 14, 2013 2:49 PM, Josh Elser josh.el...@gmail.com wrote: +1 Been bitten by goofy/half-implemented stuff in Mock too many times. I'd rather see effort placed into making MAC a reliable and fast testing mechanism than helping Mock limp along -1 MAC isn't there yet and I don't think we should deprecate it without a suitable alternative in place Can you provide why you think MAC isn't there yet and what you would consider the bar for suitable alternative please? On 11/14/13, 12:41 PM, Keith Turner wrote: Should we deprecate mock accumulo for 1.6.0? This was considered [1] for 1.5.0. I started thinking about this because I never added conditional writer to mock. [1] : https://issues.apache.org/jira/browse/ACCUMULO-878
Re: [VOTE] Deprecate mock in 1.6.0
Sent from my phone, please pardon the typos and brevity. On Nov 14, 2013 4:46 PM, Josh Elser josh.el...@gmail.com wrote: On 11/14/13, 2:41 PM, John Vines wrote: Sent from my phone, please pardon the typos and brevity. On Nov 14, 2013 2:49 PM, Josh Elser josh.el...@gmail.com wrote: +1 Been bitten by goofy/half-implemented stuff in Mock too many times. I'd rather see effort placed into making MAC a reliable and fast testing mechanism than helping Mock limp along -1 MAC isn't there yet and I don't think we should deprecate it without a suitable alternative in place Can you provide why you think MAC isn't there yet and what you would consider the bar for suitable alternative please? Faster to use, easier to use in junit tests, and easier to debug iterators in an ide. Specifically, the spin up time of mock vs Mac is orders of magnitude. Because of that, junit tests should reuse a mock, which I think the maven plugin should help with, but I don't know how well it works and what the effect is on in IDE testing. Lastly, because tservers spin up in new processes, it makes it overwhelmingly difficult to debug an iterator stack. Either mechanisms to improve debug hooks for the processes or an alternative iterator testing suite is in order. On 11/14/13, 12:41 PM, Keith Turner wrote: Should we deprecate mock accumulo for 1.6.0? This was considered [1] for 1.5.0. I started thinking about this because I never added conditional writer to mock. [1] : https://issues.apache.org/jira/browse/ACCUMULO-878
Re: [VOTE] Deprecate mock in 1.6.0
+1 I'd prefer we get a bit more rigorous about the distinction between integration testing and unit testing. MockAccumulo walks that line too closely, and I think it encourages writing bad tests, with the risk that code relies on incorrect mock behavior. I'd prefer to put some effort into making the accumulo-maven-plugin more robust and feature-ful for actual integration testing. In the meantime, raw MiniAccumuloCluster is suitable for ITs. For actual unit tests, we should encourage the use of EasyMock or Mockito or something like that, and stop trying to test end-to-end behavior in unit tests. To unit test iterators, there's already a SortedMapIterator that allows you to create an iterator from a TreeMap. I don't think we should remove it anytime soon, but we should deprecate it. The sooner we deprecate it, the less burden we have to maintain two implementations of every new feature, and the more time we'll have to spend time on improving the tests in other ways. MiniAccumuloCluster can do everything that MockAccumulo can do. The fact that it doesn't run as fast is not a good enough reason, in my opinion, to avoid deprecating it... especially since the test cases where it is being used (where the performance matters) blur the lines between unit tests and integration tests... and that tends to hurt us in terms of code coverage and test reliability. -- Christopher L Tubbs II http://gravatar.com/ctubbsii On Thu, Nov 14, 2013 at 3:41 PM, Keith Turner ke...@deenlo.com wrote: Should we deprecate mock accumulo for 1.6.0? This was considered [1] for 1.5.0. I started thinking about this because I never added conditional writer to mock. [1] : https://issues.apache.org/jira/browse/ACCUMULO-878
Re: [VOTE] Deprecate mock in 1.6.0
+0 - what's the number if I'm undecided? I'd prefer to see one uber-cool one testing mechanism instead of multiples. On Thu, Nov 14, 2013 at 5:54 PM, John Vines vi...@apache.org wrote: Sent from my phone, please pardon the typos and brevity. On Nov 14, 2013 4:46 PM, Josh Elser josh.el...@gmail.com wrote: On 11/14/13, 2:41 PM, John Vines wrote: Sent from my phone, please pardon the typos and brevity. On Nov 14, 2013 2:49 PM, Josh Elser josh.el...@gmail.com wrote: +1 Been bitten by goofy/half-implemented stuff in Mock too many times. I'd rather see effort placed into making MAC a reliable and fast testing mechanism than helping Mock limp along -1 MAC isn't there yet and I don't think we should deprecate it without a suitable alternative in place Can you provide why you think MAC isn't there yet and what you would consider the bar for suitable alternative please? Faster to use, easier to use in junit tests, and easier to debug iterators in an ide. Specifically, the spin up time of mock vs Mac is orders of magnitude. Because of that, junit tests should reuse a mock, which I think the maven plugin should help with, but I don't know how well it works and what the effect is on in IDE testing. Lastly, because tservers spin up in new processes, it makes it overwhelmingly difficult to debug an iterator stack. Either mechanisms to improve debug hooks for the processes or an alternative iterator testing suite is in order. On 11/14/13, 12:41 PM, Keith Turner wrote: Should we deprecate mock accumulo for 1.6.0? This was considered [1] for 1.5.0. I started thinking about this because I never added conditional writer to mock. [1] : https://issues.apache.org/jira/browse/ACCUMULO-878
Re: [VOTE] Deprecate mock in 1.6.0
On Thu, Nov 14, 2013 at 5:54 PM, John Vines vi...@apache.org wrote: Sent from my phone, please pardon the typos and brevity. On Nov 14, 2013 4:46 PM, Josh Elser josh.el...@gmail.com wrote: On 11/14/13, 2:41 PM, John Vines wrote: Sent from my phone, please pardon the typos and brevity. On Nov 14, 2013 2:49 PM, Josh Elser josh.el...@gmail.com wrote: +1 Been bitten by goofy/half-implemented stuff in Mock too many times. I'd rather see effort placed into making MAC a reliable and fast testing mechanism than helping Mock limp along -1 MAC isn't there yet and I don't think we should deprecate it without a suitable alternative in place Can you provide why you think MAC isn't there yet and what you would consider the bar for suitable alternative please? Faster to use, easier to use in junit tests, and easier to debug iterators in an ide. Specifically, the spin up time of mock vs Mac is orders of magnitude. Because of that, junit tests should reuse a mock, which I think the maven plugin should help with, but I don't know how well it works and what the effect is on in IDE testing. Lastly, because tservers spin up in new processes, it makes it overwhelmingly difficult to debug an iterator stack. Either mechanisms to improve debug hooks for the processes or an alternative iterator testing suite is in order. This is a really good point. What about making the deprecation javadoc point to SortedMapIterator? On 11/14/13, 12:41 PM, Keith Turner wrote: Should we deprecate mock accumulo for 1.6.0? This was considered [1] for 1.5.0. I started thinking about this because I never added conditional writer to mock. [1] : https://issues.apache.org/jira/browse/ACCUMULO-878
Native
All- There's been a bit of spam related to native maps lately (reviewboard noise, extra related tickets, patches). I'm aware of some issues related to native maps, and I intend to fix them ASAP. Specifically, there's 3 general problems I'm aware of: 1) the way we're finding/loading native maps is insufficient for the new packaging 2) the integration tests can't find the native maps 3) possibly some issues with running tests on a Mac. I'm in the process of addressing the first two issues (they are strongly related), and hope to have those fixes pushed by Monday. I haven't investigated the Mac issue as much, and will examine it more closely after I address the first two issues. If you want to help, that's great, but I am in progress on these fixes, and they'll be coming soon. I'm a bit backlogged due to being sick. Please be patient. There are fixes coming. I don't want you to duplicate work. -- Christopher L Tubbs II http://gravatar.com/ctubbsii
Re: Review Request 15499: ACCUMULO-1852 - NativeMapIT on OS X
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15499/#review28925 --- This is an issue because the way we find native maps is fundamentally broken in ways that limit packaging and deployment of native maps to a target system. This patch is insufficient to address all those issues. I already have a fix in progress that addresses loading libraries from standard locations in Linux or from the lib directory in ACCUMULO_HOME (for backwards-compatibility), as well as using LD_LIBRARY_PATH or -Djava.librath.path; I will still need to incorporate some modifications to ensure things work well on Mac, but this patch alone doesn't address all the relevant issues. - Christopher Tubbs On Nov. 13, 2013, 5:56 p.m., Bill Havanki wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15499/ --- (Updated Nov. 13, 2013, 5:56 p.m.) Review request for accumulo. Bugs: ACCUMULO-1852 https://issues.apache.org/jira/browse/ACCUMULO-1852 Repository: accumulo Description --- Changes (possibly only initial) made to get NativeMapIT running on Mac OS X. Diffs - assemble/src/main/assemblies/component.xml 3655a22b6271e1f08c479124605df7088efb2ff6 minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb82c46a1a4fd407e36a3ee13e954aa2b40 server/native/src/main/resources/Makefile 6309e6d44514432da491fd8cf8981524e5014abe Diff: https://reviews.apache.org/r/15499/diff/ Testing --- Ran `mvn verify` to completion, both specifically targeting NativeMapIT and running all tests. Platform is OS X 10.8.5, Java 1.6.0_65. I made the effort to keep the tests working under Java 7 as well, but I have not tried it myself; someone should attempt it (under the test source tree: mvn -Dtest=NativeMapIT -DfailIfNoTests=false clean verify). Thanks, Bill Havanki
Re: Hadoop 2.0 Support for Accumulo 1.4 Branch
The main thing is that I would not want to see an ACCUMULO-1790 *without* ACCUMULO-1795. Having 1792 alone would be insufficient for me. -- Christopher L Tubbs II http://gravatar.com/ctubbsii On Tue, Nov 12, 2013 at 9:22 AM, Sean Busbey bus...@clouderagovt.com wrote: On Fri, Oct 18, 2013 at 12:29 AM, Sean Busbey bus...@cloudera.com wrote: On Tue, Oct 15, 2013 at 10:20 AM, Sean Busbey bus...@cloudera.com wrote: On Tue, Oct 15, 2013 at 10:16 AM, Sean Busbey bus...@cloudera.comwrote: On Tue, Oct 15, 2013 at 7:16 AM, dlmar...@comcast.net wrote: Just to be clear, we are talking about adding profile support to the pom's for Hadoop 2.2.0 for a 1.4.5 and 1.5.1 release, correct? We are not talking about changing the default build profile for these branches are we? for 1.4.5-SNAPSHOT I am only talking about adding support Hadoop 2.2.0. I am not suggesting we change the default from building against Hadoop 0.23.203. I mean 0.20.203.0. Ugh, Hadoop versions. Okay, barring additional suggestions, tomorrow afternoon I'll break things down into an umbrella and 3 sub tasks: 1) addition of hadoop 2 support - to include backports of commits - to include making the target hadoop 2 version 2.2.0 - to include test changes that flex hadoop 2 features like fail over 2) ensuring compatibility for 0.20.203 - presuming some subset of the commits in 1) will break it since 0.20 support was left behind in 1.5 3) doc / packaging updates - the issue of binary releases per distro - doc patch for what version(s) the release tests are expected to run against Once work is put against those tickets, I'd expect things to go into a branch based on the umbrella ticket until such time as the complete work can pass the test suite that we'll use at the next release. Then it can get rebased onto the 1.4.x dev branch. -- Sean Based on recent feedback on ACCUMULO-1792 and ACCUMULO-1795, I want to resurrect this thread to make sure everyone's concerns are addressed. For context, here's a link to the start of the last thread: http://bit.ly/1aPqKuH From ACCUMULO-1792, ctubbsii: I'd be reluctant to support any Hadoop 2.x support in the 1.4 release line that breaks compatibility with 0.20. I don't think breaking 0.20 and then possibly fixing it again as a second step is acceptable (because that subsequent work may not ever be done, and I don't think we should break the compatibility contract that we've established with 1.4.0). Chris, I believe keeping all of the work in a branch under the umbrella jira of ACCUMULO-1790 will ensure that we don't end up with a 1.4 release that doesn't have proper support for 0.20.203. Is there something beyond making sure the branch passes a full set of release tests on 0.20.203 that you'd like to see? In the event that the branch only ever contains the work for adding Hadoop 2, it's a simple matter to abandon without rolling into the 1.4 development line. From ACCUMULO-1795, bills (and +1ed by elserj and ctubbsii): I'm very uncomfortable with risking breaking continuity in such an old release, and I don't think managing two lines of 1.4 releases is worth the effort. Though we have no official EOL policy, 1.3 was practically dead in the water once 1.4 was around, and I hope we start encouraging more adoption of 1.5 (and soon 1.6) versus continually propping up 1.4. I'd love to get people to move off of 1.4. However, I think adding Hadoop 2 support to 1.4 encourages this more than leaving it out. Accumulo 1.5.x places a higher burden on HDFS than 1.4 did, and I'm not surprised people find relying on 0.20 for the 1.5 WAL intimidating. Upgrading both HDFS and Accumulo across major versions at once is asking them to take on a bunch of risk. By adding in Hadoop 2 support to 1.4 we allow them to break the risk up into steps: they can upgrade HDFS versions first, get comfortable, then upgrade Accumulo to 1.5. I think the existing tickets under the umbrella of ACCUMULO-1790 should ensure that we end up with a single 1.4 line that can work with either the existing 0.20.203.0 claimed in releases or against 2.2.0. Bill (or Josh or Chris), is there stronger language you'd like to see around docs / packaging (area #3 in the original plan and currently ACCUMULO-1796)? Maybe expressly only doing a binary convenience package for 0.20.203.0? Are you looking for something beyond a full release suite to ensure 1.4 is still maintaining compatibility on Hadoop 0.20.203? -Sean
Re: Hadoop 2.0 Support for Accumulo 1.4 Branch
On Tue, Nov 12, 2013 at 4:49 PM, Sean Busbey busbey...@clouderagovt.com wrote: On Tue, Nov 12, 2013 at 3:14 PM, William Slacum wilhelm.von.cl...@accumulo.net wrote: The language of ACCUMULO-1795 indicated that an acceptable state was something that wasn't binary compatible. That's my #1 thing to avoid. Ah. So I see, not sure why I phrased that that way. Since the default build should still be 0.20.203.0, I'm not sure how it'd end up not being binary compatible. I can update the ticket to clarify the language. Any need to compile should be limited to running Hadoop 2.2.0. Sound good? +1 (The confusing wording was the basis for my concerns also.) Maybe expressly only doing a binary convenience package for 0.20.203.0? If we need an extra package, doesn't that mean a user can't just upgrade Accumulo? By binary convenience package I mean the binary distribution tarball (or rpms, or whatevs) that we make as a part of the release process. For users of Hadoop 0.20.203.0, upgrading should be unchanged from how they would normally get their Accumulo 1.4.x distribution. ACCUMULO-1796 has some leeway about the convenience packages for people who want Hadoop 2 support. On the extreme end, they'd have to build from source and then run a normal upgrade process. I'd prefer binary compatibility with a single build, but if that's too hard to achieve, I have no objection to providing a mechanism to perform an alternate build against 2.x (whether or not we provide a pre-built binary package for it), so long as the default build is 0.20.x -- Christopher L Tubbs II http://gravatar.com/ctubbsii
Re: Hadoop 2.0 Support for Accumulo 1.4 Branch
On Thu, Nov 14, 2013 at 6:27 PM, Christopher ctubb...@apache.org wrote: The main thing is that I would not want to see an ACCUMULO-1790 *without* ACCUMULO-1795. Having 1792 alone would be insufficient for me. That is precisely the intention of ACCUMULO-1790. All of the subtasks (including ACCUMULO-1792 and ACCUMULO-1795) have to be complete for things to get into the 1.4 branch. Until that time the work would just go into a feature branch for ACCUMULO-1790 (to make working and testing easier for those implementing the subtasks). If you wanted to see the full implementation you would just wait until all of the subtasks were committed to the feature branch. Am I missing something? -- Sean
Re: Hadoop 2.0 Support for Accumulo 1.4 Branch
Nope, I think we're on the same page now. -- Christopher L Tubbs II http://gravatar.com/ctubbsii On Thu, Nov 14, 2013 at 7:39 PM, Sean Busbey busbey...@clouderagovt.com wrote: On Thu, Nov 14, 2013 at 6:27 PM, Christopher ctubb...@apache.org wrote: The main thing is that I would not want to see an ACCUMULO-1790 *without* ACCUMULO-1795. Having 1792 alone would be insufficient for me. That is precisely the intention of ACCUMULO-1790. All of the subtasks (including ACCUMULO-1792 and ACCUMULO-1795) have to be complete for things to get into the 1.4 branch. Until that time the work would just go into a feature branch for ACCUMULO-1790 (to make working and testing easier for those implementing the subtasks). If you wanted to see the full implementation you would just wait until all of the subtasks were committed to the feature branch. Am I missing something? -- Sean
Re: Review Request 15482: ACCUMULO-1889 ZooKeeperInstance close method should mark instance closed.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15482/#review28930 --- Ship it! Ship It! - Christopher Tubbs On Nov. 13, 2013, 4:35 p.m., Sean Busbey wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15482/ --- (Updated Nov. 13, 2013, 4:35 p.m.) Review request for accumulo. Bugs: ACCUMULO-1889 https://issues.apache.org/jira/browse/ACCUMULO-1889 Repository: accumulo Description --- ACCUMULO-1889 mark ZKI as closed once close() is called. Once a given ZKI has .close() called, mark that instance closed regardless of outstanding client count. Diffs - src/core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java f657c07a28c3bf330556f0d4e02b9adcb0ea755e Diff: https://reviews.apache.org/r/15482/diff/ Testing --- unit tests pass, functional tests in progress. Thanks, Sean Busbey
ReviewBoard
3 things: #1 ReviewBoard has been really buggy lately. Has anybody else noticed this? Examples: errors when publishing a review, but review still gets published, or moving to a second page on a diff, during a review, but getting an error and losing your unpublished comments from the previous page. #2 I've noticed that ReviewBoard is being used more frequently lately, and for tiny, relatively trivial patches. ReviewBoard is great (when it's working), but I'm not sure it is strictly necessary to submit all contributed patches to ReviewBoard. It's fine if you wish to use it (I don't want to discourage it)... but I just don't want contributors getting the impression that it's a requirement. #3 Don't forget to close out your reviews after the patch has been applied. -- Christopher L Tubbs II http://gravatar.com/ctubbsii
Re: ReviewBoard
bq. getting an error and losing your unpublished comments from the previous page. This happened to me recently. I refreshed the previous page and got my comments back. Cheers On Thu, Nov 14, 2013 at 6:31 PM, Christopher ctubb...@apache.org wrote: 3 things: #1 ReviewBoard has been really buggy lately. Has anybody else noticed this? Examples: errors when publishing a review, but review still gets published, or moving to a second page on a diff, during a review, but getting an error and losing your unpublished comments from the previous page. #2 I've noticed that ReviewBoard is being used more frequently lately, and for tiny, relatively trivial patches. ReviewBoard is great (when it's working), but I'm not sure it is strictly necessary to submit all contributed patches to ReviewBoard. It's fine if you wish to use it (I don't want to discourage it)... but I just don't want contributors getting the impression that it's a requirement. #3 Don't forget to close out your reviews after the patch has been applied. -- Christopher L Tubbs II http://gravatar.com/ctubbsii
Re: ReviewBoard
Same; I've actually seen both cases. It's a bit unpredictable. -- Christopher L Tubbs II http://gravatar.com/ctubbsii On Thu, Nov 14, 2013 at 9:42 PM, Ted Yu yuzhih...@gmail.com wrote: bq. getting an error and losing your unpublished comments from the previous page. This happened to me recently. I refreshed the previous page and got my comments back. Cheers On Thu, Nov 14, 2013 at 6:31 PM, Christopher ctubb...@apache.org wrote: 3 things: #1 ReviewBoard has been really buggy lately. Has anybody else noticed this? Examples: errors when publishing a review, but review still gets published, or moving to a second page on a diff, during a review, but getting an error and losing your unpublished comments from the previous page. #2 I've noticed that ReviewBoard is being used more frequently lately, and for tiny, relatively trivial patches. ReviewBoard is great (when it's working), but I'm not sure it is strictly necessary to submit all contributed patches to ReviewBoard. It's fine if you wish to use it (I don't want to discourage it)... but I just don't want contributors getting the impression that it's a requirement. #3 Don't forget to close out your reviews after the patch has been applied. -- Christopher L Tubbs II http://gravatar.com/ctubbsii
