[
https://issues.apache.org/jira/browse/ZOOKEEPER-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13067981#comment-13067981
]
[email protected] commented on ZOOKEEPER-1107:
----------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1043/#review1116
-----------------------------------------------------------
This is looking pretty close!
./conf/zoo_sample.cfg
<https://reviews.apache.org/r/1043/#comment2277>
I think something like
datadircleanupmanager.snapRetainCount
and
datadircleanupmanager.purgeInterval
would be better here -- less ambiguous
(in general we need to cleanup our configuration naming/handling at some
point)
./src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml
<https://reviews.apache.org/r/1043/#comment2281>
mention that this is "new in 3.4.0" -- see some of the other parameters
such as "clientPortAddress" for an example of how to do this.
./src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml
<https://reviews.apache.org/r/1043/#comment2278>
something like the following?
the <..>snapretaincount<..> most recent snapshots ...
./src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml
<https://reviews.apache.org/r/1043/#comment2279>
replace "purges" with "deletes"?
./src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml
<https://reviews.apache.org/r/1043/#comment2280>
do you mean something like:
"Default is 3. Minimum value is 3."
I think this would be a bit more obvious. Might also be good if we print a
warning if user sets to less than 3 (and specify we are using 3)
./src/java/main/org/apache/zookeeper/server/DatadirCleanupManager.java
<https://reviews.apache.org/r/1043/#comment2282>
can we add a class javadoc describing this class?
./src/java/main/org/apache/zookeeper/server/DatadirCleanupManager.java
<https://reviews.apache.org/r/1043/#comment2283>
should be "DataDirCleanupManager.class" not nioservercnxn.class.
./src/java/main/org/apache/zookeeper/server/DatadirCleanupManager.java
<https://reviews.apache.org/r/1043/#comment2285>
move this to QPC, check it there, print a warning if user sets below this,
and set the config field to this MIN.
./src/java/main/org/apache/zookeeper/server/DatadirCleanupManager.java
<https://reviews.apache.org/r/1043/#comment2286>
if we set this in the constructor we can define these as final.
./src/java/main/org/apache/zookeeper/server/DatadirCleanupManager.java
<https://reviews.apache.org/r/1043/#comment2284>
rather than add a dependency on QPC, could we pass these 4 parameters into
a constructor for this class? Notice how this also makes your tests easier (no
need to meddle with config).
shouldn't we check the state before starting?
./src/java/main/org/apache/zookeeper/server/DatadirCleanupManager.java
<https://reviews.apache.org/r/1043/#comment2287>
move this to QPC
./src/java/main/org/apache/zookeeper/server/DatadirCleanupManager.java
<https://reviews.apache.org/r/1043/#comment2288>
I'd move this to the constructor of this class.
./src/java/main/org/apache/zookeeper/server/DatadirCleanupManager.java
<https://reviews.apache.org/r/1043/#comment2289>
nit - formatting is a bit off, typ
if (foo < bar) {
...
}
./src/java/main/org/apache/zookeeper/server/DatadirCleanupManager.java
<https://reviews.apache.org/r/1043/#comment2292>
final
- Patrick
On 2011-07-19 21:04:20, 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-19 21:04:20)
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 1146568
bq. ./src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 1146568
bq. ./src/java/main/org/apache/zookeeper/server/DatadirCleanupManager.java
PRE-CREATION
bq. ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
1146568
bq. ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java
1146568
bq.
./src/java/test/org/apache/zookeeper/server/DatadirCleanupManagerTest.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
> Assignee: Laxman
> Attachments: ZOOKEEPER-1107.1.patch, ZOOKEEPER-1107.2.patch,
> ZOOKEEPER-1107.3.patch, ZOOKEEPER-1107.4.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
