Mailing-List: contact dev-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hbase.apache.org
Received-SPF: neutral (athena.apache.org: local policy)
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Subject: Re: Review Request: Cache flush should go to tmp dir
From: "Todd Lipcon" <todd@cloudera.com>
To: "Ryan Rawson" <ryanobjc@gmail.com>, stack@duboce.net
Date: Tue, 22 Jun 2010 07:22:56 -0000
Message-ID: <20100622072256.15081.87917@ip-10-250-199-31.ec2.internal>
Cc: "Todd Lipcon" <todd@cloudera.com>, jiraposter@review.hbase.org,
	dev@hbase.apache.org
In-Reply-To: <20100622071405.15083.32456@ip-10-250-199-31.ec2.internal>
References: <20100622071405.15083.32456@ip-10-250-199-31.ec2.internal>


> On 2010-06-22 00:14:05, stack wrote:
> > +1 (if it passes all tests).  Nit-picks below.

Cool, I'll run it through my Hudson overnight, plus running a cluster test =
on my 5-node test cluster now. Will commit tomorrow midday with changes bel=
ow addressed assuming testing goes OK.


> On 2010-06-22 00:14:05, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/Store.java, line 333
> > <http://review.hbase.org/r/216/diff/1/?file=3D1529#file1529line333>
> >
> >     ? There was notion of a '_tmp' already?
> >     =

> >     I'd say name it '.tmp'... since a '.' prefix seems to be our conven=
tion given logs dir at top-level has a '.' prefix.

yea, the bulk load stuff uses a tmp dir if you ask the regionserver to load=
 a file which is stored on a different filesystem than the region itself.

I'll switch to .tmp though as you suggest (and move it into HConstants to r=
eplace the now-unused compactions.dir or what have you)


> On 2010-06-22 00:14:05, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/Store.java, line 464
> > <http://review.hbase.org/r/216/diff/1/?file=3D1529#file1529line464>
> >
> >     Why not keep old name and just move dirs?  Why create a new unique =
name?

I was worried that what's unique in the old dir might not be unique in the =
new


> On 2010-06-22 00:14:05, stack wrote:
> > src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java, line=
 411
> > <http://review.hbase.org/r/216/diff/1/?file=3D1531#file1531line411>
> >
> >     Nice test.

thx! at some point I'd like to combine this with the one in TestFSErrorsExp=
osed and make a little mini-framework out of it... something for next month=
 perhaps.


- Todd


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/216/#review261
-----------------------------------------------------------


On 2010-06-21 23:57:39, Todd Lipcon wrote:
> =

> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/216/
> -----------------------------------------------------------
> =

> (Updated 2010-06-21 23:57:39)
> =

> =

> Review request for hbase, stack and Ryan Rawson.
> =

> =

> Summary
> -------
> =

> Fixes bugs where an exception in the middle of flushing a file leaves a h=
alf-written StoreFile in the region dir, preventing that region from recove=
ring, or, in the case of transient errors, causing silent loss of half a fi=
le's worth of data.
> =

> I also got rid of the compaction dir here, and am just using one region-w=
ide tmp dir. Is there some reason this is a bad idea?
> =

> =

> This addresses bug HBASE-2729.
>     http://issues.apache.org/jira/browse/HBASE-2729
> =

> =

> Diffs
> -----
> =

>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1794df8 =

>   src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 04b7522 =

>   src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java =
9e5ca46 =

>   src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java a65e9=
47 =

> =

> Diff: http://review.hbase.org/r/216/diff
> =

> =

> Testing
> -------
> =

> Ran TestCompaction and TestStore. Will start a cluster test running befor=
e I go to bed.
> =

> =

> Thanks,
> =

> Todd
> =

>