Author: jimk
Date: Mon May 5 16:56:31 2008
New Revision: 653642
URL: http://svn.apache.org/viewvc?rev=653642&view=rev
Log:
HBASE-478 offlining of table does not run reliably
Modified:
hadoop/hbase/branches/0.1/CHANGES.txt
hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HMaster.java
hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HStore.java
hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestHBaseCluster.java
Modified: hadoop/hbase/branches/0.1/CHANGES.txt
URL:
http://svn.apache.org/viewvc/hadoop/hbase/branches/0.1/CHANGES.txt?rev=653642&r1=653641&r2=653642&view=diff
==============================================================================
--- hadoop/hbase/branches/0.1/CHANGES.txt (original)
+++ hadoop/hbase/branches/0.1/CHANGES.txt Mon May 5 16:56:31 2008
@@ -30,7 +30,8 @@
HBASE-609 Master doesn't see regionserver edits because of clock skew
HBASE-607 MultiRegionTable.makeMultiRegionTable is not deterministic
enough
for regression tests
-
+ HBASE-478 offlining of table does not run reliably
+
IMPROVEMENTS
HBASE-559 MR example job to count table rows
HBASE-578 Upgrade branch to 0.16.3 hadoop.
Modified:
hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HMaster.java
URL:
http://svn.apache.org/viewvc/hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HMaster.java?rev=653642&r1=653641&r2=653642&view=diff
==============================================================================
--- hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HMaster.java
(original)
+++ hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HMaster.java Mon
May 5 16:56:31 2008
@@ -430,10 +430,10 @@
throws IOException {
// Skip region - if ...
if (info.isOffline() // offline
- || killedRegions.contains(info.getRegionName()) // queued for
offline
- || regionsToDelete.contains(info.getRegionName())) { // queued for
delete
+ || killedRegions.contains(info.getRegionName())) { // queued for
offline
unassignedRegions.remove(info);
+ pendingRegions.remove(info);
return;
}
HServerInfo storedInfo = null;
@@ -846,13 +846,6 @@
volatile Set<Text> killedRegions =
Collections.synchronizedSet(new HashSet<Text>());
- /**
- * 'regionsToDelete' contains regions that need to be deleted, but cannot be
- * until the region server closes it
- */
- volatile Set<Text> regionsToDelete =
- Collections.synchronizedSet(new HashSet<Text>());
-
/** Set of tables currently in creation. */
private volatile Set<Text> tableInCreation =
Collections.synchronizedSet(new HashSet<Text>());
@@ -1414,8 +1407,9 @@
} else if (info.isMetaTable()) {
onlineMetaRegions.remove(info.getStartKey());
}
-
- this.unassignedRegions.put(info, ZERO_L);
+ if (!killedRegions.remove(info.getRegionName())) {
+ this.unassignedRegions.put(info, ZERO_L);
+ }
}
}
}
@@ -1678,14 +1672,9 @@
unassignRootRegion();
} else {
boolean reassignRegion = !region.isOffline();
- boolean deleteRegion = false;
if (killedRegions.remove(region.getRegionName())) {
reassignRegion = false;
}
- if (regionsToDelete.remove(region.getRegionName())) {
- reassignRegion = false;
- deleteRegion = true;
- }
if (region.isMetaTable()) {
// Region is part of the meta table. Remove it from
onlineMetaRegions
onlineMetaRegions.remove(region.getStartKey());
@@ -1696,8 +1685,7 @@
// reassigned before the close is processed.
unassignedRegions.remove(region);
try {
- toDoQueue.put(new ProcessRegionClose(region, reassignRegion,
- deleteRegion));
+ toDoQueue.put(new ProcessRegionClose(region, reassignRegion));
} catch (InterruptedException e) {
throw new RuntimeException(
@@ -2008,13 +1996,11 @@
private boolean rootRescanned;
private class ToDoEntry {
- boolean deleteRegion;
boolean regionOffline;
Text row;
HRegionInfo info;
ToDoEntry(Text row, HRegionInfo info) {
- this.deleteRegion = false;
this.regionOffline = false;
this.row = row;
this.info = info;
@@ -2106,35 +2092,26 @@
ToDoEntry todo = new ToDoEntry(row, info);
toDoList.add(todo);
- if (killList.containsKey(deadServerName)) {
- HashMap<Text, HRegionInfo> regionsToKill =
- new HashMap<Text, HRegionInfo>();
- synchronized (killList) {
+ synchronized (killList) {
+ if (killList.containsKey(deadServerName)) {
+ HashMap<Text, HRegionInfo> regionsToKill =
+ new HashMap<Text, HRegionInfo>();
+
regionsToKill.putAll(killList.get(deadServerName));
- }
- if (regionsToKill.containsKey(info.getRegionName())) {
- regionsToKill.remove(info.getRegionName());
- killList.put(deadServerName, regionsToKill);
- unassignedRegions.remove(info);
- synchronized (regionsToDelete) {
- if (regionsToDelete.contains(info.getRegionName())) {
- // Delete this region
- regionsToDelete.remove(info.getRegionName());
- todo.deleteRegion = true;
- } else {
- // Mark region offline
- todo.regionOffline = true;
- }
+ if (regionsToKill.containsKey(info.getRegionName())) {
+ regionsToKill.remove(info.getRegionName());
+ killList.put(deadServerName, regionsToKill);
+ unassignedRegions.remove(info);
+ // Mark region offline
+ todo.regionOffline = true;
}
- }
-
- } else {
- // Get region reassigned
- regions.add(info);
+ } else {
+ // Get region reassigned
+ regions.add(info);
+ }
// If it was pending, remove.
- // Otherwise will obstruct its getting reassigned.
pendingRegions.remove(info.getRegionName());
}
}
@@ -2160,9 +2137,7 @@
// Update server in root/meta entries
for (ToDoEntry e: toDoList) {
- if (e.deleteRegion) {
- HRegion.removeRegionFromMETA(server, regionName, e.row);
- } else if (e.regionOffline) {
+ if (e.regionOffline) {
HRegion.offlineRegionInMETA(server, regionName, e.info);
}
}
@@ -2282,6 +2257,7 @@
Thread.currentThread().getName());
}
}
+ killList.remove(deadServerName);
deadServers.remove(deadServerName);
break;
@@ -2373,26 +2349,21 @@
*/
private class ProcessRegionClose extends ProcessRegionStatusChange {
private boolean reassignRegion;
- private boolean deleteRegion;
/**
* @param regionInfo
* @param reassignRegion
- * @param deleteRegion
*/
- public ProcessRegionClose(HRegionInfo regionInfo, boolean reassignRegion,
- boolean deleteRegion) {
-
+ public ProcessRegionClose(HRegionInfo regionInfo, boolean reassignRegion) {
super(regionInfo);
this.reassignRegion = reassignRegion;
- this.deleteRegion = deleteRegion;
}
/** [EMAIL PROTECTED] */
@Override
public String toString() {
return "ProcessRegionClose of " + this.regionInfo.getRegionName() +
- ", " + this.reassignRegion + ", " + this.deleteRegion;
+ ", " + this.reassignRegion;
}
@Override
@@ -2414,10 +2385,7 @@
}
try {
- if (deleteRegion) {
- HRegion.removeRegionFromMETA(getMetaServer(), metaRegionName,
- regionInfo.getRegionName());
- } else if (!this.reassignRegion) {
+ if (!this.reassignRegion) {
HRegion.offlineRegionInMETA(getMetaServer(), metaRegionName,
regionInfo);
}
@@ -2435,15 +2403,6 @@
LOG.info("reassign region: " + regionInfo.getRegionName());
unassignedRegions.put(regionInfo, ZERO_L);
-
- } else if (deleteRegion) {
- try {
- HRegion.deleteRegion(fs, rootdir, regionInfo);
- } catch (IOException e) {
- e = RemoteExceptionHandler.checkIOException(e);
- LOG.error("failed delete region " + regionInfo.getRegionName(), e);
- throw e;
- }
}
return true;
}
@@ -2775,8 +2734,8 @@
HRegionInfo info = getHRegionInfo(row, map);
if (info == null) {
emptyRows.add(row);
- throw new IOException(COL_REGIONINFO + " not found on " +
- row);
+ LOG.error(COL_REGIONINFO + " not found on " + row);
+ continue;
}
String serverName =
Writables.bytesToString(map.get(COL_SERVER));
long startCode =
Writables.bytesToLong(map.get(COL_STARTCODE));
@@ -2812,7 +2771,7 @@
}
if (!tableExists) {
- throw new IOException(tableName + " does not exist");
+ throw new TableNotFoundException(tableName + " does not
exist");
}
postProcessMeta(m, server);
@@ -2822,6 +2781,11 @@
} // synchronized(metaScannerLock)
} catch (IOException e) {
+ if (e instanceof TableNotFoundException ||
+ e instanceof TableNotDisabledException ||
+ e instanceof InvalidColumnNameException) {
+ throw e;
+ }
if (tries == numRetries - 1) {
// No retries left
checkFileSystem();
@@ -2894,8 +2858,8 @@
for (HRegionInfo i: unservedRegions) {
if (i.isOffline() && i.isSplit()) {
if (LOG.isDebugEnabled()) {
- LOG.debug("Skipping region " + i.toString() + " because it is " +
- "offline because it has been split");
+ LOG.debug("Skipping region " + i.toString() +
+ " because it is offline because it has been split");
}
continue;
}
@@ -2917,8 +2881,11 @@
}
if (online) { // Bring offline regions on-line
- if (!unassignedRegions.containsKey(i)) {
- unassignedRegions.put(i, ZERO_L);
+ killedRegions.remove(i.getRegionName());
+ synchronized (unassignedRegions) {
+ if (!unassignedRegions.containsKey(i)) {
+ unassignedRegions.put(i, ZERO_L);
+ }
}
} else { // Prevent region from getting
assigned.
@@ -2943,12 +2910,6 @@
HashMap<Text, HRegionInfo> localKillList =
new HashMap<Text, HRegionInfo>();
- synchronized (killList) {
- HashMap<Text, HRegionInfo> killedRegions = killList.get(serverName);
- if (killedRegions != null) {
- localKillList.putAll(killedRegions);
- }
- }
for (HRegionInfo i: e.getValue()) {
if (LOG.isDebugEnabled()) {
LOG.debug("adding region " + i.getRegionName() +
@@ -2956,12 +2917,18 @@
}
localKillList.put(i.getRegionName(), i);
}
- if (localKillList.size() > 0) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("inserted local kill list into kill list for server " +
- serverName);
+ synchronized (killList) {
+ HashMap<Text, HRegionInfo> killedRegions = killList.get(serverName);
+ if (killedRegions != null) {
+ localKillList.putAll(killedRegions);
+ }
+ if (localKillList.size() > 0) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("inserted local kill list into kill list for server " +
+ serverName);
+ }
+ killList.put(serverName, localKillList);
}
- killList.put(serverName, localKillList);
}
}
servedRegions.clear();
@@ -2976,33 +2943,44 @@
}
/**
- * Instantiated to delete a table
- * Note that it extends ChangeTableState, which takes care of disabling
- * the table.
+ * Instantiated to delete a table. Table must be disabled first
*/
- private class TableDelete extends ChangeTableState {
+ private class TableDelete extends TableOperation {
TableDelete(Text tableName) throws IOException {
- super(tableName, false);
+ super(tableName);
}
@Override
- protected void postProcessMeta(MetaRegion m, HRegionInterface server)
- throws IOException {
-
- // For regions that are being served, mark them for deletion
+ protected void processScanItem(
+ @SuppressWarnings("unused") String serverName,
+ @SuppressWarnings("unused") long startCode,
+ final HRegionInfo info) throws IOException {
- for (HashSet<HRegionInfo> s: servedRegions.values()) {
- for (HRegionInfo i: s) {
- regionsToDelete.add(i.getRegionName());
- }
+ if (isEnabled(info)) {
+ throw new TableNotDisabledException(tableName.toString());
}
+ }
- // Unserved regions we can delete now
-
+ @Override
+ protected void postProcessMeta(MetaRegion m, HRegionInterface server)
+ throws IOException {
for (HRegionInfo i: unservedRegions) {
+ // Update meta table
+
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("updating columns in row: " + i.getRegionName());
+ }
+
+ BatchUpdate b = new BatchUpdate(rand.nextLong());
+ long lockid = b.startUpdate(i.getRegionName());
+ updateRegionInfo(lockid, b, i);
+ server.batchUpdate(m.getRegionName(), b);
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("updated columns in row: " + i.getRegionName());
+ }
+
// Delete the region
-
try {
HRegion.deleteRegion(fs, rootdir, i);
@@ -3011,11 +2989,9 @@
RemoteExceptionHandler.checkIOException(e));
}
}
- super.postProcessMeta(m, server);
}
- @Override
- protected void updateRegionInfo(BatchUpdate b,
+ private void updateRegionInfo(long lockid, BatchUpdate b,
@SuppressWarnings("unused") HRegionInfo info) {
for (int i = 0; i < ALL_META_COLUMNS.length; i++) {
// Be sure to clean all cells
@@ -3131,9 +3107,8 @@
if (families.get(columnName) != null){
families.put(columnName, descriptor);
updateRegionInfo(server, m.getRegionName(), i);
- }
- else{ // otherwise, we have an error.
- throw new IOException("Column family '" + columnName +
+ } else{ // otherwise, we have an error.
+ throw new InvalidColumnNameException("Column family '" + columnName
+
"' doesn't exist, so cannot be modified.");
}
}
Modified: hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HStore.java
URL:
http://svn.apache.org/viewvc/hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HStore.java?rev=653642&r1=653641&r2=653642&view=diff
==============================================================================
--- hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HStore.java
(original)
+++ hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HStore.java Mon
May 5 16:56:31 2008
@@ -356,7 +356,8 @@
// if there are items in the tail map, there's either a direct match to
// the search key, or a range of values between the first candidate key
// and the ultimate search key (or the end of the cache)
- if (!tailMap.isEmpty() && tailMap.firstKey().getRow().compareTo(key) <=
0) {
+ if (!tailMap.isEmpty() &&
+ tailMap.firstKey().getRow().compareTo(key) <= 0) {
key_iterator = tailMap.keySet().iterator();
// keep looking at cells as long as they are no greater than the
@@ -519,8 +520,9 @@
* equal or older timestamp. If no keys, returns an empty List. Does not
* return null.
*/
- private List<HStoreKey> internalGetKeys(final SortedMap<HStoreKey, byte
[]> map,
- final HStoreKey origin, final int versions) {
+ private List<HStoreKey> internalGetKeys(
+ final SortedMap<HStoreKey, byte []> map, final HStoreKey origin,
+ final int versions) {
List<HStoreKey> result = new ArrayList<HStoreKey>();
SortedMap<HStoreKey, byte []> tailMap = map.tailMap(origin);
@@ -609,6 +611,8 @@
}
}
+ /** [EMAIL PROTECTED] */
+ @Override
public boolean next(HStoreKey key, SortedMap<Text, byte []> results)
throws IOException {
if (this.scannerClosed) {
@@ -632,6 +636,9 @@
key.setRow(this.currentRow);
key.setVersion(this.timestamp);
getFull(key, deletes, rowResults);
+ for (Text column: deletes.keySet()) {
+ rowResults.put(column, HLogEdit.deleteBytes.get());
+ }
for (Map.Entry<Text, byte[]> e: rowResults.entrySet()) {
Text column = e.getKey();
byte [] c = e.getValue();
@@ -651,7 +658,9 @@
}
return results.size() > 0;
}
- public void close() {
+
+ /** [EMAIL PROTECTED] */
+ public void close() {
if (!scannerClosed) {
scannerClosed = true;
}
@@ -2361,9 +2370,11 @@
// Values that correspond to those keys
private byte [][] vals;
+ @SuppressWarnings("hiding")
private MapFile.Reader[] readers;
// Used around replacement of Readers if they change while we're scanning.
+ @SuppressWarnings("hiding")
private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
StoreFileScanner(long timestamp, Text[] targetCols, Text firstRow)
@@ -2531,17 +2542,21 @@
this.ts = t;
}
+ /** @return the row */
public Text getRow() {
return this.row;
}
+ /** @return the timestamp */
public long getTimestamp() {
return this.ts;
}
}
// Implementation of ChangedReadersObserver
- public void updateReaders() throws IOException {
+
+ /** [EMAIL PROTECTED] */
+ public void updateReaders() throws IOException {
this.lock.writeLock().lock();
try {
// The keys are currently lined up at the next row to fetch. Pass in
@@ -2565,8 +2580,8 @@
*/
boolean findFirstRow(int i, Text firstRow) throws IOException {
ImmutableBytesWritable ibw = new ImmutableBytesWritable();
- HStoreKey firstKey
- = (HStoreKey)readers[i].getClosest(new HStoreKey(firstRow), ibw);
+ HStoreKey firstKey =
+ (HStoreKey)readers[i].getClosest(new HStoreKey(firstRow), ibw);
if (firstKey == null) {
// Didn't find it. Close the scanner and return TRUE
closeSubScanner(i);
Modified:
hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestHBaseCluster.java
URL:
http://svn.apache.org/viewvc/hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestHBaseCluster.java?rev=653642&r1=653641&r2=653642&view=diff
==============================================================================
---
hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestHBaseCluster.java
(original)
+++
hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestHBaseCluster.java
Mon May 5 16:56:31 2008
@@ -65,7 +65,6 @@
basic();
scanner();
listTables();
- cleanup();
}
private static final int FIRST_ROW = 1;
@@ -199,11 +198,4 @@
assertTrue(families.contains(new Text(CONTENTS)));
assertTrue(families.contains(new Text(ANCHOR)));
}
-
- private void cleanup() throws IOException {
-
- // Delete the table we created
-
- admin.deleteTable(desc.getName());
- }
}
