[
https://issues.apache.org/jira/browse/HBASE-3323?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12970640#action_12970640
]
stack commented on HBASE-3323:
------------------------------
I have a bunch of praise/+1s for changes made in this patch -- the refactoring
of where params are passed to HLogSplitter, the new javadoc on the intern'ing
methods, the overall less moving parts -- but will say no more on that other
than the improvements are great.
Whats this for:
{code}
+ protected final Path rootDir;
+ protected final Path srcDir;
+ protected final Path oldLogDir;
+ protected final FileSystem fs;
+ protected final Configuration conf;
{code}
Is stuff protected rather than private for the subclassers -- the transactional
hbasers?
Minor.... the below javadoc is now stale..
{code}
* Create a new HLogSplitter using the given {...@link Configuration} and the
* <code>hbase.hlog.splitter.impl</code> property to derived the instance
* class to use.
{code}
Address on commit?
Same for this stuff:
{code}
+ * @param oldLogDir directory where processed (split) logs will be archived
to
+ * @param fs FileSystem
+ * @param conf Configuration
+ * @throws IOException will throw if corrupted hlogs aren't tolerated
...
{code}
Its javadoc of params no longer present on this method.
Mistype '+ "An HLogSplitter instance may only be used one");'
Extremely minor comment, the below formatting will be destroyed when rendered
by javadoc:
{code}
+ * through the logs to be split. For each log, we:
+ * - Recover it (take and drop HDFS lease) to ensure no other process can
write
+ * - Read each edit (see {...@link #parseHLog}
+ * - Mark as "processed" or "corrupt" depending on outcome
{code}
(... but good documentation).
It would be sweeter if this were a percentage of heap rather than hard MB
number --> maxHeapUsage .... but no biggie. Can do in later issue.
So, it looks like we keep the order in which edits were written across the
split process as best as I can tell. We just append to the Entry List in
RegionEntryBuffer. That looks right.
(Reading this patch makes me reconsider asserts)
You iterate logWriters, a synchronized Map, a couple of times. Is this safe at
the time of iteration?
You keep the old format for naming edit files? Naming them for the sequenceid
of their first edit, it seems (you use getRegionSplitEditsPath -- not in the
patch).
On the below
{code}
+ if (scopes != null) {
+ ret += ClassSize.TREEMAP;
+ ret += ClassSize.align(scopes.size() * ClassSize.MAP_ENTRY);
+ // TODO this isn't quite right, need help here
+ }
{code}
... maybe Jon can help -- but its fine for the moment I'd say.
> OOME in master splitting logs
> -----------------------------
>
> Key: HBASE-3323
> URL: https://issues.apache.org/jira/browse/HBASE-3323
> Project: HBase
> Issue Type: Bug
> Components: master
> Affects Versions: 0.90.0
> Reporter: Todd Lipcon
> Assignee: Todd Lipcon
> Priority: Blocker
> Fix For: 0.90.0
>
> Attachments: hbase-3323.4.txt, hbase-3323.txt, hbase-3323.txt,
> hbase-3323.txt, sizes.png
>
>
> In testing a RS failure under heavy increment workload I ran into an OOME
> when the master was splitting the logs.
> In this test case, I have exactly 136 bytes per log entry in all the logs,
> and the logs are all around 66-74MB). With a batch size of 3 logs, this means
> the master is loading about 500K-600K edits per log file. Each edit ends up
> creating 3 byte[] objects, the references for which are each 8 bytes of RAM,
> so we have 160 (136+8*3) bytes per edit used by the byte[]. For each edit we
> also allocate a bunch of other objects: one HLog$Entry, one WALEdit, one
> ArrayList, one LinkedList$Entry, one HLogKey, and one KeyValue. Overall this
> works out to 400 bytes of overhead per edit. So, with the default settings on
> this fairly average workload, the 1.5M log entries takes about 770MB of RAM.
> Since I had a few log files that were a bit larger (around 90MB) it exceeded
> 1GB of RAM and I got an OOME.
> For one, the 400 bytes per edit overhead is pretty bad, and we could probably
> be a lot more efficient. For two, we should actually account this rather than
> simply having a configurable "batch size" in the master.
> I think this is a blocker because I'm running with fairly default configs
> here and just killing one RS made the cluster fall over due to master OOME.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
