[
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: [email protected]
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.
