You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2020/01/03 20:47:31 UTC

[GitHub] [incubator-hudi] zhedoubushishi opened a new pull request #1175: [HUDI-495] Update deprecated HBase API

zhedoubushishi opened a new pull request #1175: [HUDI-495] Update deprecated HBase API
URL: https://github.com/apache/incubator-hudi/pull/1175
 
 
   ## *Tips*
   - *Thank you very much for contributing to Apache Hudi.*
   - *Please review https://hudi.apache.org/contributing.html before opening a pull request.*
   
   ## What is the purpose of the pull request
   Jira: https://jira.apache.org/jira/browse/HUDI-495
   
   Internally we are using HBase 2.x, and HBase 2.x no longer supports ```Htable.flushCommits()``` and it is replaced by ```BufferedMutator.flush()```.
   Thus for put operation and delete operation, we can use BufferedMutator instead.
   
   ## Brief change log
   
     - *Replace HTable with BufferedMutator*
   
   ## Verify this pull request
   
   This pull request is already covered by existing tests, such as *(please describe tests)*.
   
   ## Committer checklist
   
    - [x] Has a corresponding JIRA in PR title & commit
    
    - [ ] Commit message is descriptive of the change
    
    - [ ] CI is green
   
    - [ ] Necessary doc changes done or have another open PR
          
    - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] n3nash commented on a change in pull request #1175: [HUDI-495] Update deprecated HBase API

Posted by GitBox <gi...@apache.org>.
n3nash commented on a change in pull request #1175: [HUDI-495] Update deprecated HBase API
URL: https://github.com/apache/incubator-hudi/pull/1175#discussion_r362996436
 
 

 ##########
 File path: hudi-client/src/main/java/org/apache/hudi/index/hbase/HBaseIndex.java
 ##########
 @@ -287,13 +289,10 @@ private boolean checkIfValidCommit(HoodieTableMetaClient metaClient, String comm
           hbaseConnection = getHBaseConnection();
         }
       }
-      HTable hTable = null;
-      try {
-        hTable = (HTable) hbaseConnection.getTable(TableName.valueOf(tableName));
+      try (BufferedMutator mutator = hbaseConnection.getBufferedMutator(TableName.valueOf(tableName))) {
 
 Review comment:
   Could you comment with a small snippet of the doc mentioning this is the new API ?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] zhedoubushishi commented on a change in pull request #1175: [HUDI-495] Update deprecated HBase API

Posted by GitBox <gi...@apache.org>.
zhedoubushishi commented on a change in pull request #1175: [HUDI-495] Update deprecated HBase API
URL: https://github.com/apache/incubator-hudi/pull/1175#discussion_r363919247
 
 

 ##########
 File path: hudi-client/src/main/java/org/apache/hudi/index/hbase/HBaseIndex.java
 ##########
 @@ -287,13 +289,10 @@ private boolean checkIfValidCommit(HoodieTableMetaClient metaClient, String comm
           hbaseConnection = getHBaseConnection();
         }
       }
-      HTable hTable = null;
-      try {
-        hTable = (HTable) hbaseConnection.getTable(TableName.valueOf(tableName));
+      try (BufferedMutator mutator = hbaseConnection.getBufferedMutator(TableName.valueOf(tableName))) {
 
 Review comment:
   Yes I think so. 
   The HTable API doc shows:
   ```
   flushCommits()
   Deprecated. 
   as of 1.0.0. Replaced by BufferedMutator.flush()
   ```
   Also HTable is not threadsafe. (https://issues.apache.org/jira/browse/HBASE-17361)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] n3nash merged pull request #1175: [HUDI-495] Update deprecated HBase API

Posted by GitBox <gi...@apache.org>.
n3nash merged pull request #1175: [HUDI-495] Update deprecated HBase API
URL: https://github.com/apache/incubator-hudi/pull/1175
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] n3nash commented on a change in pull request #1175: [HUDI-495] Update deprecated HBase API

Posted by GitBox <gi...@apache.org>.
n3nash commented on a change in pull request #1175: [HUDI-495] Update deprecated HBase API
URL: https://github.com/apache/incubator-hudi/pull/1175#discussion_r364376351
 
 

 ##########
 File path: hudi-client/src/main/java/org/apache/hudi/index/hbase/HBaseIndex.java
 ##########
 @@ -287,13 +289,10 @@ private boolean checkIfValidCommit(HoodieTableMetaClient metaClient, String comm
           hbaseConnection = getHBaseConnection();
         }
       }
-      HTable hTable = null;
-      try {
-        hTable = (HTable) hbaseConnection.getTable(TableName.valueOf(tableName));
+      try (BufferedMutator mutator = hbaseConnection.getBufferedMutator(TableName.valueOf(tableName))) {
 
 Review comment:
   Ok great, thanks!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] zhedoubushishi commented on a change in pull request #1175: [HUDI-495] Update deprecated HBase API

Posted by GitBox <gi...@apache.org>.
zhedoubushishi commented on a change in pull request #1175: [HUDI-495] Update deprecated HBase API
URL: https://github.com/apache/incubator-hudi/pull/1175#discussion_r363919247
 
 

 ##########
 File path: hudi-client/src/main/java/org/apache/hudi/index/hbase/HBaseIndex.java
 ##########
 @@ -287,13 +289,10 @@ private boolean checkIfValidCommit(HoodieTableMetaClient metaClient, String comm
           hbaseConnection = getHBaseConnection();
         }
       }
-      HTable hTable = null;
-      try {
-        hTable = (HTable) hbaseConnection.getTable(TableName.valueOf(tableName));
+      try (BufferedMutator mutator = hbaseConnection.getBufferedMutator(TableName.valueOf(tableName))) {
 
 Review comment:
   Yes I think so. 
   The HBase API doc shows:
   ```
   flushCommits()
   Deprecated. 
   as of 1.0.0. Replaced by BufferedMutator.flush()
   ```
   Also HTable is not threadsafe. (https://issues.apache.org/jira/browse/HBASE-17361)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] n3nash commented on a change in pull request #1175: [HUDI-495] Update deprecated HBase API

Posted by GitBox <gi...@apache.org>.
n3nash commented on a change in pull request #1175: [HUDI-495] Update deprecated HBase API
URL: https://github.com/apache/incubator-hudi/pull/1175#discussion_r363548127
 
 

 ##########
 File path: hudi-client/src/main/java/org/apache/hudi/index/hbase/HBaseIndex.java
 ##########
 @@ -287,13 +289,10 @@ private boolean checkIfValidCommit(HoodieTableMetaClient metaClient, String comm
           hbaseConnection = getHBaseConnection();
         }
       }
-      HTable hTable = null;
-      try {
-        hTable = (HTable) hbaseConnection.getTable(TableName.valueOf(tableName));
+      try (BufferedMutator mutator = hbaseConnection.getBufferedMutator(TableName.valueOf(tableName))) {
 
 Review comment:
   Thanks for providing the link, so is the `BufferedMutator` from Hbase also the preferred new methodology ? (The above link tells me that from the JDBC driver changes are fair)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] zhedoubushishi commented on a change in pull request #1175: [HUDI-495] Update deprecated HBase API

Posted by GitBox <gi...@apache.org>.
zhedoubushishi commented on a change in pull request #1175: [HUDI-495] Update deprecated HBase API
URL: https://github.com/apache/incubator-hudi/pull/1175#discussion_r362999490
 
 

 ##########
 File path: hudi-client/src/main/java/org/apache/hudi/index/hbase/HBaseIndex.java
 ##########
 @@ -287,13 +289,10 @@ private boolean checkIfValidCommit(HoodieTableMetaClient metaClient, String comm
           hbaseConnection = getHBaseConnection();
         }
       }
-      HTable hTable = null;
-      try {
-        hTable = (HTable) hbaseConnection.getTable(TableName.valueOf(tableName));
+      try (BufferedMutator mutator = hbaseConnection.getBufferedMutator(TableName.valueOf(tableName))) {
 
 Review comment:
   Sure. Here is the doc: https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html.
   ```
   The try-with-resources statement is a try statement that declares one or more resources.
   A resource is an object that must be closed after the program is finished with it.
   The try-with-resources statement ensures that each resource is closed at the end of the statement.
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] zhedoubushishi commented on issue #1175: [HUDI-495] Update deprecated HBase API

Posted by GitBox <gi...@apache.org>.
zhedoubushishi commented on issue #1175: [HUDI-495] Update deprecated HBase API
URL: https://github.com/apache/incubator-hudi/pull/1175#issuecomment-571788946
 
 
   I think in our cases doMutations method will always do flush right after doing mutator.mutate(...). So each time you call doMutations, at first the buffer should be empty. Therefore, if the mutations array is empty, we can just return without any flush operation.
   
   That why I rewrite it like this:
   ```
   if (mutations.isEmpty()) {
     return;
   }
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services