[ https://issues.apache.org/jira/browse/HBASE-13147?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14517417#comment-14517417 ]
Andrey Stepachev edited comment on HBASE-13147 at 4/28/15 5:03 PM: ------------------------------------------------------------------- Thank you [~anoop.hbase] for reviewing. {quote} For CATALOG_FAMILY family we have allowed user to configure the number of max versions but for the newly added TABLE_FAMILY HCD, it is again hard coded to 10. Why? {quote} Should be the same, will fix that. But with this patch it is not relevant actually, Meta descriptor stored on disc and all settings are preserved. So if you modify number of versions, they would be preserved between restarts and meta migrations. {quote} Just change the doc around HConstant#META_VERSION - It speaks abt keeping the META version in the ROOT table. Instead we (will) rely on the meta version which comes from the HTD attributes. {quote} It seems we need to file additional jira. There is a bunch of code which states that. This patch doesn't use this constant. (for example HRegion#addRegionToMETA says about root too, which is incorrect and should be fixed). {quote} May be we will need another to Jira to clean up things. In the public exposed HConstant only keep things which need to be exposed. For sharing common constants (which are not be public exposed) let us have another constants file. {quote} Not sure that version is private (in compare to other exposed fields of meta). I'd keep that as is until we will really want to hide meta from user. {quote} HTableDescriptor metaDescriptor = TableDescriptor.metaTableDescriptor(c) This is still a static HTD. WHy? {quote} No, it is not. It is created from config. Static method is for bootstrap, when we are formatting cluster or repairing it in fsck. Otherwise with this patch htd from filesystem will be used. {quote} Do we really need to add public APIs to HTD (which is client side public exposed)? Can we deal with getValue() only? {quote} This method encapsulate logic for default version used. So it better to stick code locally. But if there better alternative, can think about. {quote} While creating the HTDs and TableDescriptors from reading from FS, we will call .... So for old descriptor with no meta version in it, we consider as new version Will the below code in MetaMigration will get used then? else if (current.getMetaVersion() < HConstants.META_VERSION) { ... {quote} Meta will not exist on filesystem without version. It was never created in such way (it was always created on the fly), so this code is basically for future version updates (meta will have version in it). But it seems a place for bugs here, if meta will accidentally contain no version at all. I think we need to handle no-version explicitly and take care of places where we can allow no-version meta (it doesn't make sense actually, because without version it is much safe to create new meta then trying to find out what this meta version is). Thanks for pointing to that. was (Author: octo47): Thank you [~anoop.hbase] for reviewing. {quote} For CATALOG_FAMILY family we have allowed user to configure the number of max versions but for the newly added TABLE_FAMILY HCD, it is again hard coded to 10. Why? {quote} Should be the same, will fix that. But with this patch it is not relevant actually, Meta descriptor sotred on disc and all settings are preserved. So if you modify number of versions, they would be preserved between restarts and meta migrations. {quote} Just change the doc around HConstant#META_VERSION - It speaks abt keeping the META version in the ROOT table. Instead we (will) rely on the meta version which comes from the HTD attributes. {quote} It seems we need to file additional jira. There is a bunch of code which states that. This patch doesn't use this constant. (for example HRegion#addRegionToMETA says about root too, which is incorrect and should be fixed). {quote} May be we will need another to Jira to clean up things. In the public exposed HConstant only keep things which need to be exposed. For sharing common constants (which are not be public exposed) let us have another constants file. {quote} Not sure that version is private (in compare to other exposed fields of meta). I'd keep that as is until we will really want to hide meta from user. {quote} HTableDescriptor metaDescriptor = TableDescriptor.metaTableDescriptor(c) This is still a static HTD. WHy? {quote} No, it is not. It is created from config. Static method is for bootstrap, when we are formatting cluster or repairing it in fsck. Otherwise with this patch htd from filesystem will be used. {quote} Do we really need to add public APIs to HTD (which is client side public exposed)? Can we deal with getValue() only? {quote} This method encapsulate logic for default version used. So it better to stick code locally. But if there better alternative, can think about. {quote} While creating the HTDs and TableDescriptors from reading from FS, we will call .... So for old descriptor with no meta version in it, we consider as new version Will the below code in MetaMigration will get used then? else if (current.getMetaVersion() < HConstants.META_VERSION) { ... {quote} Meta will not exist on filesystem without version. It was never created in such way (it was always created on the fly), so this code is basically for future version updates (meta will have version in it). But it seems a place for bugs here, if meta will accidentally contain no version at all. I think we need to handle no-version explicitly and take care of places where we can allow no-version meta (it doesn't make sense actually, because without version it is much safe to create new meta then trying to find out what this meta version is). Thanks for pointing to that. > Load actual META table descriptor, don't use statically defined one. > -------------------------------------------------------------------- > > Key: HBASE-13147 > URL: https://issues.apache.org/jira/browse/HBASE-13147 > Project: HBase > Issue Type: Bug > Components: master, regionserver > Affects Versions: 2.0.0 > Reporter: Andrey Stepachev > Assignee: Andrey Stepachev > Attachments: HBASE-13147-branch-1.patch, > HBASE-13147-branch-1.v2.patch, HBASE-13147.patch, HBASE-13147.v2.patch, > HBASE-13147.v3.patch, HBASE-13147.v4.patch, HBASE-13147.v4.patch, > HBASE-13147.v5.patch, HBASE-13147.v6.patch, HBASE-13147.v7.patch > > > In HBASE-13087 stumbled on the fact, that region servers don't see actual > meta descriptor, they use their own, statically compiled. > Need to fix that. -- This message was sent by Atlassian JIRA (v6.3.4#6332)