You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2019/05/22 18:31:12 UTC
[kudu] 02/03: Allow alter legacy tables in Hive Metastore Kudu
plugin
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit b36ea67d33ab7098d79d8bfb4e57e1308eee983d
Author: Hao Hao <ha...@cloudera.com>
AuthorDate: Sun May 12 19:51:34 2019 -0700
Allow alter legacy tables in Hive Metastore Kudu plugin
Currently, the Hive metastore plugin disallows schema alteration for
tables with legacy storage handler. Since the plugin is intended to
work with both before and post Kudu/HMS integration context, this patch
ensures creation/alteration/deletion on legacy tables works as expected.
Change-Id: Ie76ce2cd7dda0b4391e91abe2da2801d305a64d3
Reviewed-on: http://gerrit.cloudera.org:8080/13317
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
.../kudu/hive/metastore/KuduMetastorePlugin.java | 76 +++++++++++++++-------
.../hive/metastore/TestKuduMetastorePlugin.java | 55 +++++++++++-----
2 files changed, 91 insertions(+), 40 deletions(-)
diff --git a/java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java b/java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java
index d41497a..aaae67c 100644
--- a/java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java
+++ b/java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java
@@ -46,15 +46,19 @@ import org.apache.hadoop.hive.metastore.events.ListenerEvent;
* }
* </pre>
*
- * The plugin enforces that Kudu table entries in the HMS always
- * contain two properties: a Kudu table ID and the Kudu master addresses. It also
- * enforces that non-Kudu tables do not have these properties. The plugin
- * considers entries to be Kudu tables if they contain the Kudu storage handler.
+ * The plugin enforces that Kudu table entries in the HMS always contain
+ * two properties: a Kudu table ID and the Kudu master addresses. It also
+ * enforces that non-Kudu tables do not have these properties (except cases
+ * when upgrading tables with legacy Kudu storage handler to be Kudu tables
+ * or downgrading from the other way around). The plugin considers entries
+ * to be Kudu tables if they contain the Kudu storage handler.
*
* Additionally, the plugin checks that when particular events have an
* environment containing a Kudu table ID, that event only applies
- * to the specified Kudu table. This provides some amount of concurrency safety,
- * so that the Kudu Master can ensure it is operating on the correct table entry.
+ * to the specified Kudu table. This provides some amount of concurrency
+ * safety, so that the Kudu Master can ensure it is operating on the correct
+ * table entry. Note that such validation does not apply to tables with
+ * legacy Kudu storage handler.
*/
public class KuduMetastorePlugin extends MetaStoreEventListener {
@@ -130,31 +134,41 @@ public class KuduMetastorePlugin extends MetaStoreEventListener {
Table oldTable = tableEvent.getOldTable();
Table newTable = tableEvent.getNewTable();
- // Allow non-Kudu tables (even the legacy ones) to be altered.
- if (!isKuduTable(oldTable) && !isLegacyKuduTable(oldTable)) {
- // But ensure that the alteration isn't introducing Kudu-specific properties.
+ if (isLegacyKuduTable(oldTable)) {
+ if (isKuduTable(newTable)) {
+ // Allow legacy tables to be upgraded to Kudu tables. Validate the upgraded
+ // table entry contains the required Kudu table properties, and that any
+ // potential schema alterations are coming from the Kudu master.
+ checkKuduProperties(newTable);
+ checkOnlyKuduMasterCanAlterSchema(tableEvent, oldTable, newTable);
+ return;
+ }
+ // Allow legacy tables to be altered without introducing Kudu-specific
+ // properties.
checkNoKuduProperties(newTable);
- return;
- }
-
- // Check the altered table's properties. Kudu tables can be downgraded
- // to the legacy format.
- if (!isLegacyKuduTable(newTable)) {
+ } else if (isKuduTable(oldTable)) {
+ if (isLegacyKuduTable(newTable)) {
+ // Allow Kudu tables to be downgraded to legacy tables. Validate the downgraded
+ // table entry does not contain Kudu-specific properties, and that any potential
+ // schema alterations are coming from the Kudu master.
+ checkNoKuduProperties(newTable);
+ checkOnlyKuduMasterCanAlterSchema(tableEvent, oldTable, newTable);
+ return;
+ }
+ // Validate the new table entry contains the required Kudu table properties, and
+ // that any potential schema alterations are coming from the Kudu master.
checkKuduProperties(newTable);
- }
-
- // Check that the non legacy Kudu table ID isn't changing.
- if (!isLegacyKuduTable(oldTable) && !isLegacyKuduTable(newTable)) {
+ checkOnlyKuduMasterCanAlterSchema(tableEvent, oldTable, newTable);
+ // Check that the Kudu table ID isn't changing.
String oldTableId = oldTable.getParameters().get(KUDU_TABLE_ID_KEY);
String newTableId = newTable.getParameters().get(KUDU_TABLE_ID_KEY);
if (!newTableId.equals(oldTableId)) {
throw new MetaException("Kudu table ID does not match the existing HMS entry");
}
- }
-
- if (!isKuduMasterAction(tableEvent) &&
- !oldTable.getSd().getCols().equals(newTable.getSd().getCols())) {
- throw new MetaException("Kudu table columns may not be altered through Hive");
+ } else {
+ // Allow non-Kudu tables to be altered without introducing Kudu-specific
+ // properties.
+ checkNoKuduProperties(newTable);
}
}
@@ -223,6 +237,20 @@ public class KuduMetastorePlugin extends MetaStoreEventListener {
}
/**
+ * Checks that the table schema can only be altered by an action from the Kudu Master.
+ * @param tableEvent
+ * @param oldTable the table to be altered
+ * @param newTable the new altered table
+ */
+ private void checkOnlyKuduMasterCanAlterSchema(AlterTableEvent tableEvent,
+ Table oldTable, Table newTable) throws MetaException {
+ if (!isKuduMasterAction(tableEvent) &&
+ !oldTable.getSd().getCols().equals(newTable.getSd().getCols())) {
+ throw new MetaException("Kudu table columns may not be altered through Hive");
+ }
+ }
+
+ /**
* Returns true if the event is from the Kudu Master.
*/
private boolean isKuduMasterAction(ListenerEvent event) {
diff --git a/java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java b/java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java
index 6bafa4a..27ea56e 100644
--- a/java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java
+++ b/java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java
@@ -254,6 +254,20 @@ public class TestKuduMetastorePlugin {
client.alter_table_with_environmentContext(
table.getDbName(), table.getTableName(), table,
new EnvironmentContext(ImmutableMap.of(KuduMetastorePlugin.KUDU_MASTER_EVENT, "true")));
+
+ // Check that altering table with Kudu storage handler to legacy format
+ // succeeds.
+ {
+ Table alteredTable = table.deepCopy();
+ alteredTable.getParameters().clear();
+ alteredTable.putToParameters(hive_metastoreConstants.META_TABLE_STORAGE,
+ KuduMetastorePlugin.LEGACY_KUDU_STORAGE_HANDLER);
+ alteredTable.putToParameters(KuduMetastorePlugin.LEGACY_KUDU_TABLE_NAME,
+ "legacy_table");
+ alteredTable.putToParameters(KuduMetastorePlugin.KUDU_MASTER_ADDRS_KEY,
+ "localhost");
+ client.alter_table(table.getDbName(), table.getTableName(), alteredTable);
+ }
} finally {
client.dropTable(table.getDbName(), table.getTableName());
}
@@ -290,7 +304,7 @@ public class TestKuduMetastorePlugin {
// Check that altering the table succeeds.
client.alter_table(table.getDbName(), table.getTableName(), table);
- // Check that altering the legacy table with Kudu storage handler
+ // Check that altering the legacy table to use the Kudu storage handler
// succeeds.
{
Table alteredTable = legacyTable.deepCopy();
@@ -303,27 +317,36 @@ public class TestKuduMetastorePlugin {
client.alter_table(legacyTable.getDbName(), legacyTable.getTableName(),
alteredTable);
}
-
- // Check that altering table with Kudu storage handler to legacy format
- // succeeds.
- {
- Table alteredTable = table.deepCopy();
- alteredTable.putToParameters(hive_metastoreConstants.META_TABLE_STORAGE,
- KuduMetastorePlugin.LEGACY_KUDU_STORAGE_HANDLER);
- alteredTable.putToParameters(KuduMetastorePlugin.LEGACY_KUDU_TABLE_NAME,
- "legacy_table");
- alteredTable.putToParameters(KuduMetastorePlugin.KUDU_MASTER_ADDRS_KEY,
- "localhost");
- client.alter_table(table.getDbName(), table.getTableName(),
- alteredTable);
- }
-
} finally {
client.dropTable(table.getDbName(), table.getTableName());
}
}
@Test
+ public void testLegacyTableHandler() throws Exception {
+ // Test creating a legacy Kudu table without context succeeds.
+ Table table = newLegacyTable("legacy_table");
+ client.createTable(table);
+
+ // Check that altering legacy table's schema succeeds.
+ {
+ Table alteredTable = table.deepCopy();
+ alteredTable.getSd().addToCols(new FieldSchema("c", "int", ""));
+ client.alter_table(table.getDbName(), table.getTableName(), alteredTable);
+ }
+
+ // Check that renaming legacy table's schema succeeds.
+ final String newTable = "new_table";
+ {
+ Table alteredTable = table.deepCopy();
+ alteredTable.setTableName(newTable);
+ client.alter_table(table.getDbName(), table.getTableName(), alteredTable);
+ }
+ // Test dropping a legacy Kudu table without context succeeds.
+ client.dropTable(table.getDbName(), newTable);
+ }
+
+ @Test
public void testDropTableHandler() throws Exception {
// Test dropping a Kudu table.
Table table = newTable("table");