[ https://issues.apache.org/jira/browse/HBASE-5547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13262207#comment-13262207 ]
jirapos...@reviews.apache.org commented on HBASE-5547: ------------------------------------------------------ ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4633/#review7224 ----------------------------------------------------------- src/main/java/org/apache/hadoop/hbase/HConstants.java <https://reviews.apache.org/r/4633/#comment15915> Things shared across, IMO, should stay here. Also, the znode style seems to be to put it in constants. Probably a bigger issue around how the zkutil works. src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java <https://reviews.apache.org/r/4633/#comment15916> done. src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java <https://reviews.apache.org/r/4633/#comment15921> @Himanshu - yeah, that should help a bit. src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java <https://reviews.apache.org/r/4633/#comment15977> not anymore - new version ignores specifying an archive dir and instead goes with a cluster wide config. src/main/java/org/apache/hadoop/hbase/client/HFileArchiveManager.java <https://reviews.apache.org/r/4633/#comment15980> Done. src/main/java/org/apache/hadoop/hbase/client/HFileArchiveManager.java <https://reviews.apache.org/r/4633/#comment16003> done. src/main/java/org/apache/hadoop/hbase/client/HFileArchiveManager.java <https://reviews.apache.org/r/4633/#comment16004> this class is more of just a way to wrap this up from the admin. Otherwise, it would just be there. src/main/java/org/apache/hadoop/hbase/client/HFileArchiveManager.java <https://reviews.apache.org/r/4633/#comment16005> has to be added, since createWithParents just creates the znode, not the data in that node. Could modify ZKUtil, but this seemed sufficient and don't think we have a lot of use cases where the node has data. If we do, think it would be reasonable to add a zk call. In terms of cost, its basically 1 ping to the server to ensure that the node exists, which would have to happen anyways if we create the table node since we can't bundle calls together until we make ZK 3.4 a requirement. src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java <https://reviews.apache.org/r/4633/#comment16006> moved to o.a.h.h.backup package. src/main/java/org/apache/hadoop/hbase/regionserver/HFileArchiveMonitor.java <https://reviews.apache.org/r/4633/#comment16007> Moved to o.a.h.h.backup, but then still needs to be public. src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java <https://reviews.apache.org/r/4633/#comment16008> done. src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java <https://reviews.apache.org/r/4633/#comment16009> done. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java <https://reviews.apache.org/r/4633/#comment16010> nope as it just uses the passed in zk, rather than creating its own. start here is just a way to bootstrap it to the current state of the system. src/main/java/org/apache/hadoop/hbase/regionserver/Store.java <https://reviews.apache.org/r/4633/#comment16011> Yeah, but if you have an IOException creating a directory you are kind of hosed anyways. src/main/java/org/apache/hadoop/hbase/util/HFileArchiveUtil.java <https://reviews.apache.org/r/4633/#comment16012> Turns out this is overly complicated based on the fact that hashes here are incredibly unlikely to conflict, so still doing a timestamp, but this time just appending the ts to the name of the file and moving it, rather than all this business with directories. src/main/java/org/apache/hadoop/hbase/zookeeper/HFileArchiveTracker.java <https://reviews.apache.org/r/4633/#comment16013> gone, in backup (see above). src/main/java/org/apache/hadoop/hbase/zookeeper/RegionServerHFileTracker.java <https://reviews.apache.org/r/4633/#comment16014> yeah, we are collecting them here, but since its used across all, moved them to their own package. src/main/java/org/apache/hadoop/hbase/zookeeper/RegionServerHFileTracker.java <https://reviews.apache.org/r/4633/#comment16015> its used lower, so RS can hook in confirming joining archiving. src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java <https://reviews.apache.org/r/4633/#comment16016> agreed. Someone should think about it some more and file a jira. Maybe some configuration based loading of short-lived visitor-like classes? - Jesse On 2012-04-07 19:51:11, Jesse Yates wrote: bq. bq. ----------------------------------------------------------- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4633/ bq. ----------------------------------------------------------- bq. bq. (Updated 2012-04-07 19:51:11) bq. bq. bq. Review request for hbase, Michael Stack and Lars Hofhansl. bq. bq. bq. Summary bq. ------- bq. bq. Essentially, whenever an hfile would be deleted, it is instead moved to the archive directory. In this impl, the archive directory is on a per table basis, but defaults to '.archive'. Removing hfiles occurs in three places - compaction, merge and catalog janitor. The former and two latter are distinctly different code paths, but but did pull out some similarities. The latter two end up calling the same method, so there should be a reasonable amount of overlap. bq. bq. Implementation wise: bq. Updated the HMasterInterface to pass the calls onto the zookeeper. bq. Added a zk listener to handle updates from the master to the RS to backup. bq. Added a utility for removing files and finding archive directories bq. Added tests for the regionserver and catalogjanitor approaches. bq. Added creation of manager in regionserver. bq. bq. bq. This addresses bug HBASE-5547. bq. https://issues.apache.org/jira/browse/HBASE-5547 bq. bq. bq. Diffs bq. ----- bq. bq. src/main/java/org/apache/hadoop/hbase/HConstants.java a4b989e bq. src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java 16e4017 bq. src/main/java/org/apache/hadoop/hbase/client/HFileArchiveManager.java PRE-CREATION bq. src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 79d5fdd bq. src/main/java/org/apache/hadoop/hbase/master/HMaster.java fb21bdd bq. src/main/java/org/apache/hadoop/hbase/regionserver/HFileArchiveMonitor.java PRE-CREATION bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java c3df319 bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 8a61f7d bq. src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java 6884d53 bq. src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 509a467 bq. src/main/java/org/apache/hadoop/hbase/util/HFileArchiveUtil.java PRE-CREATION bq. src/main/java/org/apache/hadoop/hbase/zookeeper/HFileArchiveTracker.java PRE-CREATION bq. src/main/java/org/apache/hadoop/hbase/zookeeper/RegionServerHFileTracker.java PRE-CREATION bq. src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java 4fc105f bq. src/main/resources/hbase-default.xml 44ee689 bq. src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 41616c8 bq. src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java b4dcb83 bq. src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionHFileArchiving.java PRE-CREATION bq. src/test/java/org/apache/hadoop/hbase/util/HFileArchiveTestingUtil.java PRE-CREATION bq. src/test/java/org/apache/hadoop/hbase/util/MockRegionServerServices.java 7d02759 bq. bq. Diff: https://reviews.apache.org/r/4633/diff bq. bq. bq. Testing bq. ------- bq. bq. Added two tests for the separate cases - archiving via the regionserver and for the catalog tracker. Former runs in a mini cluster and also touches the changes to HMasterInterface and zookeeper. bq. bq. bq. Thanks, bq. bq. Jesse bq. bq. > Don't delete HFiles when in "backup mode" > ----------------------------------------- > > Key: HBASE-5547 > URL: https://issues.apache.org/jira/browse/HBASE-5547 > Project: HBase > Issue Type: New Feature > Reporter: Lars Hofhansl > Assignee: Jesse Yates > > This came up in a discussion I had with Stack. > It would be nice if HBase could be notified that a backup is in progress (via > a znode for example) and in that case either: > 1. rename HFiles to be delete to <file>.bck > 2. rename the HFiles into a special directory > 3. rename them to a general trash directory (which would not need to be tied > to backup mode). > That way it should be able to get a consistent backup based on HFiles (HDFS > snapshots or hard links would be better options here, but we do not have > those). > #1 makes cleanup a bit harder. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira