hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-451) Remove HTableDescriptor from HRegionInfo
Date Fri, 03 Jun 2011 23:09:47 GMT

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

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



bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/HConstants.java, line 174
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20288#file20288line174>
bq.  >
bq.  >     Copy/paste error.

Fixed.


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/HRegionInfo.java, line 211
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20289#file20289line211>
bq.  >
bq.  >     When would this be used?

Currently used from test classes. can remove it after validation.


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/HRegionInfo.java, line 512
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20289#file20289line512>
bq.  >
bq.  >     This should not be allowed?  Are you not going to mess up stuff if table gets
changed on an HRI?

Yup. totally. removed it.


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/HRegionInfo.java, line 547
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20289#file20289line547>
bq.  >
bq.  >     Should these be deprecated rather than removed?  Even if we returned a null
only?

Hmm. Any advantage of keeping it deprecated and returning NULL versus totally slaughtering?
I can bring it back on.


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/HRegionInfo.java, line 560
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20289#file20289line560>
bq.  >
bq.  >     You could just do an equals since its boolean... Bytes.equals...

Good. Done.


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/HRegionInfo.java, line 568
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20289#file20289line568>
bq.  >
bq.  >     ditto

Good. Done.


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/HRegionInfo.java, line 574
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20289#file20289line574>
bq.  >
bq.  >     This is incorrect.  Should be isMetaTable() or isRootRegion... a 'meta' region
is a .META. or -ROOT- member.

Not sure I get it. We do need support for isMetaRegion API call to determine if the Region
represented by this HRI is a meta one. How shall we do it? 


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/HRegionInfoForMigration.java, line 20
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20290#file20290line20>
bq.  >
bq.  >     Put this into a migration subpackage?

Could not agree more on this. I shall create a migration package *.hbase.migration and move
this there.


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/HRegionInfoForMigration.java, line 42
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20290#file20290line42>
bq.  >
bq.  >     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.

hmm.. makes sense. so you want to remove it from the class name.


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/HRegionInfoForMigration.java, line 48
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20290#file20290line48>
bq.  >
bq.  >     This is from elsewhere?  Does it belong here?

Its from old school HRI "as is". 


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/KeyValue.java, line 600
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20291#file20291line600>
bq.  >
bq.  >     I don't think we should do this by default.  What if KV has a value of MBs?

yup. good call. removed it.


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/catalog/MetaEditor.java, line 250
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20292#file20292line250>
bq.  >
bq.  >     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?

Yup agreed. Need to think about how to handle this. any ideas? We might probably need to read
the META rows using old school HRI till a failure (IOException trying to serialize a new HRI
into old HRI) and assume that the rest are all processed? 


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/catalog/MetaEditor.java, line 276
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20292#file20292line276>
bq.  >
bq.  >     You want to leave these in place?

Nope. slipped through the cracks


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java, line 638
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20296#file20296line638>
bq.  >
bq.  >     What a crazy way of getting HTDs.

You want to rename? suggestions?


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java, line 216
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20301#file20301line216>
bq.  >
bq.  >     Do we need these last two? Can't we use this first method to get the latter?

We definitely can. Thought we can avoid iterating over the loop esp if we have large number
of tables in the cluster. 


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java, line 2179
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20302#file20302line2179>
bq.  >
bq.  >     This method belongs in master since its going to run the migration?

Yup moved it to Master.


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java, line 421
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20305#file20305line421>
bq.  >
bq.  >     Close the file?

Yup. Done


bq.  On 2011-06-02 23:07:01, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java, line 473
bq.  > <https://reviews.apache.org/r/849/diff/1/?file=20305#file20305line473>
bq.  >
bq.  >     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?

Let me check this. I have seen this creating in tmp and renaming it later pattern. 


- Subbu


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


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

Mime
View raw message