hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "stack (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-14030) HBase Backup/Restore Phase 1
Date Mon, 28 Dec 2015 07:08:49 GMT

    [ 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)

Mime
View raw message