[
https://issues.apache.org/jira/browse/ZOOKEEPER-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13065819#comment-13065819
]
[email protected] commented on ZOOKEEPER-1107:
----------------------------------------------------------
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. > The documentation (src/docs) need to be updated - specifically the
cleanup section in the admin guide.
bq. >
bq. > 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?
Fixed the documentation part.
@Pat: I guess I'm not clear about how "hooking into JMX or 4 letter-words" will
be helpful. Can you please explain this idea? I can takeup this task in
separate JIRA.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. > ./conf/zoo_sample.cfg, lines 15-21
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22148#file22148line15>
bq. >
bq. > My personal belief is that this should be turned off by default -
i.e. comment out the parameters in the sample config.
Fixed. Also, renamed the configuration keys to make them inline with existing.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. > ./src/java/test/org/apache/zookeeper/ZooKeeperPurgeTest.java, line 37
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22151#file22151line37>
bq. >
bq. > Nice!
Corrected code format issues.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. >
./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java, lines
384-385
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22150#file22150line384>
bq. >
bq. > what does "time" refer to. elapsed time? what are the units.
Fixed.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. >
./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java, lines
374-375
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22150#file22150line374>
bq. >
bq. > the docs should reflect that this only controls the number of snaps
to keep, the logs are purged based on the corresponding purged snaps.
Fixed. Comments are moved to applicable class DatadirCleanupManager and also
documented in admin guide as suggested.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. >
./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java, line
73
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22150#file22150line73>
bq. >
bq. > specify explicit default - e.g. 0.
Fixed.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. >
./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java, line
72
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22150#file22150line72>
bq. >
bq. > specify explicit default, 3?
Fixed.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. > ./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java, line 142
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22149#file22149line142>
bq. >
bq. > Zookeeper should be referred to as "ZooKeeper"
Fixed. Redundant information in logs has been removed.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. > ./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java, line 118
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22149#file22149line118>
bq. >
bq. > I would rather we check if the state is started. (log warning if not)
Fixed.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. > ./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java, line 104
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22149#file22149line104>
bq. >
bq. > given we are tracking the state shouldn't we be testing that here?
Fixed.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. > ./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java, line 103
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22149#file22149line103>
bq. >
bq. > 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)
Called in QuorumPeerMain. Changes included in the latest patch.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. > ./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java, lines 83-86
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22149#file22149line83>
bq. >
bq. > this formatting is not very nice, please adjust it a bit.
Fixed in all the places.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. > ./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java, lines 74-77
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22149#file22149line74>
bq. >
bq. > move this check to the start method.
bq. >
bq. > 1) INFO level log if turned off
bq. > 2) exit the thread if turned off
bq.
bq. Patrick Hunt wrote:
bq. Sorry, meant exit the start method if turned off (don't start the
timer/task).
Fixed.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. > ./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java, line 66
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22149#file22149line66>
bq. >
bq. > perhaps this should be done in start?
Fixed.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. > ./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java, line 45
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22149#file22149line45>
bq. >
bq. > use TimeUnit
Fixed.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. > ./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java, lines 39-43
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22149#file22149line39>
bq. >
bq. > enum?
Introduced new enum PurgeTaskStatus.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. > ./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java, line 38
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22149#file22149line38>
bq. >
bq. > consider calling this something more descriptive - perhaps
"DatadirCleanupManager" or similar...
Renamed the source file and test file as well.
bq. On 2011-07-07 23:40:32, Patrick Hunt wrote:
bq. > ./src/java/main/org/apache/zookeeper/ZooKeeperPurger.java, line 19
bq. > <https://reviews.apache.org/r/1043/diff/1/?file=22149#file22149line19>
bq. >
bq. > should this be in zookeeper or zookeeper.server ?
Moved both source and test files to zookeeper.server package.
- Laxman
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1043/#review998
-----------------------------------------------------------
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
> 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
