[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-19 Thread yisong-yue
Github user yisong-yue commented on the issue: https://github.com/apache/zookeeper/pull/690 couldn't reproduce this failure locally. seems like a flakey test case. retest this please ---

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-16 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/690 retest this please ---

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-13 Thread yisong-yue
Github user yisong-yue commented on the issue: https://github.com/apache/zookeeper/pull/690 👍 Thanks for the feedback! 😄 ---

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-13 Thread anmolnar
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/690 @yisong-yue Yeah, that's true. Perhaps it would be too much hassle and I'm trying to over-engineer things here. Let's just leave it as it is now and only do refactoring if we could benefit

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-13 Thread yisong-yue
Github user yisong-yue commented on the issue: https://github.com/apache/zookeeper/pull/690 @anmolnar One drawback I see with this structure is that it makes it harder to dynamically pick deserialization mode based on a snapshot's filename, since `FileSnap` manages a whole

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-12 Thread anmolnar
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/690 @yisong-yue Global state can still be maintained in a static field of a concrete class, it shouldn't be a problem. Though, you're probably right about `SnapStream` cannot maintain a

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-12 Thread yisong-yue
Github user yisong-yue commented on the issue: https://github.com/apache/zookeeper/pull/690 @anmolnar I agree that concrete classes are a lot easier to test. Though in this case, I think it's more appropriate for SnapStream to be a utility class that does not hold any state. (It

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-08 Thread yisong-yue
Github user yisong-yue commented on the issue: https://github.com/apache/zookeeper/pull/690 Here are some benchmark results I did with my laptop: ``` (1) uncompressedsnappy gzip Size: ~ 15.9 MB

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-07 Thread yisong-yue
Github user yisong-yue commented on the issue: https://github.com/apache/zookeeper/pull/690 squashed two commits into one. ---

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-07 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/690 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2610/ --nonenone-- ---

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-07 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/690 @yisong-yue sorry for the spam. There is a problem with the pre-commit job. I won't disturb you anymore on this PR ---

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-07 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/690 retest this please ---

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-07 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/690 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2609/ --nonenone-- ---

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-07 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/690 retest this please ---

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-07 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/690 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2608/ ---

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-07 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/690 retest this please ---

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-06 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/690 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2606/ ---