hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sergey Shelukhin (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-12454) Setting didPerformCompaction early in HRegion#compact
Date Wed, 12 Nov 2014 21:06:34 GMT

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

Sergey Shelukhin commented on HBASE-12454:
------------------------------------------

Some clarification. Variable name is indeed not good... however I think it is right to be
set before compact call.
The end goal is to call finishCompactionRequest.
If compact is called at all on HStore, it would do so in a finally block (the code above try...
finally is not supposed to fail, I guess it could also be included in a separate try... finally).
If it wasn't called, then it is called in case of failure via cancelRequestedCompaction

The logical explanation is that we have to cancel compactions that we selected but didn't
start; if we start compaction on the store we assume it does appropriate cleanup. Perhaps
comments could be added to that effect and variable renamed "didStartCompaction"

Moving it after the call seems incorrect cause in that case cancel (and thus finishCompactionRequest)
can be called twice.

> Setting didPerformCompaction early in HRegion#compact
> -----------------------------------------------------
>
>                 Key: HBASE-12454
>                 URL: https://issues.apache.org/jira/browse/HBASE-12454
>             Project: HBase
>          Issue Type: Bug
>    Affects Versions: 0.98.8
>            Reporter: Andrew Purtell
>            Assignee: Andrew Purtell
>            Priority: Minor
>             Fix For: 2.0.0, 0.98.9, 0.99.2
>
>         Attachments: HBASE-12454-0.98.patch, HBASE-12454.patch, HBASE-12454.patch, HBASE-12454.patch
>
>
> It appears we are setting 'didPerformCompaction' to "true" before attempting the compaction
in HRegion#compact. If Store#compact throws an exception or is interrupted, we won't call
Store#cancelRequestedCompaction in the last finally block of the method as it looks like we
should.
> {code}
> --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
> +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
> @@ -1540,58 +1540,58 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver
{
>          return false;
>        }
>  
>        status = TaskMonitor.get().createStatus("Compacting " + store + " in " + this);
>        if (this.closed.get()) {
>          String msg = "Skipping compaction on " + this + " because closed";
>          LOG.debug(msg);
>          status.abort(msg);
>          return false;
>        }
>        boolean wasStateSet = false;
>        try {
>          synchronized (writestate) {
>            if (writestate.writesEnabled) {
>              wasStateSet = true;
>              ++writestate.compacting;
>            } else {
>              String msg = "NOT compacting region " + this + ". Writes disabled.";
>              LOG.info(msg);
>              status.abort(msg);
>              return false;
>            }
>          }
>          LOG.info("Starting compaction on " + store + " in region " + this
>              + (compaction.getRequest().isOffPeak()?" as an off-peak compaction":""));
>          doRegionCompactionPrep();
>          try {
>            status.setStatus("Compacting store " + store);
> -          didPerformCompaction = true;
>            store.compact(compaction);
> +          didPerformCompaction = true;
>          } catch (InterruptedIOException iioe) {
>            String msg = "compaction interrupted";
>            LOG.info(msg, iioe);
>            status.abort(msg);
>            return false;
>          }
>        } finally {
>          if (wasStateSet) {
>            synchronized (writestate) {
>              --writestate.compacting;
>              if (writestate.compacting <= 0) {
>                writestate.notifyAll();
>              }
>            }
>          }
>        }
>        status.markComplete("Compaction complete");
>        return true;
>      } finally {
>        try {
>          if (!didPerformCompaction) store.cancelRequestedCompaction(compaction);   <-----
>          if (status != null) status.cleanup();
>        } finally {
>          lock.readLock().unlock();
>        }
>      }
>    }{code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message