[
https://issues.apache.org/jira/browse/HBASE-4213?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13126344#comment-13126344
]
Ted Yu commented on HBASE-4213:
-------------------------------
I only looked at part of the patch.
For getAndStartSchemaJanitorChore(), javadoc should be corrected:
{code}
+ // Start up the load balancer chore
+ Chore chore = new Chore(name, schemaJanitorPeriod, master) {
{code}
For getAndStartBalancerChore():
{code}
+ if (doesLoadBalancerNodeExists()) {
{code}
The trailing s in the above name is unnecessary.
For doesLoadBalancerNodeExists():
{code}
+ int r = ZKUtil.checkExists(this.zooKeeper, zooKeeper.loadBalancerZNode);
+ return ZKUtil.checkExists(this.zooKeeper, zooKeeper.loadBalancerZNode)
!= -1;
{code}
The assignment above is for debugging, I guess. Please remove it.
I think a better name for waitForCurrentSchemaChange() would be
waitForCurrentSchemaChangeToComplete().
But the following debug should be removed:
{code}
+ LOG.debug("Schema change operation is in progress. Waiting for " +
+ "it to complete before running the load balancer.");
+ Thread.sleep(1000);
{code}
In balance():
{code}
+ // I don't think we need the following check because we already have
+ // acquired the lock on balancer ??
{code}
So the following isLoadBalancerRunning() call can be removed :-)
For TableEventHandler.waitForLoadBalancerToComplete(), since
master.isLoadBalancerRunning() checks zk node, should isLoadBalancerRunning()
be moved to ZKUtil ? This would save RPC call.
MasterSchemaChangeTracker.handleFailedExpiredSchemaChanges() should be called
handleFailedOrExpiredSchemaChanges()
So latest patch doesn't use the new synchronous switchBalancer(). Interesting.
There're pending questions in the patch.
{code}
+ // Should we block here until we successfully create a LB node in ZK?
+ if (createLoadBalancerNode()) {
{code}
createLoadBalancerNode() should distinguish KeeperException's that may arise
and inform Master so that Master can make better decision.
For getAlterStatusFromSchemaChangeTracker():
{code}
+ LOG.debug("MasterAlterStatus is NULL for table = "
+ + Bytes.toString(tableName));
+ // should we throw IOException here as it makes more sense?
+ return new Pair<Integer, Integer>(0,0);
{code}
I think IOE makes more sense in the above case.
After the modified patch is posted to review board, comments would have better
context.
Thanks for your perseverance, Subbu.
This feature has been requested by many customers.
> Support for fault tolerant, instant schema updates with out master's
> intervention (i.e with out enable/disable and bulk assign/unassign) through
> ZK.
> ----------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-4213
> URL: https://issues.apache.org/jira/browse/HBASE-4213
> Project: HBase
> Issue Type: Improvement
> Reporter: Subbu M Iyer
> Assignee: Subbu M Iyer
> Fix For: 0.92.0
>
> Attachments:
> 4213-101211-Support_instant_schema_changes_through_ZK.patch,
> 4213-Instant_Schema_change_through_ZK.patch,
> 4213-V10-Support_instant_schema_changes_through_ZK.patch,
> 4213-V5-Support_instant_schema_changes_through_ZK.patch,
> 4213-V7-Support_instant_schema_changes_through_ZK.patch,
> 4213-V8-Support_instant_schema_changes_through_ZK.patch,
> 4213-V9-Support_instant_schema_changes_through_ZK.patch, 4213-v9.txt,
> 4213.v6, HBASE-4213-Instant_schema_change.patch,
> HBASE-4213_Instant_schema_change_-Version_2_.patch,
> HBASE_Instant_schema_change-version_3_.patch
>
>
> This Jira is a slight variation in approach to what is being done as part of
> https://issues.apache.org/jira/browse/HBASE-1730
> Support instant schema updates such as Modify Table, Add Column, Modify
> Column operations:
> 1. With out enable/disabling the table.
> 2. With out bulk unassign/assign of regions.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
