[
https://issues.apache.org/jira/browse/HBASE-457?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571585#action_12571585
]
stack commented on HBASE-457:
-----------------------------
Is this change intentional where you use regionManager instead of master in the
below?
{code}
- while (!master.closed.get() && !master.rootScanned &&
- master.rootRegionLocation.get() == null) {
+ while (!master.closed.get() && !regionManager.rootScanned &&
+ regionManager.rootRegionLocation.get() == null) {
{code}
These are unfortunate in BaseScanner but shouldn't block the patch:
'synchronized (regionManager.killList) {' i.e. synchronizing on a lock that is
over in another class. We can work on them over time.
Should we be passing in a ServerManager into Base/MetaScanner on construction?
Would it help w/ the likes of the below:
{code}
- storedInfo = master.serversToServerInfo.get(serverName);
- deadServer = master.deadServers.contains(serverName);
+ storedInfo = master.serverManager.serversToServerInfo.get(serverName);
+ deadServer = master.serverManager.deadServers.contains(serverName);
{code}
Would suggest adding a getServerManager accessor on master otherwise.
I like these changes:
{code}
- master.onlineMetaRegions.put(region.getStartKey(), region);
+ regionManager.putMetaRegionOnline(region);
{code}
This code seems odd:
{code}
- master.initialMetaScanComplete = true;
+ regionManager.initialMetaScanComplete = true;
{code}
Why does the region manager have the flag for whether a scan is complete --
meta or root? Could this be kept in the scanner classes themselves?
You could also add a setter to regionManager for this kinda stuff (so
MetaScanner doesn't have to know about regionmanager data members)
Should you be passing RegionManager to TableOperation constructor so you don't
have to do this?
{code}
- master.onlineMetaRegions.remove(info.getStartKey());
+ master.regionManager.offlineMetaRegion(info.getStartKey());
{code}
Should below just be changed to a master.getRegionLocation?
{code}
- server =
master.connection.getHRegionConnection(master.rootRegionLocation.get());
+ server = master.connection.getHRegionConnection(
+ master.regionManager.rootRegionLocation.get());
{code}
In ServerManager you have this:
{code}
+ protected volatile Set<String> deadServers =
+ Collections.synchronizedSet(new HashSet<String>());
{code}
Would suggest the volatile be changed to a final and that you default data
members as private till some one complains they need it otherwise (Same for
other members of this class)
> Factor Master into Master, RegionManager, and ServerManager
> -----------------------------------------------------------
>
> Key: HBASE-457
> URL: https://issues.apache.org/jira/browse/HBASE-457
> Project: Hadoop HBase
> Issue Type: Sub-task
> Components: master
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Priority: Minor
> Attachments: 457-v2.patch, 457-v3.patch, 457.patch
>
>
> Even with TableOperation and descendants factored out of HMaster, it's still
> a huge class. Every bit of master state basically lives in one class, so it's
> very challenging to understand the logical groupings of everything.
> To make this a little more manageable, let's make two new abstractions,
> ServerManager and RegionManager.
> ServerManager keeps track of servers - leases, message processing, load and
> load average, etc.
> RegionManager keeps track of the root location, meta table online state,
> assigning regions to servers, and so on.
> HMaster then keeps around one of each of these classes to track state and do
> it's master stuff. The HMaster class itself does not change it's interface to
> external consumers. It also retains the main processing loop, HBase closed
> state, start and stop, etc.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
