You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2010/08/23 03:40:53 UTC
svn commit: r987974 - in /hbase/branches/0.90_master_rewrite: ./
src/main/java/org/apache/hadoop/hbase/client/
src/main/java/org/apache/hadoop/hbase/executor/
src/main/java/org/apache/hadoop/hbase/master/
src/main/java/org/apache/hadoop/hbase/master/ha...
Author: stack
Date: Mon Aug 23 01:40:52 2010
New Revision: 987974
URL: http://svn.apache.org/viewvc?rev=987974&view=rev
Log:
Making table schema modification, add/delete work.
M BRANCH_TODO.txt
Added notes on sync vs async.
M src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java
Added more tests for add/delete column and table.
M src/main/java/org/apache/hadoop/hbase/master/MasterServices.java
Added utility method that implementors can just noop.
M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
execute removed. Call process in stead.
M src/main/java/org/apache/hadoop/hbase/master/HMaster.java
Removed redundant table checks. These checks have been moved into
handler constructor.
(checkTableModifiable) Added
M src/main/java/org/apache/hadoop/hbase/master/handler/TableModifyFamilyHandler.java
M src/main/java/org/apache/hadoop/hbase/master/handler/TableAddFamilyHandler.java
Renaming variables and allow throwing exception on construction
M src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java
M src/main/java/org/apache/hadoop/hbase/master/handler/TableDeleteFamilyHandler.java
M src/main/java/org/apache/hadoop/hbase/master/handler/ModifyTableHandler.java
Move table checks into constructor rather than in processor.
M src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java
Constructor throws IOE
M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java
Updated javadoc. Moved all to do with Executor out of here and into
that class instead.
M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
Javadoc and moved executor types here from EventHandler
M src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
Make the table schema mod methods all work the same.
Modified:
hbase/branches/0.90_master_rewrite/BRANCH_TODO.txt
hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java
hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java
hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java
hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/ModifyTableHandler.java
hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableAddFamilyHandler.java
hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableDeleteFamilyHandler.java
hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java
hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableModifyFamilyHandler.java
hbase/branches/0.90_master_rewrite/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java
Modified: hbase/branches/0.90_master_rewrite/BRANCH_TODO.txt
URL: http://svn.apache.org/viewvc/hbase/branches/0.90_master_rewrite/BRANCH_TODO.txt?rev=987974&r1=987973&r2=987974&view=diff
==============================================================================
--- hbase/branches/0.90_master_rewrite/BRANCH_TODO.txt (original)
+++ hbase/branches/0.90_master_rewrite/BRANCH_TODO.txt Mon Aug 23 01:40:52 2010
@@ -8,8 +8,15 @@ remaining tasks before merge
* finish baseline implementation of new splits
* integrate load balancer
+- Looksee if we are still deleting location from meta; not needed any
+more and if we don't delete, then we can put region back on the server
+that used to be serving it; can add old location to new RegionPlan
+-- St.Ack 08/21
* ensure root/meta are last to close on cluster shutdown
+- Add asking RS what it has when only two servers remaining...
+and when only root or meta, then send explicit close of each.
+Do it this way to ensure correct shutdown order -- St.Ack 08/21
---
@@ -120,6 +127,20 @@ somewhat easier stuff
St.Ack
-- Ensure root and meta are last to close on cluster shutdown; it shoudl be the case but verify.
+From Master:
+
+ // TODO: Sync or async on this stuff? execute means sync. submit means later.
+ // Right now this will swallow exceptions either way, might need
+ // process() which throws nothing but execute() which throws IOE so
+ // synchronous stuff can throw exceptions?
+
+At the moment we have a mix of sync and async. Whats missing is a callback mechanism
+(Benoit's Twisted Deferred would do nicely here). I change EventHandler to
+remove the execute so synchronous call process directly. Also made it so
+process now throws an exception Also, changed handlers so they do checks
+in constructor and constructor throws IOE. This way, we fail fast so even if
+asynchronous operation, its possible we'll see the TableNotDisabledException.
+There is more to do in here but should be good enough for merge.
================================================================================
@@ -254,4 +275,4 @@ Later:
(if finish, need to use files not directory, and use right location)
- Put notes from reviewboard/jira into LB javadoc or hbase "book"
-
\ No newline at end of file
+
Modified: hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java?rev=987974&r1=987973&r2=987974&view=diff
==============================================================================
--- hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java (original)
+++ hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java Mon Aug 23 01:40:52 2010
@@ -594,7 +594,6 @@ public class HBaseAdmin {
*/
public void deleteColumn(final byte [] tableName, final byte [] columnName)
throws IOException {
- HTableDescriptor.isLegalTableName(tableName);
try {
getMaster().deleteColumn(tableName, columnName);
} catch (RemoteException e) {
@@ -657,9 +656,14 @@ public class HBaseAdmin {
*/
public void modifyColumn(final byte [] tableName, HColumnDescriptor descriptor)
throws IOException {
- HTableDescriptor.isLegalTableName(tableName);
- if (isTableEnabled(tableName)) throw new TableNotDisabledException(tableName);
- getMaster().modifyColumn(tableName, descriptor);
+ try {
+ getMaster().modifyColumn(tableName, descriptor);
+ } catch (RemoteException re) {
+ // Convert RE exceptions in here; client shouldn't have to deal with them,
+ // at least w/ the type of exceptions that come out of this method:
+ // TableNotFoundException, etc.
+ throw RemoteExceptionHandler.decodeRemoteException(re);
+ }
}
/**
@@ -836,9 +840,14 @@ public class HBaseAdmin {
*/
public void modifyTable(final byte [] tableName, HTableDescriptor htd)
throws IOException {
- HTableDescriptor.isLegalTableName(tableName);
- if (isTableEnabled(tableName)) throw new TableNotDisabledException(tableName);
- getMaster().modifyTable(tableName, htd);
+ try {
+ getMaster().modifyTable(tableName, htd);
+ } catch (RemoteException re) {
+ // Convert RE exceptions in here; client shouldn't have to deal with them,
+ // at least w/ the type of exceptions that come out of this method:
+ // TableNotFoundException, etc.
+ throw RemoteExceptionHandler.decodeRemoteException(re);
+ }
}
Modified: hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java?rev=987974&r1=987973&r2=987974&view=diff
==============================================================================
--- hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java (original)
+++ hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java Mon Aug 23 01:40:52 2010
@@ -19,62 +19,83 @@
*/
package org.apache.hadoop.hbase.executor;
+import java.io.IOException;
import java.util.concurrent.atomic.AtomicLong;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.hadoop.hbase.Server;
-import org.apache.hadoop.hbase.executor.ExecutorService.ExecutorType;
/**
* Abstract base class for all HBase event handlers. Subclasses should
- * implement the process() method where the actual handling of the event
- * happens.
+ * implement the {@link #process()} method. Subclasses should also do all
+ * necessary checks up in their constructor if possible -- check table exists,
+ * is disabled, etc. -- so they fail fast rather than later when process is
+ * running. Do it this way because process be invoked directly but event
+ * handlers are also
+ * run in an executor context -- i.e. asynchronously -- and in this case,
+ * exceptions thrown at process time will not be seen by the invoker, not till
+ * we implement a call-back mechanism so the client can pick them up later.
* <p>
- * EventType is a list of ALL events (which also corresponds to messages -
- * either internal to one component or between components). The event type
- * names specify the component from which the event originated, and the
- * component which is supposed to handle it.
+ * Event handlers have an {@link EventType}.
+ * {@link EventType} is a list of ALL handler event types. We need to keep
+ * a full list in one place -- and as enums is a good shorthand for an
+ * implemenations -- because event handlers can be passed to executors when
+ * they are to be run asynchronously. The
+ * hbase executor, see {@link ExecutorService}, has a switch for passing
+ * event type to executor.
* <p>
- * Listeners can listen to all the events by implementing the interface
- * EventHandlerListener, and by registering themselves as a listener. They
- * will be called back before and after the process of every event.
+ * Event listeners can be installed and will be called pre- and post- process if
+ * this EventHandler is run in a Thread (its a Runnable so if its {@link #run()}
+ * method gets called). Implement
+ * {@link EventHandlerListener}s, and registering using
+ * {@link #setListener(EventHandlerListener)}.
+ * @see {@link ExecutorService}
*/
public abstract class EventHandler implements Runnable, Comparable<Runnable> {
private static final Log LOG = LogFactory.getLog(EventHandler.class);
+
// type of event this object represents
protected EventType eventType;
- // server controller
+
protected Server server;
// sequence id generator for default FIFO ordering of events
protected static AtomicLong seqids = new AtomicLong(0);
+
// sequence id for this event
- protected long seqid;
+ private final long seqid;
- // Listener to call pre- and post- processing.
+ // Listener to call pre- and post- processing. May be null.
private EventHandlerListener listener;
/**
- * This interface provides hooks to listen to various events received by the
- * queue. A class implementing this can listen to the updates by calling
- * registerListener and stop receiving updates by calling unregisterListener
+ * This interface provides pre- and post-process hooks for events.
*/
public interface EventHandlerListener {
/**
* Called before any event is processed
+ * @param The event handler whose process method is about to be called.
*/
public void beforeProcess(EventHandler event);
/**
* Called after any event is processed
+ * @param The event handler whose process method is about to be called.
*/
public void afterProcess(EventHandler event);
}
/**
- * These are a list of HBase events that can be handled by the various
- * HBaseExecutorService's. All the events are serialized as byte values.
+ * List of all HBase event handler types. Event types are named by a
+ * convention: event type names specify the component from which the event
+ * originated and then where its destined -- e.g. RS2ZK_ prefix means the
+ * event came from a regionserver destined for zookeeper -- and then what
+ * the even is; e.g. REGION_OPENING.
+ *
+ * <p>We give the enums indices so we can add types later and keep them
+ * grouped together rather than have to add them always to the end as we
+ * would have to if we used raw enum ordinals.
*/
public enum EventType {
// Messages originating from RS (NOTE: there is NO direct communication from
@@ -109,71 +130,9 @@ public abstract class EventHandler imple
M_SERVER_SHUTDOWN (70); // Master is processing shutdown of a RS
/**
- * Returns the executor service type (the thread pool instance) for this
- * event type. Every type must be handled here. Multiple types map to
- * Called by the HMaster. Returns a name of the executor service given an
- * event type. Every event type has an entry - if the event should not be
- * handled just add the NONE executor.
- * @return name of the executor service
+ * Constructor
*/
- public ExecutorType getExecutorServiceType() {
- switch(this) {
-
- // Master executor services
-
- case RS2ZK_REGION_CLOSED:
- return ExecutorType.MASTER_CLOSE_REGION;
-
- case RS2ZK_REGION_OPENED:
- return ExecutorType.MASTER_OPEN_REGION;
-
- case M_SERVER_SHUTDOWN:
- return ExecutorType.MASTER_SERVER_OPERATIONS;
-
- case C2M_DELETE_TABLE:
- case C2M_DISABLE_TABLE:
- case C2M_ENABLE_TABLE:
- case C2M_MODIFY_TABLE:
- return ExecutorType.MASTER_TABLE_OPERATIONS;
-
- // RegionServer executor services
-
- case M2RS_OPEN_REGION:
- return ExecutorType.RS_OPEN_REGION;
-
- case M2RS_OPEN_ROOT:
- return ExecutorType.RS_OPEN_ROOT;
-
- case M2RS_OPEN_META:
- return ExecutorType.RS_OPEN_META;
-
- case M2RS_CLOSE_REGION:
- return ExecutorType.RS_CLOSE_REGION;
-
- case M2RS_CLOSE_ROOT:
- return ExecutorType.RS_CLOSE_ROOT;
-
- case M2RS_CLOSE_META:
- return ExecutorType.RS_CLOSE_META;
-
- default:
- throw new RuntimeException("Unhandled event type " + this.name());
- }
- }
-
EventType(int value) {}
-
- @Override
- public String toString() {
- switch(this) {
- case RS2ZK_REGION_CLOSED: return "CLOSED";
- case RS2ZK_REGION_CLOSING: return "CLOSING";
- case RS2ZK_REGION_OPENED: return "OPENED";
- case RS2ZK_REGION_OPENING: return "OPENING";
- case M2ZK_REGION_OFFLINE: return "OFFLINE";
- default: return this.name();
- }
- }
}
/**
@@ -185,40 +144,29 @@ public abstract class EventHandler imple
seqid = seqids.incrementAndGet();
}
- /**
- * This is a wrapper around {@link #process()} to give listeners a chance to run.
- */
public void run() {
- if (getListener() != null) this.listener.beforeProcess(this);
- // call the main process function
try {
+ if (getListener() != null) getListener().beforeProcess(this);
process();
+ if (getListener() != null) getListener().afterProcess(this);
} catch(Throwable t) {
LOG.error("Caught throwable while processing event " + eventType, t);
}
- if (getListener() != null) this.listener.afterProcess(this);
}
/**
* This method is the main processing loop to be implemented by the various
* subclasses.
+ * @throws IOException
*/
- public abstract void process();
-
- /**
- * Return the name for this event type.
- * @return
- */
- public ExecutorType getExecutorType() {
- return eventType.getExecutorServiceType();
- }
+ public abstract void process() throws IOException;
/**
* Return the event type
* @return
*/
public EventType getEventType() {
- return eventType;
+ return this.eventType;
}
/**
@@ -238,6 +186,13 @@ public abstract class EventHandler imple
}
/**
+ * @return This events' sequence id.
+ */
+ public long getSeqid() {
+ return this.seqid;
+ }
+
+ /**
* Default prioritized runnable comparator which implements a FIFO ordering.
* <p>
* Subclasses should not override this. Instead, if they want to implement
@@ -253,14 +208,6 @@ public abstract class EventHandler imple
}
/**
- * Executes this event object in the caller's thread. This is a synchronous
- * way of executing the event.
- */
- public void execute() {
- this.run();
- }
-
- /**
* @return Current listener or null if none set.
*/
public synchronized EventHandlerListener getListener() {
Modified: hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java?rev=987974&r1=987973&r2=987974&view=diff
==============================================================================
--- hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java (original)
+++ hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java Mon Aug 23 01:40:52 2010
@@ -45,9 +45,8 @@ import com.google.common.util.concurrent
* then do: <code>instance.startExecutorService("myService");</code>. When done
* call {@link #shutdown()}.
*
- * In order to use the service created above, you need to override the
- * {@link EventHandler} class and create an {@link EventHandler.EventType} that
- * submits to this service. Register pre- and post- processing listeners
+ * <p>In order to use the service created above, call
+ * {@link #submit(EventHandler)}. Register pre- and post- processing listeners
* by registering your implementation of {@link EventHandler.EventHandlerListener}
* with {@link #registerListener(EventType, EventHandlerListener)}. Be sure
* to deregister your listener when done via {@link #unregisterListener(EventType)}.
@@ -58,15 +57,17 @@ public class ExecutorService {
// hold the all the executors created in a map addressable by their names
private final ConcurrentHashMap<String, Executor> executorMap =
new ConcurrentHashMap<String, Executor>();
+
// listeners that are called before and after an event is processed
private ConcurrentHashMap<EventHandler.EventType, EventHandlerListener> eventHandlerListeners =
new ConcurrentHashMap<EventHandler.EventType, EventHandlerListener>();
+ // Name of the server hosting this executor service.
private final String servername;
/**
- * The following is a list of names for the various executor services in both
- * the master and the region server.
+ * The following is a list of all executor types, both those that run in the
+ * master and those that run in the regionserver.
*/
public enum ExecutorType {
@@ -87,12 +88,65 @@ public class ExecutorService {
ExecutorType(int value) {}
+ /**
+ * @param serverName
+ * @return Conflation of the executor type and the passed servername.
+ */
String getExecutorName(String serverName) {
return this.toString() + "-" + serverName;
}
}
/**
+ * Returns the executor service type (the thread pool instance) for the
+ * passed event handler type.
+ * @param type EventHandler type.
+ */
+ public ExecutorType getExecutorServiceType(final EventHandler.EventType type) {
+ switch(type) {
+ // Master executor services
+
+ case RS2ZK_REGION_CLOSED:
+ return ExecutorType.MASTER_CLOSE_REGION;
+
+ case RS2ZK_REGION_OPENED:
+ return ExecutorType.MASTER_OPEN_REGION;
+
+ case M_SERVER_SHUTDOWN:
+ return ExecutorType.MASTER_SERVER_OPERATIONS;
+
+ case C2M_DELETE_TABLE:
+ case C2M_DISABLE_TABLE:
+ case C2M_ENABLE_TABLE:
+ case C2M_MODIFY_TABLE:
+ return ExecutorType.MASTER_TABLE_OPERATIONS;
+
+ // RegionServer executor services
+
+ case M2RS_OPEN_REGION:
+ return ExecutorType.RS_OPEN_REGION;
+
+ case M2RS_OPEN_ROOT:
+ return ExecutorType.RS_OPEN_ROOT;
+
+ case M2RS_OPEN_META:
+ return ExecutorType.RS_OPEN_META;
+
+ case M2RS_CLOSE_REGION:
+ return ExecutorType.RS_CLOSE_REGION;
+
+ case M2RS_CLOSE_ROOT:
+ return ExecutorType.RS_CLOSE_ROOT;
+
+ case M2RS_CLOSE_META:
+ return ExecutorType.RS_CLOSE_META;
+
+ default:
+ throw new RuntimeException("Unhandled event type " + type);
+ }
+ }
+
+ /**
* Default constructor.
* @param Name of the hosting server.
*/
@@ -158,7 +212,7 @@ public class ExecutorService {
}
public void submit(final EventHandler eh) {
- getExecutor(eh.getExecutorType()).submit(eh);
+ getExecutor(getExecutorServiceType(eh.getEventType())).submit(eh);
}
/**
Modified: hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java?rev=987974&r1=987973&r2=987974&view=diff
==============================================================================
--- hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java (original)
+++ hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Mon Aug 23 01:40:52 2010
@@ -197,7 +197,7 @@ public class AssignmentManager extends Z
regionsInTransition.put(encodedName,
new RegionState(regionInfo, RegionState.State.CLOSED,
data.getStamp()));
- new ClosedRegionHandler(master, this, data, regionInfo).execute();
+ new ClosedRegionHandler(master, this, data, regionInfo).process();
break;
case RS2ZK_REGION_OPENING:
@@ -214,7 +214,7 @@ public class AssignmentManager extends Z
new RegionState(regionInfo, RegionState.State.OPENING,
data.getStamp()));
new OpenedRegionHandler(master, this, data, regionInfo,
- serverManager.getServerInfo(data.getServerName())).execute();
+ serverManager.getServerInfo(data.getServerName())).process();
break;
}
}
@@ -239,8 +239,7 @@ public class AssignmentManager extends Z
}
String encodedName = HRegionInfo.encodeRegionName(data.getRegionName());
String prettyPrintedRegionName = HRegionInfo.prettyPrint(encodedName);
- LOG.debug("Handling transition=" + data.getEventType().name() +
- "/" + data.getEventType().toString() +
+ LOG.debug("Handling transition=" + data.getEventType() +
", server=" + data.getServerName() + ", region=" + prettyPrintedRegionName);
RegionState regionState = regionsInTransition.get(encodedName);
switch(data.getEventType()) {
Modified: hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/HMaster.java?rev=987974&r1=987973&r2=987974&view=diff
==============================================================================
--- hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/HMaster.java (original)
+++ hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/HMaster.java Mon Aug 23 01:40:52 2010
@@ -46,6 +46,8 @@ import org.apache.hadoop.hbase.NotAllMet
import org.apache.hadoop.hbase.RemoteExceptionHandler;
import org.apache.hadoop.hbase.Server;
import org.apache.hadoop.hbase.TableExistsException;
+import org.apache.hadoop.hbase.TableNotDisabledException;
+import org.apache.hadoop.hbase.TableNotFoundException;
import org.apache.hadoop.hbase.ZooKeeperConnectionException;
import org.apache.hadoop.hbase.catalog.CatalogTracker;
import org.apache.hadoop.hbase.catalog.MetaEditor;
@@ -579,64 +581,38 @@ implements HMasterInterface, HMasterRegi
}
}
- private boolean isCatalogTable(final byte [] tableName) {
+ private static boolean isCatalogTable(final byte [] tableName) {
return Bytes.equals(tableName, HConstants.ROOT_TABLE_NAME) ||
Bytes.equals(tableName, HConstants.META_TABLE_NAME);
}
- // TODO: Sync or async on this stuff?
- // Right now this will swallow exceptions either way, might need
- // process() which throws nothing but execute() which throws IOE so
- // synchronous stuff can throw exceptions?
-
public void deleteTable(final byte [] tableName) throws IOException {
- if (isCatalogTable(tableName)) {
- throw new IOException("Can't delete catalog tables");
- }
- //
- new DeleteTableHandler(tableName, this, this)
- .execute();
- LOG.info("deleted table: " + Bytes.toString(tableName));
+ new DeleteTableHandler(tableName, this, this).process();
}
public void addColumn(byte [] tableName, HColumnDescriptor column)
throws IOException {
- if (isCatalogTable(tableName)) {
- throw new IOException("Can't modify catalog tables");
- }
- new TableAddFamilyHandler(tableName, column, this, this).execute();
+ new TableAddFamilyHandler(tableName, column, this, this).process();
}
public void modifyColumn(byte [] tableName, HColumnDescriptor descriptor)
throws IOException {
- if (isCatalogTable(tableName)) {
- throw new IOException("Can't modify catalog tables");
- }
- new TableModifyFamilyHandler(tableName, descriptor, this, this).execute();
+ new TableModifyFamilyHandler(tableName, descriptor, this, this).process();
}
public void deleteColumn(final byte [] tableName, final byte [] c)
throws IOException {
- if (isCatalogTable(tableName)) {
- throw new IOException("Can't modify catalog tables");
- }
- new TableDeleteFamilyHandler(tableName, c, this, this).execute();
+ new TableDeleteFamilyHandler(tableName, c, this, this).process();
}
public void enableTable(final byte [] tableName) throws IOException {
- if (isCatalogTable(tableName)) {
- throw new IOException("Can't enable catalog tables");
- }
new EnableTableHandler(this, tableName, catalogTracker, assignmentManager)
- .execute();
+ .process();
}
public void disableTable(final byte [] tableName) throws IOException {
- if (isCatalogTable(tableName)) {
- throw new IOException("Can't disable catalog tables");
- }
new DisableTableHandler(this, tableName, catalogTracker, assignmentManager)
- .execute();
+ .process();
}
/**
@@ -682,6 +658,21 @@ implements HMasterInterface, HMasterRegi
this.executorService.submit(new ModifyTableHandler(tableName, htd, this, this));
}
+ @Override
+ public void checkTableModifiable(final byte [] tableName)
+ throws IOException {
+ String tableNameStr = Bytes.toString(tableName);
+ if (isCatalogTable(tableName)) {
+ throw new IOException("Can't modify catalog tables");
+ }
+ if (!MetaReader.tableExists(getCatalogTracker(), tableNameStr)) {
+ throw new TableNotFoundException(tableNameStr);
+ }
+ if (!getAssignmentManager().isTableDisabled(Bytes.toString(tableName))) {
+ throw new TableNotDisabledException(tableName);
+ }
+ }
+
/**
* @return cluster status
*/
Modified: hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java?rev=987974&r1=987973&r2=987974&view=diff
==============================================================================
--- hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java (original)
+++ hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java Mon Aug 23 01:40:52 2010
@@ -19,6 +19,8 @@
*/
package org.apache.hadoop.hbase.master;
+import java.io.IOException;
+
import org.apache.hadoop.hbase.catalog.CatalogTracker;
import org.apache.hadoop.hbase.executor.ExecutorService;
@@ -50,4 +52,12 @@ public interface MasterServices {
* @return Master's instance of {@link ExecutorService}
*/
public ExecutorService getExecutorService();
+
+ /**
+ * Check table is modifiable; i.e. exists and is offline.
+ * @param tableName Name of table to check.
+ * @throws TableNotDisabledException
+ * @throws TableNotFoundException
+ */
+ public void checkTableModifiable(final byte [] tableName) throws IOException;
}
Modified: hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java?rev=987974&r1=987973&r2=987974&view=diff
==============================================================================
--- hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java (original)
+++ hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java Mon Aug 23 01:40:52 2010
@@ -33,7 +33,7 @@ public class DeleteTableHandler extends
private static final Log LOG = LogFactory.getLog(DeleteTableHandler.class);
public DeleteTableHandler(byte [] tableName, Server server,
- final MasterServices masterServices) {
+ final MasterServices masterServices) throws IOException {
super(EventType.C2M_DELETE_TABLE, tableName, server, masterServices);
}
Modified: hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/ModifyTableHandler.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/ModifyTableHandler.java?rev=987974&r1=987973&r2=987974&view=diff
==============================================================================
--- hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/ModifyTableHandler.java (original)
+++ hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/ModifyTableHandler.java Mon Aug 23 01:40:52 2010
@@ -33,20 +33,20 @@ public class ModifyTableHandler extends
public ModifyTableHandler(final byte [] tableName,
final HTableDescriptor htd, final Server server,
- final MasterServices masterServices) {
+ final MasterServices masterServices) throws IOException {
super(EventType.C2M_MODIFY_TABLE, tableName, server, masterServices);
this.htd = htd;
}
@Override
- protected void handleTableOperation(List<HRegionInfo> regions)
+ protected void handleTableOperation(List<HRegionInfo> hris)
throws IOException {
- for (HRegionInfo region : regions) {
+ for (HRegionInfo hri : hris) {
// Update region info in META
- region.setTableDesc(this.htd);
- MetaEditor.updateRegionInfo(this.masterServices.getCatalogTracker(), region);
+ hri.setTableDesc(this.htd);
+ MetaEditor.updateRegionInfo(this.masterServices.getCatalogTracker(), hri);
// Update region info in FS
- this.masterServices.getMasterFileSystem().updateRegionInfo(region);
+ this.masterServices.getMasterFileSystem().updateRegionInfo(hri);
}
}
}
\ No newline at end of file
Modified: hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableAddFamilyHandler.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableAddFamilyHandler.java?rev=987974&r1=987973&r2=987974&view=diff
==============================================================================
--- hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableAddFamilyHandler.java (original)
+++ hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableAddFamilyHandler.java Mon Aug 23 01:40:52 2010
@@ -39,28 +39,28 @@ public class TableAddFamilyHandler exten
private final HColumnDescriptor familyDesc;
public TableAddFamilyHandler(byte[] tableName, HColumnDescriptor familyDesc,
- Server server, final MasterServices masterServices) {
+ Server server, final MasterServices masterServices) throws IOException {
super(EventType.C2M_ADD_FAMILY, tableName, server, masterServices);
this.familyDesc = familyDesc;
}
@Override
- protected void handleTableOperation(List<HRegionInfo> regions)
+ protected void handleTableOperation(List<HRegionInfo> hris)
throws IOException {
- HTableDescriptor htd = regions.get(0).getTableDesc();
+ HTableDescriptor htd = hris.get(0).getTableDesc();
byte [] familyName = familyDesc.getName();
if(htd.hasFamily(familyName)) {
throw new InvalidFamilyOperationException(
"Family '" + Bytes.toString(familyName) + "' already exists so " +
"cannot be added");
}
- for(HRegionInfo region : regions) {
+ for(HRegionInfo hri : hris) {
// Update the HTD
- region.getTableDesc().addFamily(familyDesc);
+ hri.getTableDesc().addFamily(familyDesc);
// Update region in META
- MetaEditor.updateRegionInfo(this.masterServices.getCatalogTracker(), region);
+ MetaEditor.updateRegionInfo(this.masterServices.getCatalogTracker(), hri);
// Update region info in FS
- this.masterServices.getMasterFileSystem().updateRegionInfo(region);
+ this.masterServices.getMasterFileSystem().updateRegionInfo(hri);
}
}
}
\ No newline at end of file
Modified: hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableDeleteFamilyHandler.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableDeleteFamilyHandler.java?rev=987974&r1=987973&r2=987974&view=diff
==============================================================================
--- hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableDeleteFamilyHandler.java (original)
+++ hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableDeleteFamilyHandler.java Mon Aug 23 01:40:52 2010
@@ -39,29 +39,31 @@ public class TableDeleteFamilyHandler ex
private final byte [] familyName;
public TableDeleteFamilyHandler(byte[] tableName, byte [] familyName,
- Server server, final MasterServices masterServices) {
+ Server server, final MasterServices masterServices) throws IOException {
super(EventType.C2M_ADD_FAMILY, tableName, server, masterServices);
this.familyName = familyName;
}
@Override
- protected void handleTableOperation(List<HRegionInfo> regions) throws IOException {
- HTableDescriptor htd = regions.get(0).getTableDesc();
+ protected void handleTableOperation(List<HRegionInfo> hris) throws IOException {
+ HTableDescriptor htd = hris.get(0).getTableDesc();
if(!htd.hasFamily(familyName)) {
throw new InvalidFamilyOperationException(
"Family '" + Bytes.toString(familyName) + "' does not exist so " +
"cannot be deleted");
}
- for(HRegionInfo region : regions) {
+ for(HRegionInfo hri : hris) {
// Update the HTD
- region.getTableDesc().removeFamily(familyName);
+ hri.getTableDesc().removeFamily(familyName);
// Update region in META
- MetaEditor.updateRegionInfo(this.masterServices.getCatalogTracker(), region);
+ MetaEditor.updateRegionInfo(this.masterServices.getCatalogTracker(), hri);
MasterFileSystem mfs = this.masterServices.getMasterFileSystem();
// Update region info in FS
- mfs.updateRegionInfo(region);
+ mfs.updateRegionInfo(hri);
// Delete directory in FS
- mfs.deleteFamily(region, familyName);
+ mfs.deleteFamily(hri, familyName);
+ // Update region info in FS
+ this.masterServices.getMasterFileSystem().updateRegionInfo(hri);
}
}
-}
+}
\ No newline at end of file
Modified: hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java?rev=987974&r1=987973&r2=987974&view=diff
==============================================================================
--- hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java (original)
+++ hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java Mon Aug 23 01:40:52 2010
@@ -26,8 +26,6 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.hadoop.hbase.HRegionInfo;
import org.apache.hadoop.hbase.Server;
-import org.apache.hadoop.hbase.TableNotDisabledException;
-import org.apache.hadoop.hbase.TableNotFoundException;
import org.apache.hadoop.hbase.catalog.MetaReader;
import org.apache.hadoop.hbase.executor.EventHandler;
import org.apache.hadoop.hbase.master.MasterServices;
@@ -35,23 +33,23 @@ import org.apache.hadoop.hbase.util.Byte
/**
* Base class for performing operations against tables.
- * <p>
- * Ensures all regions of the table are offline and then executes
- * {@link #handleTableOperation(List)} with a list of regions of the
- * table.
+ * Checks on whether the process can go forward are done in constructor rather
+ * than later on in {@link #process()}. The idea is to fail fast rather than
+ * later down in an async invocation of {@link #process()} (which currently has
+ * no means of reporting back issues once started).
*/
public abstract class TableEventHandler extends EventHandler {
private static final Log LOG = LogFactory.getLog(TableEventHandler.class);
protected final MasterServices masterServices;
protected final byte [] tableName;
- protected final String tableNameStr;
public TableEventHandler(EventType eventType, byte [] tableName, Server server,
- MasterServices masterServices) {
+ MasterServices masterServices)
+ throws IOException {
super(server, eventType);
this.masterServices = masterServices;
this.tableName = tableName;
- this.tableNameStr = Bytes.toString(this.tableName);
+ this.masterServices.checkTableModifiable(tableName);
}
@Override
@@ -59,30 +57,16 @@ public abstract class TableEventHandler
try {
LOG.info("Handling table operation " + eventType + " on table " +
Bytes.toString(tableName));
- handleTableOperation(tableChecks());
+ List<HRegionInfo> hris =
+ MetaReader.getTableRegions(this.masterServices.getCatalogTracker(),
+ tableName);
+ handleTableOperation(hris);
} catch (IOException e) {
LOG.error("Error trying to delete the table " + Bytes.toString(tableName),
e);
}
}
- private List<HRegionInfo> tableChecks() throws IOException {
- // Check if table exists
- if (!MetaReader.tableExists(this.masterServices.getCatalogTracker(),
- this.tableNameStr)) {
- throw new TableNotFoundException(Bytes.toString(tableName));
- }
- // Verify table is offline
- if (!this.masterServices.getAssignmentManager().
- isTableDisabled(this.tableNameStr)) {
- throw new TableNotDisabledException(tableName);
- }
- // Get the regions of this table
- // TODO: Use in-memory state of master?
- return MetaReader.getTableRegions(this.masterServices.getCatalogTracker(),
- tableName);
- }
-
protected abstract void handleTableOperation(List<HRegionInfo> regions)
throws IOException;
}
\ No newline at end of file
Modified: hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableModifyFamilyHandler.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableModifyFamilyHandler.java?rev=987974&r1=987973&r2=987974&view=diff
==============================================================================
--- hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableModifyFamilyHandler.java (original)
+++ hbase/branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/handler/TableModifyFamilyHandler.java Mon Aug 23 01:40:52 2010
@@ -40,7 +40,7 @@ public class TableModifyFamilyHandler ex
public TableModifyFamilyHandler(byte[] tableName,
HColumnDescriptor familyDesc, Server server,
- final MasterServices masterServices) {
+ final MasterServices masterServices) throws IOException {
super(EventType.C2M_MODIFY_FAMILY, tableName, server, masterServices);
this.familyDesc = familyDesc;
}
@@ -53,13 +53,13 @@ public class TableModifyFamilyHandler ex
throw new InvalidFamilyOperationException("Family '" +
Bytes.toString(familyName) + "' doesn't exists so cannot be modified");
}
- for(HRegionInfo region : regions) {
+ for(HRegionInfo hri : regions) {
// Update the HTD
- region.getTableDesc().addFamily(familyDesc);
+ hri.getTableDesc().addFamily(familyDesc);
// Update region in META
- MetaEditor.updateRegionInfo(this.masterServices.getCatalogTracker(), region);
+ MetaEditor.updateRegionInfo(this.masterServices.getCatalogTracker(), hri);
// Update region info in FS
- this.masterServices.getMasterFileSystem().updateRegionInfo(region);
+ this.masterServices.getMasterFileSystem().updateRegionInfo(hri);
}
}
}
\ No newline at end of file
Modified: hbase/branches/0.90_master_rewrite/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90_master_rewrite/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java?rev=987974&r1=987973&r2=987974&view=diff
==============================================================================
--- hbase/branches/0.90_master_rewrite/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java (original)
+++ hbase/branches/0.90_master_rewrite/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java Mon Aug 23 01:40:52 2010
@@ -116,7 +116,7 @@ public class TestAdmin {
boolean expectedException = false;
try {
this.admin.modifyTable(tableName, copy);
- } catch (TableNotDisabledException e) {
+ } catch (TableNotDisabledException re) {
expectedException = true;
}
assertTrue(expectedException);
@@ -130,7 +130,7 @@ public class TestAdmin {
assertEquals(newFlushSize, modifiedHtd.getMemStoreFlushSize());
assertEquals(key, modifiedHtd.getValue(key));
- // Reenable table.
+ // Reenable table to test it fails if not disabled.
this.admin.enableTable(tableName);
assertFalse(this.admin.isTableDisabled(tableName));
@@ -141,25 +141,53 @@ public class TestAdmin {
int maxversions = hcd.getMaxVersions();
final int newMaxVersions = maxversions + 1;
hcd.setMaxVersions(newMaxVersions);
- expectedException = false;
final byte [] hcdName = hcd.getName();
+ expectedException = false;
try {
this.admin.modifyColumn(tableName, hcd);
- LOG.info("Modified column");
- } catch (TableNotDisabledException e) {
- LOG.error("EXCEP", e);
+ } catch (TableNotDisabledException re) {
expectedException = true;
}
assertTrue(expectedException);
this.admin.disableTable(tableName);
assertTrue(this.admin.isTableDisabled(tableName));
- modifyColumn(tableName, hcd);
+ // Modify Column is synchronous
this.admin.modifyColumn(tableName, hcd);
modifiedHtd = this.admin.getTableDescriptor(tableName);
HColumnDescriptor modifiedHcd = modifiedHtd.getFamily(hcdName);
assertEquals(newMaxVersions, modifiedHcd.getMaxVersions());
- TODO: ADD/REMOVE COLUMN, REMOVE TABLE
+ // Try adding a column
+ // Reenable table to test it fails if not disabled.
+ this.admin.enableTable(tableName);
+ assertFalse(this.admin.isTableDisabled(tableName));
+ final String xtracolName = "xtracol";
+ HColumnDescriptor xtracol = new HColumnDescriptor(xtracolName);
+ xtracol.setValue(xtracolName, xtracolName);
+ try {
+ this.admin.addColumn(tableName, xtracol);
+ } catch (TableNotDisabledException re) {
+ expectedException = true;
+ }
+ assertTrue(expectedException);
+ this.admin.disableTable(tableName);
+ assertTrue(this.admin.isTableDisabled(tableName));
+ this.admin.addColumn(tableName, xtracol);
+ modifiedHtd = this.admin.getTableDescriptor(tableName);
+ hcd = modifiedHtd.getFamily(xtracol.getName());
+ assertTrue(hcd != null);
+ assertTrue(hcd.getValue(xtracolName).equals(xtracolName));
+
+ // Delete the just-added column.
+ this.admin.deleteColumn(tableName, xtracol.getName());
+ modifiedHtd = this.admin.getTableDescriptor(tableName);
+ hcd = modifiedHtd.getFamily(xtracol.getName());
+ assertTrue(hcd == null);
+
+ // Delete the table
+ this.admin.deleteTable(tableName);
+ this.admin.listTables();
+ assertFalse(this.admin.tableExists(tableName));
}
/**
@@ -188,30 +216,6 @@ public class TestAdmin {
}
/**
- * Modify table is async so wait on completion of the table operation in master.
- * @param tableName
- * @param htd
- * @throws IOException
- */
- private void modifyColumn(final byte [] tableName, final HColumnDescriptor hcd)
- throws IOException {
- MasterServices services = TEST_UTIL.getMiniHBaseCluster().getMaster();
- ExecutorService executor = services.getExecutorService();
- AtomicBoolean done = new AtomicBoolean(false);
- executor.registerListener(EventType.C2M_MODIFY_FAMILY, new DoneListener(done));
- this.admin.modifyColumn(tableName, hcd);
- while (!done.get()) {
- synchronized (done) {
- try {
- done.wait(1000);
- } catch (InterruptedException e) {
- e.printStackTrace();
- }
- }
- }
- }
-
- /**
* Listens for when an event is done in Master.
*/
static class DoneListener implements EventHandler.EventHandlerListener {