Author: stack
Date: Tue Sep 13 19:11:49 2011
New Revision: 1170302
URL: http://svn.apache.org/viewvc?rev=1170302&view=rev
Log:
HBASE-4238 CatalogJanitor can clear a daughter that split before processing its
parent
Modified:
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
Modified:
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
URL:
http://svn.apache.org/viewvc/hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java?rev=1170302&r1=1170301&r2=1170302&view=diff
==============================================================================
---
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
(original)
+++
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
Tue Sep 13 19:11:49 2011
@@ -102,26 +102,7 @@ class CatalogJanitor extends Chore {
// Keep Map of found split parents. There are candidates for cleanup.
// Use a comparator that has split parents come before its daughters.
final Map<HRegionInfo, Result> splitParents =
- new TreeMap<HRegionInfo, Result>(new Comparator<HRegionInfo> () {
- @Override
- public int compare(HRegionInfo left, HRegionInfo right) {
- // This comparator differs from the one HRegionInfo in that it sorts
- // parent before daughters.
- if (left == null) return -1;
- if (right == null) return 1;
- // Same table name.
- int result = Bytes.compareTo(left.getTableDesc().getName(),
- right.getTableDesc().getName());
- if (result != 0) return result;
- // Compare start keys.
- result = Bytes.compareTo(left.getStartKey(), right.getStartKey());
- if (result != 0) return result;
- // Compare end keys.
- result = Bytes.compareTo(left.getEndKey(), right.getEndKey());
- if (result != 0) return -result; // Flip the result so parent comes
first.
- return result;
- }
- });
+ new TreeMap<HRegionInfo, Result>(new SplitParentFirstComparator());
// This visitor collects split parents and counts rows in the .META. table
MetaReader.Visitor visitor = new MetaReader.Visitor() {
@Override
@@ -152,6 +133,31 @@ class CatalogJanitor extends Chore {
}
/**
+ * Compare HRegionInfos in a way that has split parents sort BEFORE their
+ * daughters.
+ */
+ static class SplitParentFirstComparator implements Comparator<HRegionInfo> {
+ @Override
+ public int compare(HRegionInfo left, HRegionInfo right) {
+ // This comparator differs from the one HRegionInfo in that it sorts
+ // parent before daughters.
+ if (left == null) return -1;
+ if (right == null) return 1;
+ // Same table name.
+ int result = Bytes.compareTo(left.getTableDesc().getName(),
+ right.getTableDesc().getName());
+ if (result != 0) return result;
+ // Compare start keys.
+ result = Bytes.compareTo(left.getStartKey(), right.getStartKey());
+ if (result != 0) return result;
+ // Compare end keys.
+ result = Bytes.compareTo(left.getEndKey(), right.getEndKey());
+ if (result != 0) return -result; // Flip the result so parent comes
first.
+ return result;
+ }
+ }
+
+ /**
* Get HRegionInfo from passed Map of row values.
* @param result Map to do lookup in.
* @return Null if not found (and logs fact that expected COL_REGIONINFO
@@ -187,7 +193,7 @@ class CatalogJanitor extends Chore {
checkDaughter(parent, rowContent, HConstants.SPLITA_QUALIFIER);
Pair<Boolean, Boolean> b =
checkDaughter(parent, rowContent, HConstants.SPLITB_QUALIFIER);
- if ((a.getFirst() && !a.getSecond()) && (b.getFirst() && !b.getSecond())) {
+ if (hasNoReferences(a) && hasNoReferences(b)) {
LOG.debug("Deleting region " + parent.getRegionNameAsString() +
" because daughter splits no longer hold references");
// This latter regionOffline should not be necessary but is done for now
@@ -206,7 +212,16 @@ class CatalogJanitor extends Chore {
return result;
}
-
+ /**
+ * @param p A pair where the first boolean says whether or not the daughter
+ * region directory exists in the filesystem and then the second boolean says
+ * whether the daughter has references to the parent.
+ * @return True the passed <code>p</code> signifies no references.
+ */
+ private boolean hasNoReferences(final Pair<Boolean, Boolean> p) {
+ return !p.getFirst() || !p.getSecond();
+ }
+
/**
* See if the passed daughter has references in the filesystem to the parent
* and if not, remove the note of daughter region in the parent row: its
Modified:
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java
URL:
http://svn.apache.org/viewvc/hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java?rev=1170302&r1=1170301&r2=1170302&view=diff
==============================================================================
---
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java
(original)
+++
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java
Tue Sep 13 19:11:49 2011
@@ -276,6 +276,11 @@ public class ServerShutdownHandler exten
if (isDaughterMissing(catalogTracker, daughter)) {
LOG.info("Fixup; missing daughter " + daughter.getRegionNameAsString());
MetaEditor.addDaughter(catalogTracker, daughter, null);
+
+ // TODO: Log WARN if the regiondir does not exist in the fs. If its not
+ // there then something wonky about the split -- things will keep going
+ // but could be missing references to parent region.
+
// And assign it.
assignmentManager.assign(daughter, true);
} else {
Modified:
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
URL:
http://svn.apache.org/viewvc/hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java?rev=1170302&r1=1170301&r2=1170302&view=diff
==============================================================================
---
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
(original)
+++
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
Tue Sep 13 19:11:49 2011
@@ -255,17 +255,28 @@ class SplitTransaction {
HRegion b = createDaughterRegion(this.hri_b, this.parent.flushRequester);
// Edit parent in meta. Offlines parent region and adds splita and splitb.
+ // TODO: This can 'fail' by timing out against .META. but the edits could
+ // be applied anyways over on the server. There is no way to tell for
sure.
+ // We could try and get the edits again subsequent to their application
+ // whether we fail or not but that could fail too. We should probably move
+ // the PONR to here before the edits go in but could mean we'd abort the
+ // regionserver when we didn't need to; i.e. the edits did not make it in.
if (!testing) {
MetaEditor.offlineParentInMeta(server.getCatalogTracker(),
this.parent.getRegionInfo(), a.getRegionInfo(), b.getRegionInfo());
}
// This is the point of no return. Adding subsequent edits to .META. as we
- // do below when we do the daugther opens adding each to .META. can fail in
+ // do below when we do the daughter opens adding each to .META. can fail in
// various interesting ways the most interesting of which is a timeout
- // BUT the edits all go through (See HBASE-3872). IF we reach the POWR
+ // BUT the edits all go through (See HBASE-3872). IF we reach the PONR
// then subsequent failures need to crash out this regionserver; the
// server shutdown processing should be able to fix-up the incomplete
split.
+ // The offlined parent will have the daughters as extra columns. If
+ // we leave the daughter regions in place and do not remove them when we
+ // crash out, then they will have their references to the parent in place
+ // still and the server shutdown fixup of .META. will point to these
+ // regions.
this.journal.add(JournalEntry.PONR);
// Open daughters in parallel.
DaughterOpener aOpener = new DaughterOpener(server, services, a);
@@ -607,7 +618,9 @@ class SplitTransaction {
case PONR:
// We got to the point-of-no-return so we need to just abort. Return
- // immediately.
+ // immediately. Do not clean up created daughter regions. They need
+ // to be in place so we don't delete the parent region mistakenly.
+ // See HBASE-3872.
return false;
default:
