[
https://issues.apache.org/jira/browse/HBASE-4991?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13219412#comment-13219412
]
stack commented on HBASE-4991:
------------------------------
After looking again too at the patch, it has too much custom code that is all
about region delete.
It should take a range as Todd suggests earlier rather than list of regions.
This means you can not pass a list of discontinuous regions but thats ok I
think; just do multiple invocations.
This seems to have wrong param name and javadoc:
{code}
+ /**
+ * Gets the count of online regions of the table in a region server.
+ * This method looks at the in-memory onlineRegions.
+ * @param regionName
+ * @return int regions count
+ * @throws IOException
+ */
+ public int getRegionsCount(byte[] regionName) throws IOException;
{code}
When I see MasterDeleteRegionTracker, and the equivalent for regionservers, it
makes me yearn for a generic framework that these things could run on; it
strikes me as too much custom code and custom handlers. This we should fix.
We should come up w/ generics that can be customized to do feature specifics.
Why are we using a janitor to do the delete of regions rather than an executor?
Why we have this getDeleteRegionTracker ?
The generic soln Interface would have a method the balancer would check...
+ if (deleteRegionTracker.isDeleteRegionInProgress()) {
Rather than do the above for every feature we add.
Should this getDeleteRegionStatusFromDeleteRegionTracker be in the
DeleteRegionTracker? And should it be something that is apart from the Master
rather than in the master?
This seems wrong: getDeleteRegionTracker in the MasterServices Interface.
Why we add it there? Why can't it be independent of Master? Having to have a
Master makes it harder to test I'm sure.
DeleteRegionHandler should not be dealing w/ balancer. That seems dirty.
This seems racy: waitForInflightSplit
Do we do this every time? + moveStoreFilesToNewRegionDir(byFamily, fs,
tableDir, newRegionInfo);
If so, is this actually a merge and not a delete?
Do these methods need to be in HREgionInfo?
moveDataFromAdjacentRegionToNewRegion
createNewRegionFromAdjacentRegion
Could be in HRegion or in a RegionUtil class? RS is already bloated.
A bunch of these other methods ... adding new region and deleting old region
... would seem to have overlap with existing code where we add regions to meta
after open and also w/ merge code?
We can't have master package refernced in zookeeper package; i.e. see
MasterDeleteRegionTracker.
I've already commented on other stuff in this patch.
In general the patch is well done. It just adds a bunch of custom facility w/o
genericizing at least some aspects so could be used by other features yet to
come. In particular, this looks to be a specialization on merge. If so, lets
go for merge altogether.
> Provide capability to delete named region
> -----------------------------------------
>
> Key: HBASE-4991
> URL: https://issues.apache.org/jira/browse/HBASE-4991
> Project: HBase
> Issue Type: Improvement
> Reporter: Ted Yu
> Assignee: Mubarak Seyed
> Fix For: 0.94.0
>
> Attachments: HBASE-4991.trunk.v1.patch, HBASE-4991.trunk.v2.patch
>
>
> See discussion titled 'Able to control routing to Solr shards or not' on
> lily-discuss
> User may want to quickly dispose of out of date records by deleting specific
> 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