Github user suyogmapara commented on the issue:
https://github.com/apache/zookeeper/pull/567
Hi @anmolnar, do I need to do anything before this can be merged?
---
Github user suyogmapara commented on the issue:
https://github.com/apache/zookeeper/pull/567
@tamaashu can you please take a look and approve?
---
Github user suyogmapara commented on the issue:
https://github.com/apache/zookeeper/pull/567
Thanks @anmolnar, Hi @tamaashu, can you please let me know how should I
move my documentation changes?
---
Github user suyogmapara commented on the issue:
https://github.com/apache/zookeeper/pull/567
Hi @anmolnar please take a look at updated documentation. This time after
regeneration, other parts of the documentation has changed. I have also rebased
on the latest upstream/master
---
Github user suyogmapara commented on the issue:
https://github.com/apache/zookeeper/pull/567
Hi @breed @anmolnar, can you please take a look at this PR?
---
Github user suyogmapara commented on the issue:
https://github.com/apache/zookeeper/pull/567
@anmolnar thank you, I will surely update the docs with some more details.
I am currently waiting to see if there is anything else I need to address in
the next iteration.
---
Github user suyogmapara commented on the issue:
https://github.com/apache/zookeeper/pull/567
Hi @maoling
Regarding 2, I agree that this would be the outcome. But sorry it is not
clear to me why do you think that the result is surprising. You are setting
log-size-limit
Github user suyogmapara commented on the issue:
https://github.com/apache/zookeeper/pull/567
Hi @maoling
I tested the patch with following parameters:
1. PreAllocSize = 100 KB, TxnLogSizeLimit = 200 KB, 100 transactions of
little over 10 KBs. Outcome: 5
Github user suyogmapara commented on the issue:
https://github.com/apache/zookeeper/pull/567
Hi @maoling
Regarding 1, preAllocSize controls how big the contiguous region of
transaction file would be on disk (to reduce number seeks you need to do) while
this feature controls
Github user suyogmapara commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/567#discussion_r210988903
--- Diff:
src/java/test/org/apache/zookeeper/server/persistence/FileTxnLogTest.java ---
@@ -17,9 +17,13 @@
*/
package
Github user suyogmapara commented on the issue:
https://github.com/apache/zookeeper/pull/567
Thanks @anmolnar for the review. Regarding randomization, I think they did
it for snapCount because snapshot is potentially expensive IO and entire
ensemble doing it at the same time may
Github user suyogmapara commented on the issue:
https://github.com/apache/zookeeper/pull/567
@anmolnar I updated the documentation, please let me know if it looks
alright
---
Github user suyogmapara commented on the issue:
https://github.com/apache/zookeeper/pull/567
@anmholnar Sorry I got caught up in other work, I will update this PR with
the documents today.
---
Github user suyogmapara commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/567#discussion_r206338732
--- Diff:
src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -102,6 +102,21 @@
/** Maximum time we allow
Github user suyogmapara commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/567#discussion_r205923249
--- Diff:
src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -102,6 +102,21 @@
/** Maximum time we allow
Github user suyogmapara commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/567#discussion_r205922930
--- Diff:
src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -127,14 +127,11 @@
Long logSize
Github user suyogmapara commented on the issue:
https://github.com/apache/zookeeper/pull/567
@maoling Thanks for the review, regarding concern around PreAllocSize,
thanks to ZOOKEEPER-2249, setting txnLogSizeLimit less than preAllocSize should
not cause any issue. I can add explicit
Github user suyogmapara commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/567#discussion_r205897728
--- Diff: src/java/test/org/apache/zookeeper/test/TxnLogSizeLimitTest.java
---
@@ -0,0 +1,173 @@
+/**
+ * Licensed to the Apache Software
Github user suyogmapara commented on the issue:
https://github.com/apache/zookeeper/pull/567
Thanks for the review @anmolnar. Please take a look at unit tests when you
get a chance. I have addressed the comments. I will also add documentation in a
separate commit.
---
Github user suyogmapara commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/567#discussion_r202855015
--- Diff:
src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -110,6 +124,18 @@
if ((fsyncWarningThreshold
Github user suyogmapara commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/567#discussion_r202854476
--- Diff:
src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -110,6 +124,18 @@
if ((fsyncWarningThreshold
GitHub user suyogmapara opened a pull request:
https://github.com/apache/zookeeper/pull/567
ZOOKEEPER-3071: Add a config parameter to control transaction log size
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/suyogmapara
22 matches
Mail list logo
