virajjasani commented on a change in pull request #3852:
URL: https://github.com/apache/hbase/pull/3852#discussion_r753652443



##########
File path: src/main/asciidoc/_chapters/ops_mgt.adoc
##########
@@ -3150,23 +3150,24 @@ TTL some notion of optional TTL (and optional default 
TTL) for snapshots could b
 hbase> snapshot 'mytable', 'snapshot1234', {TTL => 86400}
 ----
 
-The above command creates snapshot `snapshot1234` with TTL of 86400 sec(24 
hours)
+The above command creates snapshot `snapshot1234` with TTL of 86400 sec (24 
hours)
 and hence, the snapshot is supposed to be cleaned up after 24 hours
 
 
 
 .Default Snapshot TTL:
-
-- FOREVER by default
-- User specified Default TTL with config `hbase.master.snapshot.ttl`
-
-
-While creating a Snapshot, if TTL in seconds is not specified, by default the 
snapshot
-would not be deleted automatically. i.e. it would be retained forever until it 
is
-manually deleted. However, the user can update this default TTL behavior by
-providing default TTL in sec for key: `hbase.master.snapshot.ttl`.
-Value 0 for this config indicates TTL: FOREVER
-
+- User specified default TTL with config `hbase.master.snapshot.ttl`
+- FOREVER if `hbase.master.snapshot.ttl` is not set
+
+While creating a snapshot, if TTL in seconds is not explicitly specified, the 
above logic will be
+followed to determine the TTL. If no configs are changed, the default behavior 
is that all snapshots
+will be retained forever (until manual deletion). If a different default TTL 
behavior is desired,
+`hbase.master.snapshot.ttl` can be set to a default TTL in seconds. Any 
snapshot created without
+an explicit TTL will take this new value.
+
+NOTE: If `hbase.master.snapshot.ttl` is set, a snapshot with an explicit {TTL 
=> 0} or
+{TTL => -1} will also take this value. In this case, a TTL < -1 (such as {TTL 
=> -2} should be used
+to indicate FOREVER.

Review comment:
       @Apache9 yes this behaviour is not perfect in the sense that both values 
`0` and `-1` of TTL represents as if TTL was not provided by user.
   SnapshotDescription at client side has default value of TTL as -1 and 
Snapshot proto has ttl's default value as 0. Since we don't want to update 
default TTL of proto definition (and not make this incompatible change because 
of this), we thought of keeping special meaning to both values: 0 and -1 
(rather than just one of them).
   
   If you really recommend changing behaviour, I would suggest only treating 
`0` as special (as if TTL was not provided by user so use the config) and avoid 
`-1`. But so far, as long as users keep above 4 points in mind, snapshot TTL 
works as expected.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to