Re: Review Request 31836: [HELIX-573] Add support to automatically compress/uncompress data in Zookeeper
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31836/ --- (Updated March 11, 2015, 3:49 p.m.) Review request for helix. Changes --- incorporated the feedback, also added the code in external view compute stage to copy the enableCompression from Idealstate to ExternalView Repository: helix-git Description --- commit 1ef44c2e9a132df3513a51e3a8dac658236a2263 Author: Kishore Gopalakrishna g.kish...@gmail.com Date: Sun Mar 8 16:40:29 2015 -0700 [HELIX-573] Add support to automatically compress/uncompress data in Zookeeper :100644 100644 4419fdd... 1f34529... M helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordSerializer.java :100644 100644 2d7cb3c... 26d7e2b... M helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordStreamingSerializer.java :00 100644 000... 90c1e8e... A helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordSerializer.java :100644 100644 e4b0b25... 95064f8... M helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordStreamingSerializer.java :100755 100755 e869f25... 99ef81c... M hpost-review.sh Diffs (updated) - helix-core/src/main/java/org/apache/helix/controller/stages/ExternalViewComputeStage.java 903840c helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordSerializer.java 4419fdd helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordStreamingSerializer.java 2d7cb3c helix-core/src/main/java/org/apache/helix/util/GZipCompressionUtil.java PRE-CREATION helix-core/src/test/java/org/apache/helix/integration/TestEnableCompression.java PRE-CREATION helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordSerializer.java PRE-CREATION helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordStreamingSerializer.java e4b0b25 Diff: https://reviews.apache.org/r/31836/diff/ Testing --- Added basic test for compress/uncompress Thanks, Kishore Gopalakrishna
Re: Review Request 31836: [HELIX-573] Add support to automatically compress/uncompress data in Zookeeper
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31836/#review76077 --- Ship it! helix-core/src/main/java/org/apache/helix/controller/stages/ExternalViewComputeStage.java https://reviews.apache.org/r/31836/#comment123472 whitespace helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordStreamingSerializer.java https://reviews.apache.org/r/31836/#comment123474 indentation helix-core/src/main/java/org/apache/helix/util/GZipCompressionUtil.java https://reviews.apache.org/r/31836/#comment123473 License should come before imports. helix-core/src/main/java/org/apache/helix/util/GZipCompressionUtil.java https://reviews.apache.org/r/31836/#comment123475 whitespace helix-core/src/test/java/org/apache/helix/integration/TestEnableCompression.java https://reviews.apache.org/r/31836/#comment123476 whitespace helix-core/src/test/java/org/apache/helix/integration/TestEnableCompression.java https://reviews.apache.org/r/31836/#comment123477 whitespace - Kanak Biscuitwala On March 11, 2015, 8:49 a.m., Kishore Gopalakrishna wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31836/ --- (Updated March 11, 2015, 8:49 a.m.) Review request for helix. Repository: helix-git Description --- commit 1ef44c2e9a132df3513a51e3a8dac658236a2263 Author: Kishore Gopalakrishna g.kish...@gmail.com Date: Sun Mar 8 16:40:29 2015 -0700 [HELIX-573] Add support to automatically compress/uncompress data in Zookeeper :100644 100644 4419fdd... 1f34529... M helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordSerializer.java :100644 100644 2d7cb3c... 26d7e2b... M helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordStreamingSerializer.java :00 100644 000... 90c1e8e... A helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordSerializer.java :100644 100644 e4b0b25... 95064f8... M helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordStreamingSerializer.java :100755 100755 e869f25... 99ef81c... M hpost-review.sh Diffs - helix-core/src/main/java/org/apache/helix/controller/stages/ExternalViewComputeStage.java 903840c helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordSerializer.java 4419fdd helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordStreamingSerializer.java 2d7cb3c helix-core/src/main/java/org/apache/helix/util/GZipCompressionUtil.java PRE-CREATION helix-core/src/test/java/org/apache/helix/integration/TestEnableCompression.java PRE-CREATION helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordSerializer.java PRE-CREATION helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordStreamingSerializer.java e4b0b25 Diff: https://reviews.apache.org/r/31836/diff/ Testing --- Added basic test for compress/uncompress Thanks, Kishore Gopalakrishna
Review Request 31836: [HELIX-573] Add support to automatically compress/uncompress data in Zookeeper
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31836/ --- Review request for helix. Repository: helix-git Description --- commit 1ef44c2e9a132df3513a51e3a8dac658236a2263 Author: Kishore Gopalakrishna g.kish...@gmail.com Date: Sun Mar 8 16:40:29 2015 -0700 [HELIX-573] Add support to automatically compress/uncompress data in Zookeeper :100644 100644 4419fdd... 1f34529... M helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordSerializer.java :100644 100644 2d7cb3c... 26d7e2b... M helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordStreamingSerializer.java :00 100644 000... 90c1e8e... A helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordSerializer.java :100644 100644 e4b0b25... 95064f8... M helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordStreamingSerializer.java :100755 100755 e869f25... 99ef81c... M hpost-review.sh Diffs - helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixDataAccessor.java 169c993 helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordSerializer.java 4419fdd helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordStreamingSerializer.java 2d7cb3c helix-core/src/test/java/org/apache/helix/integration/TestBucketizedResource.java 207a318 helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordSerializer.java PRE-CREATION helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordStreamingSerializer.java e4b0b25 hpost-review.sh 82cbcf9 Diff: https://reviews.apache.org/r/31836/diff/ Testing --- Added basic test for compress/uncompress Thanks, Kishore Gopalakrishna
Re: Review Request 31836: [HELIX-573] Add support to automatically compress/uncompress data in Zookeeper
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31836/#review75661
---
helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordSerializer.java
https://reviews.apache.org/r/31836/#comment122869
As currently written, compression will be default because valueOf returns
true if the string is null. I'm not sure if you want that, or at the very
least, it should be made explicit, i.e.:
```
if (record.getBooleanField(enableCompression, defaultValue) {
```
helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordSerializer.java
https://reviews.apache.org/r/31836/#comment122870
Lines 91 and 92 can be combined into 1 line.
helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordSerializer.java
https://reviews.apache.org/r/31836/#comment122868
This log message won't be very useful if the bytes are compressed.
helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordStreamingSerializer.java
https://reviews.apache.org/r/31836/#comment122876
Same comment as above. Use getBooleanField. Also, consider putting this
code in a common area since it's copy-pasted from the other serializer class.
helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordStreamingSerializer.java
https://reviews.apache.org/r/31836/#comment122877
Same comment about code duplication.
helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordStreamingSerializer.java
https://reviews.apache.org/r/31836/#comment122878
This method too.
- Kanak Biscuitwala
On March 8, 2015, 4:48 p.m., Kishore Gopalakrishna wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31836/
---
(Updated March 8, 2015, 4:48 p.m.)
Review request for helix.
Repository: helix-git
Description
---
commit 1ef44c2e9a132df3513a51e3a8dac658236a2263
Author: Kishore Gopalakrishna g.kish...@gmail.com
Date: Sun Mar 8 16:40:29 2015 -0700
[HELIX-573] Add support to automatically compress/uncompress data in
Zookeeper
:100644 100644 4419fdd... 1f34529... M
helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordSerializer.java
:100644 100644 2d7cb3c... 26d7e2b... M
helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordStreamingSerializer.java
:00 100644 000... 90c1e8e... A
helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordSerializer.java
:100644 100644 e4b0b25... 95064f8... M
helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordStreamingSerializer.java
:100755 100755 e869f25... 99ef81c... M hpost-review.sh
Diffs
-
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixDataAccessor.java
169c993
helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordSerializer.java
4419fdd
helix-core/src/main/java/org/apache/helix/manager/zk/ZNRecordStreamingSerializer.java
2d7cb3c
helix-core/src/test/java/org/apache/helix/integration/TestBucketizedResource.java
207a318
helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordSerializer.java
PRE-CREATION
helix-core/src/test/java/org/apache/helix/manager/zk/TestZNRecordStreamingSerializer.java
e4b0b25
hpost-review.sh 82cbcf9
Diff: https://reviews.apache.org/r/31836/diff/
Testing
---
Added basic test for compress/uncompress
Thanks,
Kishore Gopalakrishna
