hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "HBase Review Board (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HBASE-1025) Reconstruction log playback has no bounds on memory used
Date Mon, 14 Jun 2010 22:36:14 GMT

    [ https://issues.apache.org/jira/browse/HBASE-1025?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12878781#action_12878781
] 

HBase Review Board commented on HBASE-1025:
-------------------------------------------

Message from: stack@duboce.net


bq.  On 2010-06-14 14:14:55, Todd Lipcon wrote:
bq.  > Regarding rewriting recovered edits back to the WAL, I'm afraid we may have an issue
with the following situation:
bq.  > - RS A writes edits 1-100 to log
bq.  > - RS A fails
bq.  > - RS B starts recovering log, rewrites edits 1-50
bq.  > - RS B fails while recovering
bq.  > - RS C starts up. It *should* replay 1-100. Instead I fear it might only replay
1-50, or replay 1-100 followed by 1-50, either of which would be incorrect.
bq.  > 
bq.  > We should add a test case where the RS fails in the middle of recovering a prior
failure.
bq.  > 
bq.  > 
bq.  > Also, replayRecoveredEdits returns the highest seqid from the logs. Is it possible
for storeFiles to have a higher seqid than what's in the log in any situation? We should add
some kind of assert perhaps.

On your scenario, yes, there is a hole.  Sorry, meant to point it out.  Was going to file
issue after this went in (Previous to this patch we'd have lost all 1-100 so this is at least
an improvement).  Currently, there is no means for C to startup and replay 1-100.  It has
no access to the recovered edits that B was processing.  As is, it'll replay 1-50 from the
split of B's WAL log.  The splitter needs to look for recovered.edits files and merge them
in for C on startup.


"Is it possible for storeFiles to have a higher seqid than what's in the log in any situation?"

Should not happen but going to add check for it and use the max from store files if its larger
than whats in logs.  Let me add test for this too.


bq.  On 2010-06-14 14:14:55, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 359
bq.  > <http://review.hbase.org/r/179/diff/1/?file=1335#file1335line359>
bq.  >
bq.  >     this returns -1 if there is nothing to replay. Shouldn't it return the maxSeqId
+ 1 perhaps?
bq.  >     
bq.  >     Do we have a test which makes a region, writes some data, closes the region,
opens the region, writes some data, then recovers logs?

Yes, it should return the largest of maxSeqId and replayRecoveredEdits.  Let me add test.

No test that writes edits into a (multifamily region), then opens it again and recovers (I
made comment in test that we need such a thing).  Let me add a test to do this.  It'll likely
flush out more sequenceid issues.


bq.  On 2010-06-14 14:14:55, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 378
bq.  > <http://review.hbase.org/r/179/diff/1/?file=1335#file1335line378>
bq.  >
bq.  >     we used to print the sequence ID after the increment, now we print before the
increment. Maybe change log to "sequenceid for next edit will be NNN"

Done


bq.  On 2010-06-14 14:14:55, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 1904
bq.  > <http://review.hbase.org/r/179/diff/1/?file=1335#file1335line1904>
bq.  >
bq.  >     should probably use LOG.error for these

done


bq.  On 2010-06-14 14:14:55, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 1952
bq.  > <http://review.hbase.org/r/179/diff/1/?file=1335#file1335line1952>
bq.  >
bq.  >     I agree, i don't think the splitter would ever write a partial one. We should
treat an IOE here as a real error

done


bq.  On 2010-06-14 14:14:55, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 1978
bq.  > <http://review.hbase.org/r/179/diff/1/?file=1335#file1335line1978>
bq.  >
bq.  >     when would this happen? deserves a comment I think

done (should never happen -- maybe schema gets changed between crash and replay?  I added
this because in original j-d test he had some messing adding edit for family not in schema...
I kept that messing and added a handler here.  If it goes into HRegion#put/#delete, it'll
cause dirty exception).


bq.  On 2010-06-14 14:14:55, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 1985
bq.  > <http://review.hbase.org/r/179/diff/1/?file=1335#file1335line1985>
bq.  >
bq.  >     can we rename maxSeqIdInLog to currentEditSeqId or something? since that's what
we're really talking about here. Also reversing this inequality to: if (currentEditSeqId <
storeMaxSeqId>) I think it would read more clearly

done


bq.  On 2010-06-14 14:14:55, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 2017
bq.  > <http://review.hbase.org/r/179/diff/1/?file=1335#file1335line2017>
bq.  >
bq.  >     I think this will crash when the region contains stores that were the result
of an HFOF bulk load (they have no sequence id)

You mean it will throw: throw new IllegalAccessError("Has not been initialized");?  Lets fix
this.  Is this your bug from today?


bq.  On 2010-06-14 14:14:55, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 2026
bq.  > <http://review.hbase.org/r/179/diff/1/?file=1335#file1335line2026>
bq.  >
bq.  >     maybe rename to restoreEdit() or something to be clear that this is during restoration
only?

done


bq.  On 2010-06-14 14:14:55, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 2032
bq.  > <http://review.hbase.org/r/179/diff/1/?file=1335#file1335line2032>
bq.  >
bq.  >     possibly more efficient:
bq.  >     Collections.singletonMap(kv.getFamily(), Collections.singletonList(kv))
bq.  >     
bq.  >     though there may be a comparator issue?

Yeah, comparator issue for Map (used your singletonList suggestion though).


bq.  On 2010-06-14 14:14:55, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/Store.java, line 241
bq.  > <http://review.hbase.org/r/179/diff/1/?file=1337#file1337line241>
bq.  >
bq.  >     comment is now out of date, right?

done


bq.  On 2010-06-14 14:14:55, Todd Lipcon wrote:
bq.  > src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java, line 850
bq.  > <http://review.hbase.org/r/179/diff/1/?file=1340#file1340line850>
bq.  >
bq.  >     maybe just Preconditions.checkArgument here so it throws if you try to pass
a non-DFS?

done


- stack


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





> Reconstruction log playback has no bounds on memory used
> --------------------------------------------------------
>
>                 Key: HBASE-1025
>                 URL: https://issues.apache.org/jira/browse/HBASE-1025
>             Project: HBase
>          Issue Type: Bug
>            Reporter: stack
>            Assignee: stack
>             Fix For: 0.21.0
>
>         Attachments: 1025-v2.txt, 1025-v3.txt, 1025-v5.patch, 1025.txt
>
>
> Makes a TreeMap and just keeps adding edits without regard for size of edits applied;
could cause OOME (I've not seen a definitive case though have seen an OOME around time of
a reconstructionlog replay -- perhaps this the straw that broke the fleas antlers?)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message