hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ted Yu (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-8541) implement flush-into-stripes in stripe compactions
Date Wed, 12 Jun 2013 17:11:20 GMT

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

Ted Yu commented on HBASE-8541:
-------------------------------

{code}
+  public boolean doFlushToL0() {
+    return flushIntoL0;
{code}
doFlushToL0 sounds like the method would do the actual flushing. Please rename.
{code}
-  private class CompactionResultsMergeCopy {
+  private class CompactionOrFlushMergeCopy {
{code}
Mind updating javadoc for the above method w.r.t. flushing ?
{code}
+          if (isFlush) {
+            long newSize = StripeCompactionPolicy.getTotalFileSize(newStripes.values());
+            LOG.error("Stripes were created by a flush, but results of size " + newSize
+                + " cannot be added due to a concurrent flush also creating stripes");
+            canAddNewStripes = false;
+            filesForL0 = newStripes.values();
{code}
In code above, newStripes is put in filesForL0 - meaning we ignore conflictingFiles. But the
error message says that newSize cannot be added. This seems to be inconsistent, right ?
{code}
+public class StripeStoreFlusher extends StoreFlusher {
+  private static final Log LOG = LogFactory.getLog(DefaultStoreFlusher.class);
{code}
Add audience annotation.
Log gets wrong class.
{code}
+    } finally {
+      if (!success && (mw != null)) {
+        result.clear();
+        for (StoreFile.Writer writer : mw.getAllWriters()) {
+          if (writer == null) continue;
+          writer.close();
{code}
You may want to catch IOException w.r.t. writer.close() call.
{code}
+    if (scanner == null) {
+      return null; // NULL from coprocessor here means skip normal processing
+    }
+    return scanner;
{code}
'return scanner' should be good enough for above.
{code}
+     * @param targetCount The maximum number of stripes to flush into.
+     * @param targetKvs The KV count of each segment. If targetKvs*targetCount is less than
{code}
Can the above two parameters get better names ? How about targetStripeCount and targetKvsPerStripe
?


                
> implement flush-into-stripes in stripe compactions
> --------------------------------------------------
>
>                 Key: HBASE-8541
>                 URL: https://issues.apache.org/jira/browse/HBASE-8541
>             Project: HBase
>          Issue Type: Improvement
>            Reporter: Sergey Shelukhin
>         Attachments: HBASE-8541-latest-with-dependencies.patch, HBASE-8541-v0.patch
>
>
> Flush will be able to flush into multiple files under this design, avoiding L0 I/O amplification.
> I have the patch which is missing just one feature - support for concurrent flushes and
stripe changes. This can be done via extensive try-locking of stripe changes and flushes,
or advisory flags without blocking flushes, dumping conflicting flushes into L0 in case of
(very rare) collisions. For file loading for the latter, a set-cover-like problem needs to
be solved to determine optimal stripes. That will also address Jimmy's concern of getting
rid of metadata, btw. However currently I don't have time for that. I plan to attach the try-locking
patch first, but this won't happen for a couple weeks probably and should not block main reviews.
Hopefully this will be added on top of main reviews.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message