[ 
https://issues.apache.org/jira/browse/HBASE-451?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13043122#comment-13043122
 ] 

jirapos...@reviews.apache.org commented on HBASE-451:
-----------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/849/#review749
-----------------------------------------------------------


This looks excellent.  Lets get it committed before it rots.   Do we still need 
.regioninfo after this goes in?


src/main/java/org/apache/hadoop/hbase/HConstants.java
<https://reviews.apache.org/r/849/#comment1541>

    Copy/paste error.



src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
<https://reviews.apache.org/r/849/#comment1542>

    WhooHoo... this is starting out good!
    
    I think you should take the opportunity of changing HRI to be a 
VersionedWritable.



src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
<https://reviews.apache.org/r/849/#comment1543>

    Excellent! (HRI not having ref to HTD)



src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
<https://reviews.apache.org/r/849/#comment1544>

    This works?  Thats interesting.



src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
<https://reviews.apache.org/r/849/#comment1545>

    When would this be used?



src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
<https://reviews.apache.org/r/849/#comment1546>

    This should not be allowed?  Are you not going to mess up stuff if table 
gets changed on an HRI?



src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
<https://reviews.apache.org/r/849/#comment1547>

    Should these be deprecated rather than removed?  Even if we returned a null 
only?



src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
<https://reviews.apache.org/r/849/#comment1548>

    This is perverse, begin able to change the HTD under an HRI.  Can't believe 
this actually was allowed exist.



src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
<https://reviews.apache.org/r/849/#comment1549>

    You could just do an equals since its boolean... Bytes.equals...



src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
<https://reviews.apache.org/r/849/#comment1550>

    ditto



src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
<https://reviews.apache.org/r/849/#comment1551>

    This is incorrect.  Should be isMetaTable() or isRootRegion... a 'meta' 
region is a .META. or -ROOT- member.



src/main/java/org/apache/hadoop/hbase/HRegionInfoForMigration.java
<https://reviews.apache.org/r/849/#comment1552>

    Put this into a migration subpackage?



src/main/java/org/apache/hadoop/hbase/HRegionInfoForMigration.java
<https://reviews.apache.org/r/849/#comment1554>

    Oh, so what is this?  Its old-school HRI?  If so, call it HRI090?  Or if 
you have it in a subpackage  migration, it can have same name and just be fully 
specified whereever it is used.. the migration subpackage will make it 
explicity taht this is not same as new HRI.



src/main/java/org/apache/hadoop/hbase/HRegionInfoForMigration.java
<https://reviews.apache.org/r/849/#comment1553>

    This is from elsewhere?  Does it belong here?



src/main/java/org/apache/hadoop/hbase/KeyValue.java
<https://reviews.apache.org/r/849/#comment1555>

    I don't think we should do this by default.  What if KV has a value of MBs?



src/main/java/org/apache/hadoop/hbase/catalog/MetaEditor.java
<https://reviews.apache.org/r/849/#comment1557>

    What if we crash half-way through migration?  We should be able to deal w/ 
a .META. that is half the old style HRI and half the new style?



src/main/java/org/apache/hadoop/hbase/catalog/MetaEditor.java
<https://reviews.apache.org/r/849/#comment1556>

    You want to leave these in place?



src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<https://reviews.apache.org/r/849/#comment1558>

    What a crazy way of getting HTDs.



src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java
<https://reviews.apache.org/r/849/#comment1559>

    Do we need these last two? Can't we use this first method to get the latter?



src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
<https://reviews.apache.org/r/849/#comment1560>

    Is this going to the FS to read tables?  And we're holding sync while we 
are doing it?  Is that necessary?



src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
<https://reviews.apache.org/r/849/#comment1561>

    This method belongs in master since its going to run the migration?



src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
<https://reviews.apache.org/r/849/#comment1562>

    Close the file?



src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
<https://reviews.apache.org/r/849/#comment1563>

    Good.  The human readable version is included.



src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
<https://reviews.apache.org/r/849/#comment1564>

    I wonder if you have to create the file elsewhere and then do a rename to 
put it in place?  Else if crash, could be half-written?  Do we do this w/ 
.regioninfo?


- Michael


On 2011-06-02 20:52:15, Michael Stack wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/849/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-06-02 20:52:15)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Posting for Subbu  See issue for his description of change.
bq.  
bq.  
bq.  This addresses bug HBASE-451.
bq.      https://issues.apache.org/jira/browse/HBASE-451
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/HConstants.java bd4c64c 
bq.    src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 9502b1d 
bq.    src/main/java/org/apache/hadoop/hbase/HRegionInfoForMigration.java 
PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/KeyValue.java 7033800 
bq.    src/main/java/org/apache/hadoop/hbase/catalog/MetaEditor.java a25b0f0 
bq.    src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java eb57197 
bq.    src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java 7594822 
bq.    src/main/java/org/apache/hadoop/hbase/client/HConnection.java f722155 
bq.    src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 
94ee1a0 
bq.    src/main/java/org/apache/hadoop/hbase/client/HTable.java c82e1dd 
bq.    src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java 2734f30 
bq.    
src/main/java/org/apache/hadoop/hbase/client/UnmodifyableHRegionInfo.java 
23e7a6b 
bq.    src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java 
d531b8d 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java 4704c39 
bq.    src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 
38914a8 
bq.    src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 5b4a4b7 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java b8489ac 
bq.    src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 
b22a3e4 
bq.    
src/main/java/org/apache/hadoop/hbase/master/handler/ClosedRegionHandler.java 
c98ed17 
bq.    
src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java 
5a8bb20 
bq.    
src/main/java/org/apache/hadoop/hbase/master/handler/ModifyTableHandler.java 
6380520 
bq.    
src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 
3d16e47 
bq.    
src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java 
dace150 
bq.    
src/main/java/org/apache/hadoop/hbase/master/handler/TableAddFamilyHandler.java 
fcea483 
bq.    
src/main/java/org/apache/hadoop/hbase/master/handler/TableDeleteFamilyHandler.java
 a963c6c 
bq.    
src/main/java/org/apache/hadoop/hbase/master/handler/TableModifyFamilyHandler.java
 4029893 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java e5bd154 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/LogRoller.java 
9ccf248 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 
072fd8d 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 21468ad 
bq.    
src/main/java/org/apache/hadoop/hbase/regionserver/handler/OpenRegionHandler.java
 ba2daa9 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 0716788 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALObserver.java 
3def4b6 
bq.    
src/main/java/org/apache/hadoop/hbase/replication/regionserver/Replication.java 
1a87947 
bq.    src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3409108 
bq.    src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 39591a0 
bq.    src/main/java/org/apache/hadoop/hbase/util/HMerge.java c07f8dc 
bq.    src/main/java/org/apache/hadoop/hbase/util/MetaUtils.java 540d7df 
bq.    src/main/java/org/apache/hadoop/hbase/util/RegionSplitter.java 6d0c803 
bq.    src/main/java/org/apache/hadoop/hbase/util/Writables.java 3e60f97 
bq.    src/test/java/org/apache/hadoop/hbase/HBaseTestCase.java 4c58791 
bq.    src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java babd788 
bq.    src/test/java/org/apache/hadoop/hbase/TestCompare.java bbac815 
bq.    src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java 
1f51703 
bq.    src/test/java/org/apache/hadoop/hbase/TestSerialization.java 05f0efc 
bq.    src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java 1105509 
bq.    src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java 
c3b23fe 
bq.    src/test/java/org/apache/hadoop/hbase/client/TestMetaMigration.java 
PRE-CREATION 
bq.    src/test/java/org/apache/hadoop/hbase/client/TestTimestamp.java db42192 
bq.    
src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java 
f0418d1 
bq.    
src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java
 46ba184 
bq.    
src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverStacking.java
 18380c6 
bq.    src/test/java/org/apache/hadoop/hbase/coprocessor/TestWALObserver.java 
19397fb 
bq.    src/test/java/org/apache/hadoop/hbase/filter/TestColumnPrefixFilter.java 
e1eb02a 
bq.    
src/test/java/org/apache/hadoop/hbase/filter/TestDependentColumnFilter.java 
04705c3 
bq.    src/test/java/org/apache/hadoop/hbase/filter/TestFilter.java bfa3c72 
bq.    src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 
ada2af6 
bq.    
src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 
ba87bc0 
bq.    src/test/java/org/apache/hadoop/hbase/master/TestLoadBalancer.java 
d909997 
bq.    src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 
2022767 
bq.    
src/test/java/org/apache/hadoop/hbase/master/TestMasterStatusServlet.java 
1fef788 
bq.    
src/test/java/org/apache/hadoop/hbase/regionserver/TestColumnSeeking.java 
e2f4507 
bq.    
src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 
48fa162 
bq.    
src/test/java/org/apache/hadoop/hbase/regionserver/TestGetClosestAtOrBefore.java
 3b7c7e8 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 
e106b45 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 
516139b 
bq.    
src/test/java/org/apache/hadoop/hbase/regionserver/TestRSStatusServlet.java 
40d352e 
bq.    
src/test/java/org/apache/hadoop/hbase/regionserver/TestResettingCounters.java 
f092371 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/TestScanner.java 
ef8a4b2 
bq.    
src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 
b85b912 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 
dbe5fb1 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/TestWideScanner.java 
2cc197f 
bq.    
src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java
 bcf9024 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLog.java 
e2c158a 
bq.    
src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALObserver.java 
5b95154 
bq.    
src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 
4de5b95 
bq.    
src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java
 3f9302a 
bq.    
src/test/java/org/apache/hadoop/hbase/rest/model/TestTableRegionModel.java 
c02dfda 
bq.    src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java a6bdbe0 
bq.    src/test/java/org/apache/hadoop/hbase/util/TestMergeTable.java 3039df2 
bq.    src/test/java/org/apache/hadoop/hbase/util/TestMergeTool.java 18cd055 
bq.  
bq.  Diff: https://reviews.apache.org/r/849/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Michael
bq.  
bq.



> Remove HTableDescriptor from HRegionInfo
> ----------------------------------------
>
>                 Key: HBASE-451
>                 URL: https://issues.apache.org/jira/browse/HBASE-451
>             Project: HBase
>          Issue Type: Improvement
>          Components: master, regionserver
>    Affects Versions: 0.2.0
>            Reporter: Jim Kellerman
>            Assignee: Subbu M Iyer
>            Priority: Critical
>             Fix For: 0.92.0
>
>         Attachments: 451_support_for_removing_HTD_from_HRI_trunk.txt, 
> HBASE-451_-_First_draft_support_for_removing_HTD_from_HRI1.patch
>
>
> There is an HRegionInfo for every region in HBase. Currently HRegionInfo also 
> contains the HTableDescriptor (the schema). That means we store the schema n 
> times where n is the number of regions in the table.
> Additionally, for every region of the same table that the region server has 
> open, there is a copy of the schema. Thus it is stored in memory once for 
> each open region.
> If HRegionInfo merely contained the table name the HTableDescriptor could be 
> stored in a separate file and easily found.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to