You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Hiroshi Ikeda (JIRA)" <ji...@apache.org> on 2012/12/05 04:00:58 UTC
[jira] [Created] (HBASE-7278) HTableDesciptor
Hiroshi Ikeda created HBASE-7278:
------------------------------------
Summary: HTableDesciptor
Key: HBASE-7278
URL: https://issues.apache.org/jira/browse/HBASE-7278
Project: HBase
Issue Type: Bug
Reporter: Hiroshi Ikeda
Priority: Minor
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (HBASE-7278) Some bugs of HTableDesciptor
Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7278?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hiroshi Ikeda updated HBASE-7278:
---------------------------------
Summary: Some bugs of HTableDesciptor (was: HTableDesciptor)
> Some bugs of HTableDesciptor
> ----------------------------
>
> Key: HBASE-7278
> URL: https://issues.apache.org/jira/browse/HBASE-7278
> Project: HBase
> Issue Type: Bug
> Reporter: Hiroshi Ikeda
> Priority: Minor
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (HBASE-7278) Some bugs of HTableDesciptor
Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7278?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hiroshi Ikeda updated HBASE-7278:
---------------------------------
Description:
There are some bugs of the class HTableDescriptor.
{code}
public HTableDescriptor(final byte [] name) {
super();
setMetaFlags(this.name);
this.name = this.isMetaRegion()? name: isLegalTableName(name);
this.nameAsString = Bytes.toString(this.name);
}
{code}
I think "setMetaFlags(this.name)" should be "setMetaFlags(name)".
{code}
/**
* Check passed byte buffer, "tableName", is legal user-space table name.
* @return Returns passed <code>tableName</code> param
* @throws NullPointerException If passed <code>tableName</code> is null
* @throws IllegalArgumentException if passed a tableName
* that is made of other than 'word' characters or underscores: i.e.
* <code>[a-zA-Z_0-9].
*/
public static byte [] isLegalTableName(final byte [] tableName) {
if (tableName == null || tableName.length <= 0) {
throw new IllegalArgumentException("Name is null or empty");
}
{code}
The implementation is against the contract of throwing NullPointerException.
I'm not sure the contract is wrong or the implementation is wrong.
Also the contract of throwing IllegalArgumentException is a little different from the actual implementation, and in general we must actually call this method and catch IllegalArgumentException in order to know whether the given name can be used as a table name.
I feel HTableDescriptor allows itself to be in invalid states, and I cannot fix the class well.
I think we should start to remove implementing WritableComparable, but it might greatly break the compatibility.
> Some bugs of HTableDesciptor
> ----------------------------
>
> Key: HBASE-7278
> URL: https://issues.apache.org/jira/browse/HBASE-7278
> Project: HBase
> Issue Type: Bug
> Reporter: Hiroshi Ikeda
> Priority: Minor
>
> There are some bugs of the class HTableDescriptor.
> {code}
> public HTableDescriptor(final byte [] name) {
> super();
> setMetaFlags(this.name);
> this.name = this.isMetaRegion()? name: isLegalTableName(name);
> this.nameAsString = Bytes.toString(this.name);
> }
> {code}
> I think "setMetaFlags(this.name)" should be "setMetaFlags(name)".
> {code}
> /**
> * Check passed byte buffer, "tableName", is legal user-space table name.
> * @return Returns passed <code>tableName</code> param
> * @throws NullPointerException If passed <code>tableName</code> is null
> * @throws IllegalArgumentException if passed a tableName
> * that is made of other than 'word' characters or underscores: i.e.
> * <code>[a-zA-Z_0-9].
> */
> public static byte [] isLegalTableName(final byte [] tableName) {
> if (tableName == null || tableName.length <= 0) {
> throw new IllegalArgumentException("Name is null or empty");
> }
> {code}
> The implementation is against the contract of throwing NullPointerException.
> I'm not sure the contract is wrong or the implementation is wrong.
> Also the contract of throwing IllegalArgumentException is a little different from the actual implementation, and in general we must actually call this method and catch IllegalArgumentException in order to know whether the given name can be used as a table name.
> I feel HTableDescriptor allows itself to be in invalid states, and I cannot fix the class well.
> I think we should start to remove implementing WritableComparable, but it might greatly break the compatibility.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-7278) Some bugs of HTableDesciptor
Posted by "stack (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7278?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13510289#comment-13510289 ]
stack commented on HBASE-7278:
------------------------------
We should remove setName. It makes not sense doing a setTable name on an HTD post construction I'd say.
> Some bugs of HTableDesciptor
> ----------------------------
>
> Key: HBASE-7278
> URL: https://issues.apache.org/jira/browse/HBASE-7278
> Project: HBase
> Issue Type: Bug
> Reporter: Hiroshi Ikeda
> Priority: Minor
>
> There are some bugs of the class HTableDescriptor.
> {code}
> public HTableDescriptor(final byte [] name) {
> super();
> setMetaFlags(this.name);
> this.name = this.isMetaRegion()? name: isLegalTableName(name);
> this.nameAsString = Bytes.toString(this.name);
> }
> {code}
> I think "setMetaFlags(this.name)" should be "setMetaFlags(name)".
> {code}
> /**
> * Check passed byte buffer, "tableName", is legal user-space table name.
> * @return Returns passed <code>tableName</code> param
> * @throws NullPointerException If passed <code>tableName</code> is null
> * @throws IllegalArgumentException if passed a tableName
> * that is made of other than 'word' characters or underscores: i.e.
> * <code>[a-zA-Z_0-9].
> */
> public static byte [] isLegalTableName(final byte [] tableName) {
> if (tableName == null || tableName.length <= 0) {
> throw new IllegalArgumentException("Name is null or empty");
> }
> {code}
> The implementation is against the contract of throwing NullPointerException.
> I'm not sure the contract is wrong or the implementation is wrong.
> Also the contract of throwing IllegalArgumentException is a little different from the actual implementation, and in general we must actually call this method and catch IllegalArgumentException in order to know whether the given name can be used as a table name.
> I feel HTableDescriptor allows itself to be in invalid states, and I cannot fix the class well.
> I think we should start to remove implementing WritableComparable, but it might greatly break the compatibility.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-7278) Some bugs of HTableDesciptor
Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7278?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13510272#comment-13510272 ]
Hiroshi Ikeda commented on HBASE-7278:
--------------------------------------
The constructors HTableDescriptor(String name) and HTableDescriptor(byte[] name) reject invalid user-space table name, but you can set any table name via setName(byte[] name). That seems a bug, but I'm not sure whether this class only uses the tables which are acceptable by the constructors and we can change the spec of setName().
> Some bugs of HTableDesciptor
> ----------------------------
>
> Key: HBASE-7278
> URL: https://issues.apache.org/jira/browse/HBASE-7278
> Project: HBase
> Issue Type: Bug
> Reporter: Hiroshi Ikeda
> Priority: Minor
>
> There are some bugs of the class HTableDescriptor.
> {code}
> public HTableDescriptor(final byte [] name) {
> super();
> setMetaFlags(this.name);
> this.name = this.isMetaRegion()? name: isLegalTableName(name);
> this.nameAsString = Bytes.toString(this.name);
> }
> {code}
> I think "setMetaFlags(this.name)" should be "setMetaFlags(name)".
> {code}
> /**
> * Check passed byte buffer, "tableName", is legal user-space table name.
> * @return Returns passed <code>tableName</code> param
> * @throws NullPointerException If passed <code>tableName</code> is null
> * @throws IllegalArgumentException if passed a tableName
> * that is made of other than 'word' characters or underscores: i.e.
> * <code>[a-zA-Z_0-9].
> */
> public static byte [] isLegalTableName(final byte [] tableName) {
> if (tableName == null || tableName.length <= 0) {
> throw new IllegalArgumentException("Name is null or empty");
> }
> {code}
> The implementation is against the contract of throwing NullPointerException.
> I'm not sure the contract is wrong or the implementation is wrong.
> Also the contract of throwing IllegalArgumentException is a little different from the actual implementation, and in general we must actually call this method and catch IllegalArgumentException in order to know whether the given name can be used as a table name.
> I feel HTableDescriptor allows itself to be in invalid states, and I cannot fix the class well.
> I think we should start to remove implementing WritableComparable, but it might greatly break the compatibility.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-7278) Some bugs of HTableDesciptor
Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7278?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13510263#comment-13510263 ]
Hiroshi Ikeda commented on HBASE-7278:
--------------------------------------
This class meaninglessly uses synchronized and volatile. But removing them might wake up potential bugs somewhere.
Some of the methods return unmodifiable collection views, while the methods' contracts say the return collection is immutable.
I don't sure which is wrong, the contract or the implementation.
In order to keep the compatibility, we should keep the contract and create a new collection and wrap it by Collections.unmodifiable*.
> Some bugs of HTableDesciptor
> ----------------------------
>
> Key: HBASE-7278
> URL: https://issues.apache.org/jira/browse/HBASE-7278
> Project: HBase
> Issue Type: Bug
> Reporter: Hiroshi Ikeda
> Priority: Minor
>
> There are some bugs of the class HTableDescriptor.
> {code}
> public HTableDescriptor(final byte [] name) {
> super();
> setMetaFlags(this.name);
> this.name = this.isMetaRegion()? name: isLegalTableName(name);
> this.nameAsString = Bytes.toString(this.name);
> }
> {code}
> I think "setMetaFlags(this.name)" should be "setMetaFlags(name)".
> {code}
> /**
> * Check passed byte buffer, "tableName", is legal user-space table name.
> * @return Returns passed <code>tableName</code> param
> * @throws NullPointerException If passed <code>tableName</code> is null
> * @throws IllegalArgumentException if passed a tableName
> * that is made of other than 'word' characters or underscores: i.e.
> * <code>[a-zA-Z_0-9].
> */
> public static byte [] isLegalTableName(final byte [] tableName) {
> if (tableName == null || tableName.length <= 0) {
> throw new IllegalArgumentException("Name is null or empty");
> }
> {code}
> The implementation is against the contract of throwing NullPointerException.
> I'm not sure the contract is wrong or the implementation is wrong.
> Also the contract of throwing IllegalArgumentException is a little different from the actual implementation, and in general we must actually call this method and catch IllegalArgumentException in order to know whether the given name can be used as a table name.
> I feel HTableDescriptor allows itself to be in invalid states, and I cannot fix the class well.
> I think we should start to remove implementing WritableComparable, but it might greatly break the compatibility.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-7278) Some bugs of HTableDesciptor
Posted by "stack (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7278?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13510268#comment-13510268 ]
stack commented on HBASE-7278:
------------------------------
Thanks for the review Hiroshi. These look like legit issues that you've turned up. Thanks.
bq. I think we should start to remove implementing WritableComparable, but it might greatly break the compatibility.
That is ok for trunk.
We are moving to protobuf and stripping Writables in trunk (including WritableComparable which maybe just becomes Comparable? Depends on context).
> Some bugs of HTableDesciptor
> ----------------------------
>
> Key: HBASE-7278
> URL: https://issues.apache.org/jira/browse/HBASE-7278
> Project: HBase
> Issue Type: Bug
> Reporter: Hiroshi Ikeda
> Priority: Minor
>
> There are some bugs of the class HTableDescriptor.
> {code}
> public HTableDescriptor(final byte [] name) {
> super();
> setMetaFlags(this.name);
> this.name = this.isMetaRegion()? name: isLegalTableName(name);
> this.nameAsString = Bytes.toString(this.name);
> }
> {code}
> I think "setMetaFlags(this.name)" should be "setMetaFlags(name)".
> {code}
> /**
> * Check passed byte buffer, "tableName", is legal user-space table name.
> * @return Returns passed <code>tableName</code> param
> * @throws NullPointerException If passed <code>tableName</code> is null
> * @throws IllegalArgumentException if passed a tableName
> * that is made of other than 'word' characters or underscores: i.e.
> * <code>[a-zA-Z_0-9].
> */
> public static byte [] isLegalTableName(final byte [] tableName) {
> if (tableName == null || tableName.length <= 0) {
> throw new IllegalArgumentException("Name is null or empty");
> }
> {code}
> The implementation is against the contract of throwing NullPointerException.
> I'm not sure the contract is wrong or the implementation is wrong.
> Also the contract of throwing IllegalArgumentException is a little different from the actual implementation, and in general we must actually call this method and catch IllegalArgumentException in order to know whether the given name can be used as a table name.
> I feel HTableDescriptor allows itself to be in invalid states, and I cannot fix the class well.
> I think we should start to remove implementing WritableComparable, but it might greatly break the compatibility.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira