You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/03/03 15:24:02 UTC
[GitHub] [hive] marton-bod opened a new pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
marton-bod opened a new pull request #3076:
URL: https://github.com/apache/hive/pull/3076
… the underlying HBase table
### What changes were proposed in this pull request?
Introduce a CTLT flag to check if a table was created using CTLT, the same way it's done for CTAS.
### Why are the changes needed?
In the case of HBase tables, if you use a CTLT command, your new table will point to the same underlying HBase table as your original table. In case of a failure, currently the HBaseMetaHook rollback logic deletes the HBase table, leading to data loss. However, using the CTLT flag, we can stop that from doing so.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Unit tests
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3076:
URL: https://github.com/apache/hive/pull/3076#discussion_r822387571
##########
File path: hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestHBaseQueries.java
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.hbase;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import org.apache.hadoop.hbase.HBaseConfiguration;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.MiniHBaseCluster;
+import org.apache.hadoop.hbase.zookeeper.MiniZooKeeperCluster;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.ql.DriverFactory;
+import org.apache.hadoop.hive.ql.IDriver;
+import org.apache.hadoop.hive.ql.processors.CommandProcessorException;
+import org.apache.hadoop.hive.ql.security.authorization.plugin.sqlstd.SQLStdHiveAuthorizerFactory;
+import org.apache.hadoop.hive.ql.session.SessionState;
+import org.apache.hadoop.hive.metastore.utils.TestTxnDbUtil;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestHBaseQueries {
+
+ private static MiniZooKeeperCluster zooKeeperCluster;
+ private static MiniHBaseCluster miniHBaseCluster;
+ private final HiveConf baseConf;
+ private IDriver driver;
+
+ /**
+ * Test class for running queries using HBase tables. Creates a mini ZK and an HBase test cluster.
+ * Each test method must instantiate a driver object first, using either the baseConf or a modified version of that.
+ * Once finished, each test method must take care of cleaning up any database objects they've created (tables,
Review comment:
Is there an easy way to do this in the before/after methods?
Not super important, as we do not have multiple tests ATM, just asking
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3076:
URL: https://github.com/apache/hive/pull/3076#discussion_r822388545
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/formatter/TextDescTableFormatter.java
##########
@@ -390,7 +391,7 @@ private void displayAllParameters(Map<String, String> params, StringBuilder tabl
Collections.sort(keys);
for (String key : keys) {
String value = params.get(key);
- if (TABLE_IS_CTAS.equals(key)) {
+ if (TABLE_IS_CTAS.equals(key) || TABLE_IS_CTLT.equals(key)) {
Review comment:
Does this mean that we litter the TABLE_PARAMS with this keys? Are they remaining after the table creation?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] marton-bod commented on pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
Posted by GitBox <gi...@apache.org>.
marton-bod commented on pull request #3076:
URL: https://github.com/apache/hive/pull/3076#issuecomment-1060754957
@nareshpr Thanks Naresh for the reminder! This file does not exist upstream, but I'll make sure to update it when doing the downstream backport.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] marton-bod commented on pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
Posted by GitBox <gi...@apache.org>.
marton-bod commented on pull request #3076:
URL: https://github.com/apache/hive/pull/3076#issuecomment-1062037297
@pvary Can you please review this patch? 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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] marton-bod commented on a change in pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #3076:
URL: https://github.com/apache/hive/pull/3076#discussion_r822577828
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/formatter/TextDescTableFormatter.java
##########
@@ -390,7 +391,7 @@ private void displayAllParameters(Map<String, String> params, StringBuilder tabl
Collections.sort(keys);
for (String key : keys) {
String value = params.get(key);
- if (TABLE_IS_CTAS.equals(key)) {
+ if (TABLE_IS_CTAS.equals(key) || TABLE_IS_CTLT.equals(key)) {
Review comment:
Yeah, I removed changes to all the friends too :)
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3076:
URL: https://github.com/apache/hive/pull/3076#discussion_r822562227
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/formatter/TextDescTableFormatter.java
##########
@@ -390,7 +391,7 @@ private void displayAllParameters(Map<String, String> params, StringBuilder tabl
Collections.sort(keys);
for (String key : keys) {
String value = params.get(key);
- if (TABLE_IS_CTAS.equals(key)) {
+ if (TABLE_IS_CTAS.equals(key) || TABLE_IS_CTLT.equals(key)) {
Review comment:
and even some more changes, like the formatted output and friends?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] marton-bod commented on a change in pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #3076:
URL: https://github.com/apache/hive/pull/3076#discussion_r822560228
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/formatter/TextDescTableFormatter.java
##########
@@ -390,7 +391,7 @@ private void displayAllParameters(Map<String, String> params, StringBuilder tabl
Collections.sort(keys);
for (String key : keys) {
String value = params.get(key);
- if (TABLE_IS_CTAS.equals(key)) {
+ if (TABLE_IS_CTAS.equals(key) || TABLE_IS_CTLT.equals(key)) {
Review comment:
Sure
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] marton-bod commented on a change in pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #3076:
URL: https://github.com/apache/hive/pull/3076#discussion_r822525523
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/formatter/TextDescTableFormatter.java
##########
@@ -390,7 +391,7 @@ private void displayAllParameters(Map<String, String> params, StringBuilder tabl
Collections.sort(keys);
for (String key : keys) {
String value = params.get(key);
- if (TABLE_IS_CTAS.equals(key)) {
+ if (TABLE_IS_CTAS.equals(key) || TABLE_IS_CTLT.equals(key)) {
Review comment:
Yes, currently that's the case. In order to have the table property available during the MetaHook#rollback, it's gotta be part of the Table object. It's just filtered out during describe commands and such but the value is there in the props. Do you think that's a dealbreaker?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] marton-bod merged pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
Posted by GitBox <gi...@apache.org>.
marton-bod merged pull request #3076:
URL: https://github.com/apache/hive/pull/3076
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] nareshpr commented on pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
Posted by GitBox <gi...@apache.org>.
nareshpr commented on pull request #3076:
URL: https://github.com/apache/hive/pull/3076#issuecomment-1059591262
Please update versionmap.txt as thrift is getting updated.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] marton-bod commented on a change in pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #3076:
URL: https://github.com/apache/hive/pull/3076#discussion_r822556400
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/formatter/TextDescTableFormatter.java
##########
@@ -390,7 +391,7 @@ private void displayAllParameters(Map<String, String> params, StringBuilder tabl
Collections.sort(keys);
for (String key : keys) {
String value = params.get(key);
- if (TABLE_IS_CTAS.equals(key)) {
+ if (TABLE_IS_CTAS.equals(key) || TABLE_IS_CTLT.equals(key)) {
Review comment:
Actually, I've refactored it so that we don't persist the CTLT flag into the table props.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3076:
URL: https://github.com/apache/hive/pull/3076#discussion_r822557579
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/formatter/TextDescTableFormatter.java
##########
@@ -390,7 +391,7 @@ private void displayAllParameters(Map<String, String> params, StringBuilder tabl
Collections.sort(keys);
for (String key : keys) {
String value = params.get(key);
- if (TABLE_IS_CTAS.equals(key)) {
+ if (TABLE_IS_CTAS.equals(key) || TABLE_IS_CTLT.equals(key)) {
Review comment:
Can we revert this change then?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3076:
URL: https://github.com/apache/hive/pull/3076#discussion_r822388980
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/show/properties/ShowTablePropertiesOperation.java
##########
@@ -67,8 +69,10 @@ public int execute() throws HiveException {
} else {
Map<String, String> properties = new TreeMap<String, String>(tbl.getParameters());
for (Entry<String, String> entry : properties.entrySet()) {
- ShowUtils.appendNonNull(builder, entry.getKey(), true);
- ShowUtils.appendNonNull(builder, entry.getValue());
+ if (!entry.getKey().equals(TABLE_IS_CTLT)) {
Review comment:
Why `TABLE_IS_CTAS` is not checked here?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] marton-bod commented on a change in pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #3076:
URL: https://github.com/apache/hive/pull/3076#discussion_r822523268
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/show/properties/ShowTablePropertiesOperation.java
##########
@@ -67,8 +69,10 @@ public int execute() throws HiveException {
} else {
Map<String, String> properties = new TreeMap<String, String>(tbl.getParameters());
for (Entry<String, String> entry : properties.entrySet()) {
- ShowUtils.appendNonNull(builder, entry.getKey(), true);
- ShowUtils.appendNonNull(builder, entry.getValue());
+ if (!entry.getKey().equals(TABLE_IS_CTLT)) {
Review comment:
Good question. I think it was just an omission. I'll add that in here too
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] marton-bod commented on a change in pull request #3076: HIVE-25989: HBaseStorageHandler CTLT failure rollback should not drop…
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #3076:
URL: https://github.com/apache/hive/pull/3076#discussion_r822520656
##########
File path: hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestHBaseQueries.java
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.hbase;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import org.apache.hadoop.hbase.HBaseConfiguration;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.MiniHBaseCluster;
+import org.apache.hadoop.hbase.zookeeper.MiniZooKeeperCluster;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.ql.DriverFactory;
+import org.apache.hadoop.hive.ql.IDriver;
+import org.apache.hadoop.hive.ql.processors.CommandProcessorException;
+import org.apache.hadoop.hive.ql.security.authorization.plugin.sqlstd.SQLStdHiveAuthorizerFactory;
+import org.apache.hadoop.hive.ql.session.SessionState;
+import org.apache.hadoop.hive.metastore.utils.TestTxnDbUtil;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestHBaseQueries {
+
+ private static MiniZooKeeperCluster zooKeeperCluster;
+ private static MiniHBaseCluster miniHBaseCluster;
+ private final HiveConf baseConf;
+ private IDriver driver;
+
+ /**
+ * Test class for running queries using HBase tables. Creates a mini ZK and an HBase test cluster.
+ * Each test method must instantiate a driver object first, using either the baseConf or a modified version of that.
+ * Once finished, each test method must take care of cleaning up any database objects they've created (tables,
Review comment:
yes that would be ideal, but I did not find a good way to do that unfortunately
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org