HBase
  1. HBase
  2. HBASE-3506

Allow table name expressed in regex in drop, disable, enable operations

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      Ability to disable, drop and enable tables using regex expression is desirable.

      1. HBASE-3506-4.patch
        18 kB
        Joey Echeverria
      2. HBASE-3506-3.patch
        17 kB
        Joey Echeverria
      3. HBASE-3506-2.patch
        16 kB
        Joey Echeverria
      4. HBASE-3506-1.patch
        15 kB
        Joey Echeverria

        Activity

        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #1995 (See https://builds.apache.org/job/HBase-TRUNK/1995/)

        Show
        Hudson added a comment - Integrated in HBase-TRUNK #1995 (See https://builds.apache.org/job/HBase-TRUNK/1995/ )
        Hide
        Joey Echeverria added a comment -

        You're welcome, happy to help

        Show
        Joey Echeverria added a comment - You're welcome, happy to help
        Hide
        Ted Yu added a comment -

        Committed to TRUNK.

        Thanks for the nice work Joey.

        Show
        Ted Yu added a comment - Committed to TRUNK. Thanks for the nice work Joey.
        Hide
        Joey Echeverria added a comment -

        I modified the output per Eric's suggestion:

        hbase(main):007:0> drop_all 'test.*'
        test1                                                                           
        test2                                                                           
        
        Drop the above 2 tables (y/n)?
        y
        1 tables successfully dropped
        1 tables not dropped due to an exception: test1
        
        hbase(main):008:0>
        

        I also added the same kind of exception catching to enable since I suppose it could work on some tables, but not others.

        Lastly I rebased the patch as I saw that some changes got checked into trunk.

        Show
        Joey Echeverria added a comment - I modified the output per Eric's suggestion: hbase(main):007:0> drop_all 'test.*' test1 test2 Drop the above 2 tables (y/n)? y 1 tables successfully dropped 1 tables not dropped due to an exception: test1 hbase(main):008:0> I also added the same kind of exception catching to enable since I suppose it could work on some tables, but not others. Lastly I rebased the patch as I saw that some changes got checked into trunk.
        Hide
        Eric Charles added a comment -

        on my previous post: 9 + 2 != 12, sigh... please read 11 (it's early here).

        Show
        Eric Charles added a comment - on my previous post: 9 + 2 != 12, sigh... please read 11 (it's early here).
        Hide
        Eric Charles added a comment -

        To better inform, the message after the question
        Disable the 12 above tables (y/n)?
        y

        ... could be
        9 tables successfully desactivated
        2 tables not desactivated due to exception: Table_E, Table_Y

        Show
        Eric Charles added a comment - To better inform, the message after the question Disable the 12 above tables (y/n)? y ... could be 9 tables successfully desactivated 2 tables not desactivated due to exception: Table_E, Table_Y
        Hide
        Ted Yu added a comment -

        Good to go if tests pass.

        Show
        Ted Yu added a comment - Good to go if tests pass.
        Hide
        Joey Echeverria added a comment -

        Modified the commands to print a list of tables that couldn't be disabled/dropped.

        Show
        Joey Echeverria added a comment - Modified the commands to print a list of tables that couldn't be disabled/dropped.
        Hide
        Joey Echeverria added a comment -

        Good idea, I'll add that.

        Show
        Joey Echeverria added a comment - Good idea, I'll add that.
        Hide
        Ted Yu added a comment -

        What I meant was to display the tables not disabled/dropped so that user can perform follow-on action:

        +    def disable_all(regex)
        +      regex = regex.to_s
        +      @admin.disableTables(regex)
        +    end
        
        Show
        Ted Yu added a comment - What I meant was to display the tables not disabled/dropped so that user can perform follow-on action: + def disable_all(regex) + regex = regex.to_s + @admin.disableTables(regex) + end
        Hide
        Joey Echeverria added a comment -

        Incorporated Ted's feedback about catching errors and returning tables that couldn't be disabled/deleted.

        Show
        Joey Echeverria added a comment - Incorporated Ted's feedback about catching errors and returning tables that couldn't be disabled/deleted.
        Hide
        Joey Echeverria added a comment -

        Thanks Ted. Good ideas all around. I'll work up a new patch.

        Show
        Joey Echeverria added a comment - Thanks Ted. Good ideas all around. I'll work up a new patch.
        Hide
        Ted Yu added a comment -

        Nice work Joey.

        I think we should catch exception(s) coming out of deleteTable() so that an exception in the middle of the operation doesn't stop other tables from being dropped:

        +  public void deleteTables(Pattern pattern) throws IOException {
        +    for (HTableDescriptor table : listTables(pattern)) {
        +      deleteTable(table.getName());
        +    }
        +  }
        

        Also, it would be nice to change the return type to HTableDescriptor[] which contain the table(s) that might have not been deleted.

        Similar comment applies to disableTables().

        Show
        Ted Yu added a comment - Nice work Joey. I think we should catch exception(s) coming out of deleteTable() so that an exception in the middle of the operation doesn't stop other tables from being dropped: + public void deleteTables(Pattern pattern) throws IOException { + for (HTableDescriptor table : listTables(pattern)) { + deleteTable(table.getName()); + } + } Also, it would be nice to change the return type to HTableDescriptor[] which contain the table(s) that might have not been deleted. Similar comment applies to disableTables().
        Hide
        Joey Echeverria added a comment -

        I went with disable_all, drop_all and enable_all:

        hbase(main):001:0> disable_all 'test.*'
        test1                                                                           
        test2                                                                           
        
        Disable the above tables (y/n)?
        y
        0 row(s) in 4.0600 seconds
        
        hbase(main):002:0>
        

        The list command in the shell already supported regular expressions, but I added methods in HBaseAdmin so that the java support is on par with the shell.

        Please review and let me know what you think.

        Show
        Joey Echeverria added a comment - I went with disable_all, drop_all and enable_all: hbase(main):001:0> disable_all 'test.*' test1 test2 Disable the above tables (y/n)? y 0 row(s) in 4.0600 seconds hbase(main):002:0> The list command in the shell already supported regular expressions, but I added methods in HBaseAdmin so that the java support is on par with the shell. Please review and let me know what you think.
        Hide
        Joey Echeverria added a comment -

        Unless there are any objections, I'm going to go with Eric's suggestion and add new commands. I'll also modify list to accept an optional regex, e.g.

        hbase> list 'test.*'
        TABLE                                                                           
        test1                                                                           
        test2                                                                           
        2 row(s) in 0.0140 seconds
        
        Show
        Joey Echeverria added a comment - Unless there are any objections, I'm going to go with Eric's suggestion and add new commands. I'll also modify list to accept an optional regex, e.g. hbase> list 'test.*' TABLE test1 test2 2 row(s) in 0.0140 seconds
        Hide
        Eric Charles added a comment -

        Sounds to me dangerous to have it via the same command names, even with prompt.
        What about using disableRegExp, dropRegExp, enableRegExp?

        Show
        Eric Charles added a comment - Sounds to me dangerous to have it via the same command names, even with prompt. What about using disableRegExp, dropRegExp, enableRegExp?
        Hide
        Joey Echeverria added a comment -

        You could prompt before each table. At the very least there should be a list that accepts a regex.

        Show
        Joey Echeverria added a comment - You could prompt before each table. At the very least there should be a list that accepts a regex.
        Hide
        mingjian added a comment -

        I think it is a very dangerous op. If you want to disable or drop some tables, you can write them in a file and use "hbase shell file"

        e.g:

         
         []$cat drops
         drop 't1'
         drop 't2'
         drop 't3'
         exit
         []$hbase shell drops
         0 row(s) in 1.5880 seconds
         0 row(s) in 1.5880 seconds
         0 row(s) in 1.5880 seconds
        
        Show
        mingjian added a comment - I think it is a very dangerous op. If you want to disable or drop some tables, you can write them in a file and use "hbase shell file" e.g: []$cat drops drop 't1' drop 't2' drop 't3' exit []$hbase shell drops 0 row(s) in 1.5880 seconds 0 row(s) in 1.5880 seconds 0 row(s) in 1.5880 seconds

          People

          • Assignee:
            Joey Echeverria
            Reporter:
            Ted Yu
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development