[
https://issues.apache.org/jira/browse/HBASE-14030?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15072495#comment-15072495
]
stack commented on HBASE-14030:
-------------------------------
This patch is hard to review. Just looking at HConstants file, it is all
gratuitous reformatting some of which makes the formatting worse (enums all on
one line, lists in javadoc turned into one line making them unreadable in code
and they are lacking html formatting so unreadable as javadoc). As best as I
can tell, four lines are added but there are 40 or 50 changes in this single
file? (I see that the reformatting does not show on RB interestingly....). I
see this reformatting continues through the patch. Would the patch be half the
size if this reformatting were not done? What is odd is that in spite of the
attention to reformatting, we then get this as single change in FSHLog:
{code}
public long getFilenum()
{
return filenum.get();
}
{code}
... which is ill-fomatted... white spacing added before and aft, and then
ill-bracketing.... (Why we letting out filenum here? Ain't that internals?)
What is up w/ this change in root pom:
<surefire.timeout>90000</surefire.timeout>
Are we changing our medium timeout across all tests?
Any doc for this big new feature? Seems like a big one and w/o doc is destined
to rot because no one will know it exsts.
This is interesting:
{code}
105 private static void disableUselessLoggers() {
106 // disable zookeeper log to avoid it mess up command output
107 Logger zkLogger = Logger.getLogger("org.apache.zookeeper");
108 LOG.debug("Zookeeper log level before set: " + zkLogger.getLevel());
109 zkLogger.setLevel(Level.OFF);
110 LOG.debug("Zookeeper log level after set: " + zkLogger.getLevel());
111
112 // disable hbase zookeeper tool log to avoid it mess up command
output
113 Logger hbaseZkLogger =
Logger.getLogger("org.apache.hadoop.hbase.zookeeper");
114 LOG.debug("HBase zookeeper log level before set: " +
hbaseZkLogger.getLevel());
115 hbaseZkLogger.setLevel(Level.OFF);
116 LOG.debug("HBase Zookeeper log level after set: " +
hbaseZkLogger.getLevel());
117
118 // disable hbase client log to avoid it mess up command output
119 Logger hbaseClientLogger =
Logger.getLogger("org.apache.hadoop.hbase.client");
120 LOG.debug("HBase client log level before set: " +
hbaseClientLogger.getLevel());
121 hbaseClientLogger.setLevel(Level.OFF);
122 LOG.debug("HBase client log level after set: " +
hbaseClientLogger.getLevel());
123 }
{code}
We should make it generally available? ZK logging is a PITA.
boolean isTargetExist = false; is bad name for a variable.. its the name
of the method you'd use to check the setting of a variable named targetExist in
javabean-speak
We don't ask the Master to run the backup? I don't see any RPC added in Master.
We don't use new procedure stuff to run backups? I just skimmed patch. Late to
the review.
> HBase Backup/Restore Phase 1
> ----------------------------
>
> Key: HBASE-14030
> URL: https://issues.apache.org/jira/browse/HBASE-14030
> Project: HBase
> Issue Type: Umbrella
> Affects Versions: 2.0.0
> Reporter: Vladimir Rodionov
> Assignee: Vladimir Rodionov
> Fix For: 2.0.0
>
> Attachments: HBASE-14030-v0.patch, HBASE-14030-v1.patch,
> HBASE-14030-v10.patch, HBASE-14030-v11.patch, HBASE-14030-v12.patch,
> HBASE-14030-v13.patch, HBASE-14030-v14.patch, HBASE-14030-v15.patch,
> HBASE-14030-v17.patch, HBASE-14030-v18.patch, HBASE-14030-v2.patch,
> HBASE-14030-v20.patch, HBASE-14030-v21.patch, HBASE-14030-v22.patch,
> HBASE-14030-v23.patch, HBASE-14030-v24.patch, HBASE-14030-v3.patch,
> HBASE-14030-v4.patch, HBASE-14030-v5.patch, HBASE-14030-v6.patch,
> HBASE-14030-v7.patch, HBASE-14030-v8.patch
>
>
> This is the umbrella ticket for Backup/Restore Phase 1. See HBASE-7912 design
> doc for the phase description.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
