Review Board 1.7.22


hbase-4238 CatalogJanitor can clear a daughter that split before processing its parent

Review Request #1819 - Created Sept. 13, 2011 and updated

Michael Stack
hbase-4238
Reviewers
hbase
hbase-git
Previous, we'd not clean up a parent if its daughter region didn't exist in the fs.  This stipulation was added by HBASE-3872.  This patch undoes this barrier to parent cleanup (See  HBASE-3872 for why its ok to do this).

CatalogJanitor

+ Break out the Comparator used by CatalogJanitor.  It was an anonymous class.  Instead we make it a static inner class so can add test that its actually sorting properly.
+ Added method hasNoReferences that will return true if no daughter dir OR no refs in daughter dir

Added some TODOs around SplitTransaction -- vaguely related to this patch.

Added new Test that checks cleanParent to ensure it works properly.  Refactored bits of previous tests so they use common code.

 
src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
Revision b53e9a0 New Change
[20] 31 lines
[+20]
32
import org.apache.hadoop.fs.FileSystem;
32
import org.apache.hadoop.fs.FileSystem;
33
import org.apache.hadoop.fs.Path;
33
import org.apache.hadoop.fs.Path;
34
import org.apache.hadoop.fs.PathFilter;
34
import org.apache.hadoop.fs.PathFilter;
35
import org.apache.hadoop.hbase.Chore;
35
import org.apache.hadoop.hbase.Chore;
36
import org.apache.hadoop.hbase.HColumnDescriptor;
36
import org.apache.hadoop.hbase.HColumnDescriptor;
37
import org.apache.hadoop.hbase.HTableDescriptor;

   
38
import org.apache.hadoop.hbase.HConstants;
37
import org.apache.hadoop.hbase.HConstants;
39
import org.apache.hadoop.hbase.HRegionInfo;
38
import org.apache.hadoop.hbase.HRegionInfo;

    
   
39
import org.apache.hadoop.hbase.HTableDescriptor;
40
import org.apache.hadoop.hbase.Server;
40
import org.apache.hadoop.hbase.Server;
41
import org.apache.hadoop.hbase.TableExistsException;
41
import org.apache.hadoop.hbase.TableExistsException;
42
import org.apache.hadoop.hbase.catalog.MetaEditor;
42
import org.apache.hadoop.hbase.catalog.MetaEditor;
43
import org.apache.hadoop.hbase.catalog.MetaReader;
43
import org.apache.hadoop.hbase.catalog.MetaReader;
44
import org.apache.hadoop.hbase.client.Result;
44
import org.apache.hadoop.hbase.client.Result;
45
import org.apache.hadoop.hbase.regionserver.HRegion;
45
import org.apache.hadoop.hbase.regionserver.HRegion;
46
import org.apache.hadoop.hbase.regionserver.Store;
46
import org.apache.hadoop.hbase.regionserver.Store;
47
import org.apache.hadoop.hbase.regionserver.StoreFile;
47
import org.apache.hadoop.hbase.regionserver.StoreFile;
48
import org.apache.hadoop.hbase.util.Bytes;
48
import org.apache.hadoop.hbase.util.Bytes;
49
import org.apache.hadoop.hbase.util.Pair;
49
import org.apache.hadoop.hbase.util.Pair;
50
import org.apache.hadoop.hbase.util.Bytes;

   
51
import org.apache.hadoop.hbase.util.Writables;
50
import org.apache.hadoop.hbase.util.Writables;
52

    
   
51

   
53

    
   
52

   
54
/**
53
/**
55
 * A janitor for the catalog tables.  Scans the <code>.META.</code> catalog
54
 * A janitor for the catalog tables.  Scans the <code>.META.</code> catalog
[+20] [20] 49 lines
[+20] [+] protected void chore() {
105
    // TODO: Only works with single .META. region currently.  Fix.
104
    // TODO: Only works with single .META. region currently.  Fix.
106
    final AtomicInteger count = new AtomicInteger(0);
105
    final AtomicInteger count = new AtomicInteger(0);
107
    // Keep Map of found split parents.  There are candidates for cleanup.
106
    // Keep Map of found split parents.  There are candidates for cleanup.
108
    // Use a comparator that has split parents come before its daughters.
107
    // Use a comparator that has split parents come before its daughters.
109
    final Map<HRegionInfo, Result> splitParents =
108
    final Map<HRegionInfo, Result> splitParents =
110
      new TreeMap<HRegionInfo, Result>(new Comparator<HRegionInfo> () {
109
      new TreeMap<HRegionInfo, Result>(new SplitParentFirstComparator());
111
        @Override
Moved to 144

   
112
        public int compare(HRegionInfo left, HRegionInfo right) {
Moved to 145

   
113
          // This comparator differs from the one HRegionInfo in that it sorts
Moved to 146

   
114
          // parent before daughters.
Moved to 147

   
115
          if (left == null) return -1;
Moved to 148

   
116
          if (right == null) return 1;
Moved to 149

   
117
          // Same table name.
Moved to 150

   
118
          int result = Bytes.compareTo(left.getTableName(),
Moved to 151

   
119
            right.getTableName());
Moved to 152

   
120
          if (result != 0) return result;
Moved to 153

   
121
          // Compare start keys.
Moved to 154

   
122
          result = Bytes.compareTo(left.getStartKey(), right.getStartKey());
Moved to 155

   
123
          if (result != 0) return result;
Moved to 156

   
124
          // Compare end keys.
Moved to 157

   
125
          result = Bytes.compareTo(left.getEndKey(), right.getEndKey());
Moved to 158

   
126
          if (result != 0) return -result; // Flip the result so parent comes first.
Moved to 159

   
127
          return result;
Moved to 160

   
128
        }
Moved to 161

   
129
      });

   
130
    // This visitor collects split parents and counts rows in the .META. table
110
    // This visitor collects split parents and counts rows in the .META. table
131
    MetaReader.Visitor visitor = new MetaReader.Visitor() {
111
    MetaReader.Visitor visitor = new MetaReader.Visitor() {
132
      @Override
112
      @Override
133
      public boolean visit(Result r) throws IOException {
113
      public boolean visit(Result r) throws IOException {
134
        if (r == null || r.isEmpty()) return true;
114
        if (r == null || r.isEmpty()) return true;
[+20] [20] 20 lines
[+20] public boolean visit(Result r) throws IOException {
155
      " unreferenced parent region(s)");
135
      " unreferenced parent region(s)");
156
    }
136
    }
157
  }
137
  }
158

    
   
138

   
159
  /**
139
  /**

    
   
140
   * Compare HRegionInfos in a way that has split parents sort BEFORE their

    
   
141
   * daughters.

    
   
142
   */

    
   
143
  static class SplitParentFirstComparator implements Comparator<HRegionInfo> {
Moved from 111

    
   
144
    @Override
Moved from 112

    
   
145
    public int compare(HRegionInfo left, HRegionInfo right) {
Moved from 113

    
   
146
      // This comparator differs from the one HRegionInfo in that it sorts
Moved from 114

    
   
147
      // parent before daughters.
Moved from 115

    
   
148
      if (left == null) return -1;
Moved from 116

    
   
149
      if (right == null) return 1;
Moved from 117

    
   
150
      // Same table name.
Moved from 118

    
   
151
      int result = Bytes.compareTo(left.getTableName(),
Moved from 119

    
   
152
          right.getTableName());
Moved from 120

    
   
153
      if (result != 0) return result;
Moved from 121

    
   
154
      // Compare start keys.
Moved from 122

    
   
155
      result = Bytes.compareTo(left.getStartKey(), right.getStartKey());
Moved from 123

    
   
156
      if (result != 0) return result;
Moved from 124

    
   
157
      // Compare end keys.
Moved from 125

    
   
158
      result = Bytes.compareTo(left.getEndKey(), right.getEndKey());
Moved from 126

    
   
159
      if (result != 0) return -result; // Flip the result so parent comes first.
Moved from 127

    
   
160
      return result;
Moved from 128

    
   
161
    }

    
   
162
  }

    
   
163

   

    
   
164
  /**
160
   * Get HRegionInfo from passed Map of row values.
165
   * Get HRegionInfo from passed Map of row values.
161
   * @param result Map to do lookup in.
166
   * @param result Map to do lookup in.
162
   * @return Null if not found (and logs fact that expected COL_REGIONINFO
167
   * @return Null if not found (and logs fact that expected COL_REGIONINFO
163
   * was missing) else deserialized {@link HRegionInfo}
168
   * was missing) else deserialized {@link HRegionInfo}
164
   * @throws IOException
169
   * @throws IOException
[+20] [20] 25 lines
[+20] [+] static HRegionInfo getHRegionInfo(final Result result)
190
    // Run checks on each daughter split.
195
    // Run checks on each daughter split.
191
    Pair<Boolean, Boolean> a =
196
    Pair<Boolean, Boolean> a =
192
      checkDaughter(parent, rowContent, HConstants.SPLITA_QUALIFIER);
197
      checkDaughter(parent, rowContent, HConstants.SPLITA_QUALIFIER);
193
    Pair<Boolean, Boolean> b =
198
    Pair<Boolean, Boolean> b =
194
      checkDaughter(parent, rowContent, HConstants.SPLITB_QUALIFIER);
199
      checkDaughter(parent, rowContent, HConstants.SPLITB_QUALIFIER);
195
    if ((a.getFirst() && !a.getSecond()) && (b.getFirst() && !b.getSecond())) {
200
    if (hasNoReferences(a) && hasNoReferences(b)) {
196
      LOG.debug("Deleting region " + parent.getRegionNameAsString() +
201
      LOG.debug("Deleting region " + parent.getRegionNameAsString() +
197
        " because daughter splits no longer hold references");
202
        " because daughter splits no longer hold references");
198
      // This latter regionOffline should not be necessary but is done for now
203
      // This latter regionOffline should not be necessary but is done for now
199
      // until we let go of regionserver to master heartbeats.  See HBASE-3368.
204
      // until we let go of regionserver to master heartbeats.  See HBASE-3368.
200
      if (this.services.getAssignmentManager() != null) {
205
      if (this.services.getAssignmentManager() != null) {
[+20] [20] 8 lines
[+20] static HRegionInfo getHRegionInfo(final Result result)
209
      result = true;
214
      result = true;
210
    }
215
    }
211
    return result;
216
    return result;
212
  }
217
  }
213

    
   
218

   
214
  

   
215
  /**
219
  /**

    
   
220
   * @param p A pair where the first boolean says whether or not the daughter

    
   
221
   * region directory exists in the filesystem and then the second boolean says

    
   
222
   * whether the daughter has references to the parent.

    
   
223
   * @return True the passed <code>p</code> signifies no references.

    
   
224
   */

    
   
225
  private boolean hasNoReferences(final Pair<Boolean, Boolean> p) {

    
   
226
    return !p.getFirst() || !p.getSecond();

    
   
227
  }

    
   
228

   

    
   
229
  /**
216
   * See if the passed daughter has references in the filesystem to the parent
230
   * See if the passed daughter has references in the filesystem to the parent
217
   * and if not, remove the note of daughter region in the parent row: its
231
   * and if not, remove the note of daughter region in the parent row: its
218
   * column info:splitA or info:splitB.
232
   * column info:splitA or info:splitB.
219
   * @param parent
233
   * @param parent
220
   * @param rowContent
234
   * @param rowContent
[+20] [20] 114 lines
src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java
Revision 742aea4 New Change
 
src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
Revision abafe5e New Change
 
src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java
Revision 78e7d62 New Change
 
  1. src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java: Loading...
  2. src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java: Loading...
  3. src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java: Loading...
  4. src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java: Loading...