[ 
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)

Reply via email to