You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Stack <st...@duboce.net> on 2012/02/03 05:59:50 UTC
Re: svn commit: r1239930 - in /hbase/branches/0.89-fb/src:
main/java/org/apache/hadoop/hbase/master/SplitLogManager.java test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java
Mikhail, the below doesn't have the hbase jira in it?
St.Ack
On Thu, Feb 2, 2012 at 3:30 PM, <mb...@apache.org> wrote:
> Author: mbautin
> Date: Thu Feb 2 23:30:28 2012
> New Revision: 1239930
>
> URL: http://svn.apache.org/viewvc?rev=1239930&view=rev
> Log:
> fix fs.delete(path, false) usage
>
> Summary: Facebook's internal hdfs always fails for fs.delete(path, false). hdfs
> 0.23 works as expected - it will delete path if it is a file or if it is an
> empty directory. This issue is only applicable to 89-fb, so it does need to be
> ported to HBase trunk.
>
> Test Plan: modified unit test. the test fails w/o this diff
>
> Reviewers: kannan, liyintang, pritam
>
> Reviewed By: pritam
>
> CC: hbase-eng@lists
>
> Differential Revision: https://phabricator.fb.com/D400044
>
>
> Modified:
> hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
> hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java
>
> Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
> URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java?rev=1239930&r1=1239929&r2=1239930&view=diff
> ==============================================================================
> --- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java (original)
> +++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java Thu Feb 2 23:30:28 2012
> @@ -275,17 +275,19 @@ public class SplitLogManager implements
> for (Path logDir : logDirs) {
> status.setStatus("Cleaning up log directory...");
> try {
> - if (fs.exists(logDir) && !fs.delete(logDir, false)) {
> - LOG.warn("Unable to delete log src dir. Ignoring. " + logDir);
> + if (fs.exists(logDir)) {
> + FileStatus[] files = fs.listStatus(logDir);
> + if (files == null || files.length == 0) {
> + if (fs.delete(logDir, true) == false) {
> + LOG.warn("Unable to delete log src dir. Ignoring. " + logDir);
> + }
> + } else {
> + LOG.warn("returning success without actually splitting and " +
> + "deleting all the log files in path " + logDir);
> + }
> }
> } catch (IOException ioe) {
> - FileStatus[] files = fs.listStatus(logDir);
> - if (files != null && files.length > 0) {
> - LOG.warn("returning success without actually splitting and " +
> - "deleting all the log files in path " + logDir);
> - } else {
> - LOG.warn("Unable to delete log src dir. Ignoring. " + logDir, ioe);
> - }
> + LOG.warn("Unable to delete log src dir. Ignoring. " + logDir, ioe);
> }
> }
> tot_mgr_log_split_batch_success.incrementAndGet();
>
> Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java
> URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java?rev=1239930&r1=1239929&r2=1239930&view=diff
> ==============================================================================
> --- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java (original)
> +++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java Thu Feb 2 23:30:28 2012
> @@ -37,6 +37,7 @@ import org.apache.hadoop.hbase.master.Sp
> import org.apache.hadoop.hbase.zookeeper.ZKSplitLog;
> import org.apache.hadoop.hbase.zookeeper.ZKSplitLog.TaskState;
> import org.apache.hadoop.hbase.zookeeper.ZooKeeperWrapper;
> +import org.apache.hadoop.hdfs.MiniDFSCluster;
> import org.apache.log4j.Level;
> import org.apache.log4j.Logger;
> import org.apache.zookeeper.CreateMode;
> @@ -436,12 +437,14 @@ public class TestSplitLogManager {
> LOG.info("testEmptyLogDir");
> slm = new SplitLogManager(zkw, conf, stopper, "dummy-master", null);
> slm.finishInitialization();
> - FileSystem fs = TEST_UTIL.getTestFileSystem();
> + MiniDFSCluster cluster = TEST_UTIL.startMiniDFSCluster(1);
> + FileSystem fs = cluster.getFileSystem();
> Path emptyLogDirPath = new Path(fs.getWorkingDirectory(),
> UUID.randomUUID().toString());
> fs.mkdirs(emptyLogDirPath);
> slm.splitLogDistributed(emptyLogDirPath);
> assertFalse(fs.exists(emptyLogDirPath));
> + cluster.shutdown();
> }
>
> @Test
>
>
Re: svn commit: r1239930 - in /hbase/branches/0.89-fb/src:
main/java/org/apache/hadoop/hbase/master/SplitLogManager.java test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java
Posted by Mikhail Bautin <ba...@gmail.com>.
Hi Stack,
Sorry about that. We meant to tag this commit as 89-fb-only, but we are
still figuring out our tagging system. I guess "[master]" would have been
the right tag in this case, since this changes log-splitting functionality.
Thanks,
--Mikhail
On Fri, Feb 3, 2012 at 10:52 AM, Nicolas Spiegelberg <ns...@fb.com>wrote:
> @Stack: this is an 89-master change. It should have that tag. I'll look
> to figure out why the precommit hook isn't working properly.
>
> On 2/2/12 8:59 PM, "Stack" <st...@duboce.net> wrote:
>
> >Mikhail, the below doesn't have the hbase jira in it?
> >St.Ack
> >
> >On Thu, Feb 2, 2012 at 3:30 PM, <mb...@apache.org> wrote:
> >> Author: mbautin
> >> Date: Thu Feb 2 23:30:28 2012
> >> New Revision: 1239930
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1239930&view=rev
> >> Log:
> >> fix fs.delete(path, false) usage
> >>
> >> Summary: Facebook's internal hdfs always fails for fs.delete(path,
> >>false). hdfs
> >> 0.23 works as expected - it will delete path if it is a file or if it
> >>is an
> >> empty directory. This issue is only applicable to 89-fb, so it does
> >>need to be
> >> ported to HBase trunk.
> >>
> >> Test Plan: modified unit test. the test fails w/o this diff
> >>
> >> Reviewers: kannan, liyintang, pritam
> >>
> >> Reviewed By: pritam
> >>
> >> CC: hbase-eng@lists
> >>
> >> Differential Revision: https://phabricator.fb.com/D400044
> >>
> >>
> >> Modified:
> >>
> >>hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/Split
> >>LogManager.java
> >>
> >>hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/master/TestS
> >>plitLogManager.java
> >>
> >> Modified:
> >>hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/Split
> >>LogManager.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apa
> >>che/hadoop/hbase/master/SplitLogManager.java?rev=1239930&r1=1239929&r2=12
> >>39930&view=diff
> >>
> >>=========================================================================
> >>=====
> >> ---
> >>hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/Split
> >>LogManager.java (original)
> >> +++
> >>hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/Split
> >>LogManager.java Thu Feb 2 23:30:28 2012
> >> @@ -275,17 +275,19 @@ public class SplitLogManager implements
> >> for (Path logDir : logDirs) {
> >> status.setStatus("Cleaning up log directory...");
> >> try {
> >> - if (fs.exists(logDir) && !fs.delete(logDir, false)) {
> >> - LOG.warn("Unable to delete log src dir. Ignoring. " +
> >>logDir);
> >> + if (fs.exists(logDir)) {
> >> + FileStatus[] files = fs.listStatus(logDir);
> >> + if (files == null || files.length == 0) {
> >> + if (fs.delete(logDir, true) == false) {
> >> + LOG.warn("Unable to delete log src dir. Ignoring. " +
> >>logDir);
> >> + }
> >> + } else {
> >> + LOG.warn("returning success without actually splitting
> >>and " +
> >> + "deleting all the log files in path " + logDir);
> >> + }
> >> }
> >> } catch (IOException ioe) {
> >> - FileStatus[] files = fs.listStatus(logDir);
> >> - if (files != null && files.length > 0) {
> >> - LOG.warn("returning success without actually splitting and
> >>" +
> >> - "deleting all the log files in path " + logDir);
> >> - } else {
> >> - LOG.warn("Unable to delete log src dir. Ignoring. " +
> >>logDir, ioe);
> >> - }
> >> + LOG.warn("Unable to delete log src dir. Ignoring. " +
> >>logDir, ioe);
> >> }
> >> }
> >> tot_mgr_log_split_batch_success.incrementAndGet();
> >>
> >> Modified:
> >>hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/master/TestS
> >>plitLogManager.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apa
> >>che/hadoop/hbase/master/TestSplitLogManager.java?rev=1239930&r1=1239929&r
> >>2=1239930&view=diff
> >>
> >>=========================================================================
> >>=====
> >> ---
> >>hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/master/TestS
> >>plitLogManager.java (original)
> >> +++
> >>hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/master/TestS
> >>plitLogManager.java Thu Feb 2 23:30:28 2012
> >> @@ -37,6 +37,7 @@ import org.apache.hadoop.hbase.master.Sp
> >> import org.apache.hadoop.hbase.zookeeper.ZKSplitLog;
> >> import org.apache.hadoop.hbase.zookeeper.ZKSplitLog.TaskState;
> >> import org.apache.hadoop.hbase.zookeeper.ZooKeeperWrapper;
> >> +import org.apache.hadoop.hdfs.MiniDFSCluster;
> >> import org.apache.log4j.Level;
> >> import org.apache.log4j.Logger;
> >> import org.apache.zookeeper.CreateMode;
> >> @@ -436,12 +437,14 @@ public class TestSplitLogManager {
> >> LOG.info("testEmptyLogDir");
> >> slm = new SplitLogManager(zkw, conf, stopper, "dummy-master", null);
> >> slm.finishInitialization();
> >> - FileSystem fs = TEST_UTIL.getTestFileSystem();
> >> + MiniDFSCluster cluster = TEST_UTIL.startMiniDFSCluster(1);
> >> + FileSystem fs = cluster.getFileSystem();
> >> Path emptyLogDirPath = new Path(fs.getWorkingDirectory(),
> >> UUID.randomUUID().toString());
> >> fs.mkdirs(emptyLogDirPath);
> >> slm.splitLogDistributed(emptyLogDirPath);
> >> assertFalse(fs.exists(emptyLogDirPath));
> >> + cluster.shutdown();
> >> }
> >>
> >> @Test
> >>
> >>
>
>
Re: svn commit: r1239930 - in /hbase/branches/0.89-fb/src:
main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java
Posted by Nicolas Spiegelberg <ns...@fb.com>.
@Stack: this is an 89-master change. It should have that tag. I'll look
to figure out why the precommit hook isn't working properly.
On 2/2/12 8:59 PM, "Stack" <st...@duboce.net> wrote:
>Mikhail, the below doesn't have the hbase jira in it?
>St.Ack
>
>On Thu, Feb 2, 2012 at 3:30 PM, <mb...@apache.org> wrote:
>> Author: mbautin
>> Date: Thu Feb 2 23:30:28 2012
>> New Revision: 1239930
>>
>> URL: http://svn.apache.org/viewvc?rev=1239930&view=rev
>> Log:
>> fix fs.delete(path, false) usage
>>
>> Summary: Facebook's internal hdfs always fails for fs.delete(path,
>>false). hdfs
>> 0.23 works as expected - it will delete path if it is a file or if it
>>is an
>> empty directory. This issue is only applicable to 89-fb, so it does
>>need to be
>> ported to HBase trunk.
>>
>> Test Plan: modified unit test. the test fails w/o this diff
>>
>> Reviewers: kannan, liyintang, pritam
>>
>> Reviewed By: pritam
>>
>> CC: hbase-eng@lists
>>
>> Differential Revision: https://phabricator.fb.com/D400044
>>
>>
>> Modified:
>>
>>hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/Split
>>LogManager.java
>>
>>hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/master/TestS
>>plitLogManager.java
>>
>> Modified:
>>hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/Split
>>LogManager.java
>> URL:
>>http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apa
>>che/hadoop/hbase/master/SplitLogManager.java?rev=1239930&r1=1239929&r2=12
>>39930&view=diff
>>
>>=========================================================================
>>=====
>> ---
>>hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/Split
>>LogManager.java (original)
>> +++
>>hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/Split
>>LogManager.java Thu Feb 2 23:30:28 2012
>> @@ -275,17 +275,19 @@ public class SplitLogManager implements
>> for (Path logDir : logDirs) {
>> status.setStatus("Cleaning up log directory...");
>> try {
>> - if (fs.exists(logDir) && !fs.delete(logDir, false)) {
>> - LOG.warn("Unable to delete log src dir. Ignoring. " +
>>logDir);
>> + if (fs.exists(logDir)) {
>> + FileStatus[] files = fs.listStatus(logDir);
>> + if (files == null || files.length == 0) {
>> + if (fs.delete(logDir, true) == false) {
>> + LOG.warn("Unable to delete log src dir. Ignoring. " +
>>logDir);
>> + }
>> + } else {
>> + LOG.warn("returning success without actually splitting
>>and " +
>> + "deleting all the log files in path " + logDir);
>> + }
>> }
>> } catch (IOException ioe) {
>> - FileStatus[] files = fs.listStatus(logDir);
>> - if (files != null && files.length > 0) {
>> - LOG.warn("returning success without actually splitting and
>>" +
>> - "deleting all the log files in path " + logDir);
>> - } else {
>> - LOG.warn("Unable to delete log src dir. Ignoring. " +
>>logDir, ioe);
>> - }
>> + LOG.warn("Unable to delete log src dir. Ignoring. " +
>>logDir, ioe);
>> }
>> }
>> tot_mgr_log_split_batch_success.incrementAndGet();
>>
>> Modified:
>>hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/master/TestS
>>plitLogManager.java
>> URL:
>>http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apa
>>che/hadoop/hbase/master/TestSplitLogManager.java?rev=1239930&r1=1239929&r
>>2=1239930&view=diff
>>
>>=========================================================================
>>=====
>> ---
>>hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/master/TestS
>>plitLogManager.java (original)
>> +++
>>hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/master/TestS
>>plitLogManager.java Thu Feb 2 23:30:28 2012
>> @@ -37,6 +37,7 @@ import org.apache.hadoop.hbase.master.Sp
>> import org.apache.hadoop.hbase.zookeeper.ZKSplitLog;
>> import org.apache.hadoop.hbase.zookeeper.ZKSplitLog.TaskState;
>> import org.apache.hadoop.hbase.zookeeper.ZooKeeperWrapper;
>> +import org.apache.hadoop.hdfs.MiniDFSCluster;
>> import org.apache.log4j.Level;
>> import org.apache.log4j.Logger;
>> import org.apache.zookeeper.CreateMode;
>> @@ -436,12 +437,14 @@ public class TestSplitLogManager {
>> LOG.info("testEmptyLogDir");
>> slm = new SplitLogManager(zkw, conf, stopper, "dummy-master", null);
>> slm.finishInitialization();
>> - FileSystem fs = TEST_UTIL.getTestFileSystem();
>> + MiniDFSCluster cluster = TEST_UTIL.startMiniDFSCluster(1);
>> + FileSystem fs = cluster.getFileSystem();
>> Path emptyLogDirPath = new Path(fs.getWorkingDirectory(),
>> UUID.randomUUID().toString());
>> fs.mkdirs(emptyLogDirPath);
>> slm.splitLogDistributed(emptyLogDirPath);
>> assertFalse(fs.exists(emptyLogDirPath));
>> + cluster.shutdown();
>> }
>>
>> @Test
>>
>>