Hi,
Looking the the two methods
1. isMetaTable() - returns if table is meta and not root, i.e returns if the
table represents .META.

    public boolean isMetaTable() {
        return isMetaRegion() && !isRootRegion();
    }

2. isMetaTable(String tableName) returns if a table is either .META. or
-ROOT-

    public static boolean isMetaTable(final byte [] tableName) {
        return Bytes.equals(tableName, HConstants.ROOT_TABLE_NAME) ||
          Bytes.equals(tableName, HConstants.META_TABLE_NAME);
    }

This overload of the function is not at all ok. Same method behaving very
differently in two situations is very confusing even with the detailed
Javadoc in place.

I'd suggest changing the implementation of 2 to return only if the table is
.META.

Like to hear your thoughts

Cheers,
Akash A

On Sun, Oct 2, 2011 at 2:07 AM, Shumin Wu <[email protected]> wrote:

> Hello All,
>
> I am new here. So very nice to meet you guys.
>
> Here are my thoughts on this issue. I agree with Akash that the name of
> this
> method would confuse the user as there is a .META. notion that means
> something else. So why don't we create a function alias for isMetaTable()
> as
> a temporary solution? When users gradually migrate to the new API
> (isCatalogTable), we can then deprecate the isMetaTable function. Of course
> the javadoc will need to well explain it.
>
>
> Code Snippet:
>
>  public boolean isMetaTable() {
>    return isCatalogTable();
>  }
>
>  public boolean isCatalogTable() {
>    return isMetaRegion() && !isRootRegion();
>  }
>
> Thanks,
> Shumin
>
> On Sat, Oct 1, 2011 at 1:16 PM, Akash Ashok <[email protected]>
> wrote:
>
> > Ok Thanks. I shall just add the javadoc. I've already opened a JIRA for
> > improving the javadoc
> >
> > https://issues.apache.org/jira/browse/HBASE-4486
> >
> > Cheers,
> > Akash A
> >
> > On Sun, Oct 2, 2011 at 12:21 AM, Dhruba Borthakur <[email protected]>
> > wrote:
> >
> > > +1 for Option 4.
> > >
> > > -dhruba
> > >
> > > On Sat, Oct 1, 2011 at 11:26 AM, Ted Yu <[email protected]> wrote:
> > >
> > > > I prefer the fourth option.
> > > > Only truly broken APIs should be removed.
> > > >
> > > > Cheers
> > > >
> > > > On Oct 1, 2011, at 10:02 AM, Doug Meil <
> [email protected]>
> > > > wrote:
> > > >
> > > > >
> > > > > Be very careful about code-changes that causes compilation failures
> > > upon
> > > > > upgrade.  Once in a while that may happen on a major upgrade, but
> it
> > > had
> > > > > better be for a very good, explainable, reason that the community
> is
> > > > > behind.
> > > > >
> > > > >
> > > > > A fourth option is to add Javadoc to the explaining what this
> method
> > > does
> > > > > and not add any new methods.
> > > > >
> > > > >
> > > > > On 10/1/11 8:02 AM, "Akash Ashok" <[email protected]> wrote:
> > > > >
> > > > >> While I was doing this I found out that there is a method in
> > > > >> HTableDescriptor called isMetaTable() which basically checks if
> the
> > > > table
> > > > >> is
> > > > >> a catalog table and the name kind of misleading. I am planning to
> > > rename
> > > > >> this method to isCatalogTable().
> > > > >>
> > > > >> 1. Is it ok to do this renaming considering that this is a client
> > side
> > > > >> API/
> > > > >> Admin API might lead to a lot of changes on client side for people
> > > > already
> > > > >> using this method?
> > > > >>
> > > > >> 2. If yes, is it ok to do it under the same JIRA HBase-4486 or
> > should
> > > > >> another another JIRA be file for this ?
> > > > >>
> > > > >> 3. If no, could this method be deprecated and another new method
> > > > >> isCatalogTable() be added ?
> > > > >>
> > > > >> Cheers,
> > > > >> Akash A
> > > > >>
> > > > >> On Mon, Sep 26, 2011 at 10:25 PM, Jean-Daniel Cryans
> > > > >> <[email protected]>wrote:
> > > > >>
> > > > >>> Be my guest!
> > > > >>>
> > > > >>> J-D
> > > > >>>
> > > > >>> On Sat, Sep 24, 2011 at 9:16 AM, Akash <[email protected]>
> > > wrote:
> > > > >>>> Hi,
> > > > >>>>    I was just lookin at the HTableDescriptor and realized it has
> > > very
> > > > >>> minimal javadoc. Considering this is an admin API I feel there
> > should
> > > > be
> > > > >>> better javadoc. If there's no Jira already open I would go ahead
> > and
> > > > >>> file
> > > > >>> it. Thanks
> > > > >>>>
> > > > >>>> Sent from my iPhone
> > > > >>>
> > > > >
> > > >
> > >
> > >
> > >
> > > --
> > > Connect to me at http://www.facebook.com/dhruba
> > >
> >
>

Reply via email to