You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Ted Yu <yu...@gmail.com> on 2011/07/23 06:50:11 UTC

Re: hiding default ctor of HTD

Looks like I need to enhance the following code in HbaseObjectWritable (line
575):
        Writable writable = WritableFactories.newInstance(instanceClass,
conf);

If HTableDescriptor has the following method where T is HTableDescriptor:
public <T> T newInstance(DataInput dataInput)

HbaseObjectWritable.readObject() would be able to create an HTD instance
without relying on its default ctor.

On Fri, Jul 22, 2011 at 9:31 PM, Stack <st...@duboce.net> wrote:

> Try your proposal.  I don't think it will work.  It'll fail over in
> RPC where expectation is that there is a null/default constructor on
> every Writable.
> St.Ack
>
> On Fri, Jul 22, 2011 at 9:24 PM, Ted Yu <yu...@gmail.com> wrote:
> > My proposal below would make default HTableDescriptor ctor package
> private.
> >
> > If there is no object, I will log a new JIRA.
> >
> > On Fri, Jul 22, 2011 at 4:27 PM, Ted Yu (JIRA) <ji...@apache.org> wrote:
> >
> >>
> >>    [
> >>
> https://issues.apache.org/jira/browse/HBASE-4127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13069840#comment-13069840
> ]
> >>
> >> Ted Yu commented on HBASE-4127:
> >> -------------------------------
> >>
> >> +1 on the patch.
> >>
> >> Long term, we should hide this ctor, HTableDescriptor().
> >> Its existence is to support the following pattern:
> >> {code}
> >>      hTableDescriptor = new HTableDescriptor();
> >>      hTableDescriptor.readFields(fsDataInputStream);
> >> {code}
> >> I think we should replace this ctor with the following:
> >> {code}
> >> public HTableDescriptor getTableDescriptor(DataInputStream
> dataInputStream)
> >> {code}
> >>
> >> > HBaseAdmin : Don't modify table's name away
> >> > -------------------------------------------
> >> >
> >> >                 Key: HBASE-4127
> >> >                 URL: https://issues.apache.org/jira/browse/HBASE-4127
> >> >             Project: HBase
> >> >          Issue Type: Bug
> >> >          Components: client, master
> >> >    Affects Versions: 0.92.0
> >> >            Reporter: Nicolas Spiegelberg
> >> >            Assignee: Nicolas Spiegelberg
> >> >            Priority: Blocker
> >> >         Attachments: HBASE-4127.patch
> >> >
> >> >
> >> > One of the developers was using the default constructor for
> >> HTableDescriptor, which is sadly a bad constructor that should never be
> >> used. It made the tablename empty in META & caused an ERROR cycle as
> region
> >> onlining kept failing. We should have never let this happen. Don't do
> table
> >> modifications if the HTableDescriptor name doesn't match the table name
> >> passed in.
> >>
> >> --
> >> This message is automatically generated by JIRA.
> >> For more information on JIRA, see:
> http://www.atlassian.com/software/jira
> >>
> >>
> >>
> >
>

Re: hiding default ctor of HTD

Posted by Ted Yu <yu...@gmail.com>.
Here is the skeleton of proposed change to HbaseObjectWritable.readObject():

      if(Writable.class.isAssignableFrom(instanceClass)){

if(WritableWithoutDefaultCtor.class.isAssignableFrom(instanceClass)){
          Method method = instanceClass.getMethod("newInstance",
DataInput.class);
          try {
            Writable writable = method.invoke(null, in);
          } catch (Exception e) {
            ...
          }
        }
      }

On Fri, Jul 22, 2011 at 10:48 PM, Ted Yu <yu...@gmail.com> wrote:

> The scope of changes is bigger than I thought initially.
>
> The introduction of this method:
>   public static Writable newInstance(DataInput dataInput)
> would probably lead to an abstract class which is called, e.g.
> WritableWithoutDefaultCtor.
>
>       if(Writable.class.isAssignableFrom(instanceClass)){
>
> if(WritableWithoutDefaultCtor.class.isAssignableFrom(instanceClass)){
>
> HTD would be the first beneficiary.
>
>
> On Fri, Jul 22, 2011 at 10:03 PM, Stack <st...@duboce.net> wrote:
>
>> You are fixing it for HTD only?  Not all Writables?
>> St>Ack
>>
>> On Fri, Jul 22, 2011 at 9:50 PM, Ted Yu <yu...@gmail.com> wrote:
>> > Looks like I need to enhance the following code in HbaseObjectWritable
>> (line
>> > 575):
>> >        Writable writable = WritableFactories.newInstance(instanceClass,
>> > conf);
>> >
>> > If HTableDescriptor has the following method where T is
>> HTableDescriptor:
>> > public <T> T newInstance(DataInput dataInput)
>> >
>> > HbaseObjectWritable.readObject() would be able to create an HTD instance
>> > without relying on its default ctor.
>> >
>> > On Fri, Jul 22, 2011 at 9:31 PM, Stack <st...@duboce.net> wrote:
>> >
>> >> Try your proposal.  I don't think it will work.  It'll fail over in
>> >> RPC where expectation is that there is a null/default constructor on
>> >> every Writable.
>> >> St.Ack
>> >>
>> >> On Fri, Jul 22, 2011 at 9:24 PM, Ted Yu <yu...@gmail.com> wrote:
>> >> > My proposal below would make default HTableDescriptor ctor package
>> >> private.
>> >> >
>> >> > If there is no object, I will log a new JIRA.
>> >> >
>> >> > On Fri, Jul 22, 2011 at 4:27 PM, Ted Yu (JIRA) <ji...@apache.org>
>> wrote:
>> >> >
>> >> >>
>> >> >>    [
>> >> >>
>> >>
>> https://issues.apache.org/jira/browse/HBASE-4127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13069840#comment-13069840
>> >> ]
>> >> >>
>> >> >> Ted Yu commented on HBASE-4127:
>> >> >> -------------------------------
>> >> >>
>> >> >> +1 on the patch.
>> >> >>
>> >> >> Long term, we should hide this ctor, HTableDescriptor().
>> >> >> Its existence is to support the following pattern:
>> >> >> {code}
>> >> >>      hTableDescriptor = new HTableDescriptor();
>> >> >>      hTableDescriptor.readFields(fsDataInputStream);
>> >> >> {code}
>> >> >> I think we should replace this ctor with the following:
>> >> >> {code}
>> >> >> public HTableDescriptor getTableDescriptor(DataInputStream
>> >> dataInputStream)
>> >> >> {code}
>> >> >>
>> >> >> > HBaseAdmin : Don't modify table's name away
>> >> >> > -------------------------------------------
>> >> >> >
>> >> >> >                 Key: HBASE-4127
>> >> >> >                 URL:
>> https://issues.apache.org/jira/browse/HBASE-4127
>> >> >> >             Project: HBase
>> >> >> >          Issue Type: Bug
>> >> >> >          Components: client, master
>> >> >> >    Affects Versions: 0.92.0
>> >> >> >            Reporter: Nicolas Spiegelberg
>> >> >> >            Assignee: Nicolas Spiegelberg
>> >> >> >            Priority: Blocker
>> >> >> >         Attachments: HBASE-4127.patch
>> >> >> >
>> >> >> >
>> >> >> > One of the developers was using the default constructor for
>> >> >> HTableDescriptor, which is sadly a bad constructor that should never
>> be
>> >> >> used. It made the tablename empty in META & caused an ERROR cycle as
>> >> region
>> >> >> onlining kept failing. We should have never let this happen. Don't
>> do
>> >> table
>> >> >> modifications if the HTableDescriptor name doesn't match the table
>> name
>> >> >> passed in.
>> >> >>
>> >> >> --
>> >> >> This message is automatically generated by JIRA.
>> >> >> For more information on JIRA, see:
>> >> http://www.atlassian.com/software/jira
>> >> >>
>> >> >>
>> >> >>
>> >> >
>> >>
>> >
>>
>
>

Re: hiding default ctor of HTD

Posted by Ted Yu <yu...@gmail.com>.
The scope of changes is bigger than I thought initially.

The introduction of this method:
  public static Writable newInstance(DataInput dataInput)
would probably lead to an abstract class which is called, e.g.
WritableWithoutDefaultCtor.

      if(Writable.class.isAssignableFrom(instanceClass)){

if(WritableWithoutDefaultCtor.class.isAssignableFrom(instanceClass)){

HTD would be the first beneficiary.

On Fri, Jul 22, 2011 at 10:03 PM, Stack <st...@duboce.net> wrote:

> You are fixing it for HTD only?  Not all Writables?
> St>Ack
>
> On Fri, Jul 22, 2011 at 9:50 PM, Ted Yu <yu...@gmail.com> wrote:
> > Looks like I need to enhance the following code in HbaseObjectWritable
> (line
> > 575):
> >        Writable writable = WritableFactories.newInstance(instanceClass,
> > conf);
> >
> > If HTableDescriptor has the following method where T is HTableDescriptor:
> > public <T> T newInstance(DataInput dataInput)
> >
> > HbaseObjectWritable.readObject() would be able to create an HTD instance
> > without relying on its default ctor.
> >
> > On Fri, Jul 22, 2011 at 9:31 PM, Stack <st...@duboce.net> wrote:
> >
> >> Try your proposal.  I don't think it will work.  It'll fail over in
> >> RPC where expectation is that there is a null/default constructor on
> >> every Writable.
> >> St.Ack
> >>
> >> On Fri, Jul 22, 2011 at 9:24 PM, Ted Yu <yu...@gmail.com> wrote:
> >> > My proposal below would make default HTableDescriptor ctor package
> >> private.
> >> >
> >> > If there is no object, I will log a new JIRA.
> >> >
> >> > On Fri, Jul 22, 2011 at 4:27 PM, Ted Yu (JIRA) <ji...@apache.org>
> wrote:
> >> >
> >> >>
> >> >>    [
> >> >>
> >>
> https://issues.apache.org/jira/browse/HBASE-4127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13069840#comment-13069840
> >> ]
> >> >>
> >> >> Ted Yu commented on HBASE-4127:
> >> >> -------------------------------
> >> >>
> >> >> +1 on the patch.
> >> >>
> >> >> Long term, we should hide this ctor, HTableDescriptor().
> >> >> Its existence is to support the following pattern:
> >> >> {code}
> >> >>      hTableDescriptor = new HTableDescriptor();
> >> >>      hTableDescriptor.readFields(fsDataInputStream);
> >> >> {code}
> >> >> I think we should replace this ctor with the following:
> >> >> {code}
> >> >> public HTableDescriptor getTableDescriptor(DataInputStream
> >> dataInputStream)
> >> >> {code}
> >> >>
> >> >> > HBaseAdmin : Don't modify table's name away
> >> >> > -------------------------------------------
> >> >> >
> >> >> >                 Key: HBASE-4127
> >> >> >                 URL:
> https://issues.apache.org/jira/browse/HBASE-4127
> >> >> >             Project: HBase
> >> >> >          Issue Type: Bug
> >> >> >          Components: client, master
> >> >> >    Affects Versions: 0.92.0
> >> >> >            Reporter: Nicolas Spiegelberg
> >> >> >            Assignee: Nicolas Spiegelberg
> >> >> >            Priority: Blocker
> >> >> >         Attachments: HBASE-4127.patch
> >> >> >
> >> >> >
> >> >> > One of the developers was using the default constructor for
> >> >> HTableDescriptor, which is sadly a bad constructor that should never
> be
> >> >> used. It made the tablename empty in META & caused an ERROR cycle as
> >> region
> >> >> onlining kept failing. We should have never let this happen. Don't do
> >> table
> >> >> modifications if the HTableDescriptor name doesn't match the table
> name
> >> >> passed in.
> >> >>
> >> >> --
> >> >> This message is automatically generated by JIRA.
> >> >> For more information on JIRA, see:
> >> http://www.atlassian.com/software/jira
> >> >>
> >> >>
> >> >>
> >> >
> >>
> >
>

Re: hiding default ctor of HTD

Posted by Stack <st...@duboce.net>.
You are fixing it for HTD only?  Not all Writables?
St>Ack

On Fri, Jul 22, 2011 at 9:50 PM, Ted Yu <yu...@gmail.com> wrote:
> Looks like I need to enhance the following code in HbaseObjectWritable (line
> 575):
>        Writable writable = WritableFactories.newInstance(instanceClass,
> conf);
>
> If HTableDescriptor has the following method where T is HTableDescriptor:
> public <T> T newInstance(DataInput dataInput)
>
> HbaseObjectWritable.readObject() would be able to create an HTD instance
> without relying on its default ctor.
>
> On Fri, Jul 22, 2011 at 9:31 PM, Stack <st...@duboce.net> wrote:
>
>> Try your proposal.  I don't think it will work.  It'll fail over in
>> RPC where expectation is that there is a null/default constructor on
>> every Writable.
>> St.Ack
>>
>> On Fri, Jul 22, 2011 at 9:24 PM, Ted Yu <yu...@gmail.com> wrote:
>> > My proposal below would make default HTableDescriptor ctor package
>> private.
>> >
>> > If there is no object, I will log a new JIRA.
>> >
>> > On Fri, Jul 22, 2011 at 4:27 PM, Ted Yu (JIRA) <ji...@apache.org> wrote:
>> >
>> >>
>> >>    [
>> >>
>> https://issues.apache.org/jira/browse/HBASE-4127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13069840#comment-13069840
>> ]
>> >>
>> >> Ted Yu commented on HBASE-4127:
>> >> -------------------------------
>> >>
>> >> +1 on the patch.
>> >>
>> >> Long term, we should hide this ctor, HTableDescriptor().
>> >> Its existence is to support the following pattern:
>> >> {code}
>> >>      hTableDescriptor = new HTableDescriptor();
>> >>      hTableDescriptor.readFields(fsDataInputStream);
>> >> {code}
>> >> I think we should replace this ctor with the following:
>> >> {code}
>> >> public HTableDescriptor getTableDescriptor(DataInputStream
>> dataInputStream)
>> >> {code}
>> >>
>> >> > HBaseAdmin : Don't modify table's name away
>> >> > -------------------------------------------
>> >> >
>> >> >                 Key: HBASE-4127
>> >> >                 URL: https://issues.apache.org/jira/browse/HBASE-4127
>> >> >             Project: HBase
>> >> >          Issue Type: Bug
>> >> >          Components: client, master
>> >> >    Affects Versions: 0.92.0
>> >> >            Reporter: Nicolas Spiegelberg
>> >> >            Assignee: Nicolas Spiegelberg
>> >> >            Priority: Blocker
>> >> >         Attachments: HBASE-4127.patch
>> >> >
>> >> >
>> >> > One of the developers was using the default constructor for
>> >> HTableDescriptor, which is sadly a bad constructor that should never be
>> >> used. It made the tablename empty in META & caused an ERROR cycle as
>> region
>> >> onlining kept failing. We should have never let this happen. Don't do
>> table
>> >> modifications if the HTableDescriptor name doesn't match the table name
>> >> passed in.
>> >>
>> >> --
>> >> This message is automatically generated by JIRA.
>> >> For more information on JIRA, see:
>> http://www.atlassian.com/software/jira
>> >>
>> >>
>> >>
>> >
>>
>