[
https://issues.apache.org/jira/browse/ZOOKEEPER-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13061675#comment-13061675
]
[email protected] commented on ZOOKEEPER-1107:
----------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1043/#review998
-----------------------------------------------------------
The documentation (src/docs) need to be updated - specifically the cleanup
section in the admin guide.
Have you considered hooking this into JMX or the 4letterwords? It would be nice
for operators to get basic information. In JMX they could also control the
settings... consider for a follow-on JIRA?
./conf/zoo_sample.cfg
<https://reviews.apache.org/r/1043/#comment2060>
My personal belief is that this should be turned off by default - i.e.
comment out the parameters in the sample config.
./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java
<https://reviews.apache.org/r/1043/#comment2081>
should this be in zookeeper or zookeeper.server ?
./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java
<https://reviews.apache.org/r/1043/#comment2080>
consider calling this something more descriptive - perhaps
"DatadirCleanupManager" or similar...
./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java
<https://reviews.apache.org/r/1043/#comment2069>
enum?
./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java
<https://reviews.apache.org/r/1043/#comment2070>
use TimeUnit
./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java
<https://reviews.apache.org/r/1043/#comment2076>
perhaps this should be done in start?
./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java
<https://reviews.apache.org/r/1043/#comment2074>
move this check to the start method.
1) INFO level log if turned off
2) exit the thread if turned off
./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java
<https://reviews.apache.org/r/1043/#comment2073>
this formatting is not very nice, please adjust it a bit.
./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java
<https://reviews.apache.org/r/1043/#comment2075>
I see tests, which is great, however where is this method being called in
ZooKeeper server code proper? (what I mean is the server doesn't seem to be
running this)
./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java
<https://reviews.apache.org/r/1043/#comment2079>
given we are tracking the state shouldn't we be testing that here?
./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java
<https://reviews.apache.org/r/1043/#comment2077>
I would rather we check if the state is started. (log warning if not)
./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java
<https://reviews.apache.org/r/1043/#comment2078>
Zookeeper should be referred to as "ZooKeeper"
./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
<https://reviews.apache.org/r/1043/#comment2067>
specify explicit default, 3?
./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
<https://reviews.apache.org/r/1043/#comment2068>
specify explicit default - e.g. 0.
./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
<https://reviews.apache.org/r/1043/#comment2065>
the docs should reflect that this only controls the number of snaps to
keep, the logs are purged based on the corresponding purged snaps.
./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
<https://reviews.apache.org/r/1043/#comment2066>
what does "time" refer to. elapsed time? what are the units.
./src/java/test/org/apache/zookeeper/ZooKeeperPurgeTest.java
<https://reviews.apache.org/r/1043/#comment2082>
Nice!
- Patrick
On 2011-07-07 23:10:13, Patrick Hunt wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/1043/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-07-07 23:10:13)
bq.
bq.
bq. Review request for zookeeper, Patrick Hunt, Benjamin Reed, and Mahadev
Konar.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. I like to have ZK itself manage the amount of snapshots and logs kept,
instead of relying on the PurgeTxnLog utility.
bq.
bq.
bq. This addresses bug ZOOKEEPER-1107.
bq. https://issues.apache.org/jira/browse/ZOOKEEPER-1107
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. ./conf/zoo_sample.cfg 1141901
bq. ./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java PRE-CREATION
bq. ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
1141901
bq. ./src/java/test/org/apache/zookeeper/ZooKeeperPurgeTest.java
PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/1043/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. test added, passing hudson qa bot.
bq.
bq.
bq. Thanks,
bq.
bq. Patrick
bq.
bq.
> automating log and snapshot cleaning
> ------------------------------------
>
> Key: ZOOKEEPER-1107
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1107
> Project: ZooKeeper
> Issue Type: Improvement
> Components: server
> Affects Versions: 3.3.3
> Reporter: Jun Rao
> Attachments: ZOOKEEPER-1107.1.patch, ZOOKEEPER-1107.2.patch,
> ZOOKEEPER-1107.3.patch, ZOOKEEPER-1107.patch
>
>
> I like to have ZK itself manage the amount of snapshots and logs kept,
> instead of relying on the PurgeTxnLog utility.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
