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/07/27 01:25:16 UTC
[GitHub] [hive] DanielZhu58 opened a new pull request, #3477: Hive 26012: HMS APIs to be enhanced for metadata replication
DanielZhu58 opened a new pull request, #3477:
URL: https://github.com/apache/hive/pull/3477
### What changes were proposed in this pull request?
Change the CreateDatabaseRequest struct, to add a boolean value skipFSWrites. This boolean value decides whether to create the directory of this database in file system or not.
Same as CreateTableRequest struct and AddPartitionsRequest struct.
Meanwhile, enhance the HMS API to accept additional boolean skipFSWrites parameters to skip the creation on a need-to basis.
### Why are the changes needed?
The following DDL statements in hive result in creation of a directories on the DFS filesystem.
"create database", "create table", "alter table 'table_name' add partition"
This is done to ensure that the user (be it be the service user or end user) has the privileges to access the underlying filesystem location. But it also sets up the subsequent queries from not having the burden of creating the directories “on demand”. For example, having the execution for “create table” statement also create the database root directories would be a little out of place.
Making this change can let the users to skip the creation on a need-to basis.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Unit tests, Thrift build, Maven build
--
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] saihemanth-cloudera commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r942048552
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,103 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+ public void testIfFSWritesIsSkippedForDatabase() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = new DatabaseBuilder()
+ .setName(TEST_DB1_NAME)
+ .setLocation(dbLocation)
+ .setManagedLocation(mgdLocation)
+ .build(conf);
+ req.setDatabase(db);
+ req.setDatabaseName(TEST_DB1_NAME);
+ client.createDatabase(req);
+
+ Path dbPath = new Path(db.getLocationUri());
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Database's file system directory is skipped", fs.exists(new Path(dbLocation)));
+ fs = FileSystem.get(new Path(mgdLocation).toUri(), conf);
+ assertTrue("Database's managed location is not skipped", fs.exists(new Path(mgdLocation)));
+ } catch (Throwable e) {
+ System.err.println(StringUtils.stringifyException(e));
Review Comment:
You'll have to remove the System.err and e.printStrackTrace() and log all the messages into the logger object.
--
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] dengzhhu653 commented on a diff in pull request #3477: Hive 26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r930579455
##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2040,7 +2044,10 @@ struct CreateDatabaseRequest {
9: optional i32 createTime,
10: optional string managedLocationUri,
11: optional string type,
- 12: optional string dataConnectorName
+ 12: optional string dataConnectorName,
+ // use boolean skipFSWrites to decide whether create directory in file system or not
+ 13: optional bool skipFSWrites=false,
+ 14: optional Database database
Review Comment:
Do we need `optional Database database` here? or can we put the `skipFSWrites` into the struct `Database`, so that the old client can be able to skip create directory with less changes?
--
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] DanielZhu58 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
DanielZhu58 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r942221981
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1399,6 +1403,43 @@ public void create_database(final Database db)
}
}
+ @Override
+ public void create_database_req(final CreateDatabaseRequest req)
+ throws AlreadyExistsException, InvalidObjectException, MetaException {
+ Database db = req.getDatabase();
+ boolean skipFSWrites = req.isSkipFSWrites();
+ startFunction("create_database_req", ": " + db.toString());
+ boolean success = false;
+ Exception ex = null;
+ if (!db.isSetCatalogName()) {
+ db.setCatalogName(getDefaultCatalog(conf));
+ }
+ try {
+ try {
+ if (null != get_database_core(db.getCatalogName(), db.getName())) {
+ throw new AlreadyExistsException("Database " + db.getName() + " already exists");
+ }
+ } catch (NoSuchObjectException e) {
+ // expected
+ }
+ create_database_core(getMS(), db, skipFSWrites);
+ success = true;
+ } catch (Exception e) {
+ ex = e;
Review Comment:
Yes, I can.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1400,6 +1405,43 @@ public void create_database(final Database db)
}
}
+ @Override
+ public void create_database_req(final CreateDatabaseRequest req)
+ throws AlreadyExistsException, InvalidObjectException, MetaException {
+ Database db = req.getDatabase();
+ boolean skipFSWrites = req.isSkipFSWrites();
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] github-actions[bot] commented on pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #3477:
URL: https://github.com/apache/hive/pull/3477#issuecomment-1283179423
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.
--
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] DanielZhu58 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
DanielZhu58 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r942220756
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,42 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+
+ public void testIfFSWritesIsSkipped() throws Throwable {
+ skipFSWritesTester();
Review Comment:
Yes. The extra method is removed.
--
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] saihemanth-cloudera commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r942049528
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,104 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+ public void testIfFSWritesIsSkippedForDatabase() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = new DatabaseBuilder()
+ .setName(TEST_DB1_NAME)
+ .setLocation(dbLocation)
+ .setManagedLocation(mgdLocation)
+ .build(conf);
+ req.setDatabase(db);
+ req.setDatabaseName(TEST_DB1_NAME);
+ req.setLocationUri(dbLocation);
+ req.setManagedLocationUri(mgdLocation);
+ client.createDatabase(req);
+
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Database's file system directory is skipped", fs.exists(new Path(dbLocation)));
+ fs = FileSystem.get(new Path(mgdLocation).toUri(), conf);
+ assertFalse("Database's managed location is not skipped", fs.exists(new Path(mgdLocation)));
+ } catch (Throwable e) {
+ System.err.println(StringUtils.stringifyException(e));
+ e.printStackTrace();
+ System.err.println("testIfFSWritesIsSkippedForDatabase() failed.");
+ throw e;
+ }
+ }
+
+ @Test
+ public void testIfFSWritesIsSkippedForTable() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+ String tableName1 = "test_table1";
+ String tableName2 = "test_table2";
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ Database db = new DatabaseBuilder()
+ .setName(TEST_DB1_NAME)
+ .setLocation(dbLocation)
+ .setManagedLocation(mgdLocation)
+ .build(conf);
+ client.createDatabase(db);
+
+ Table tbl1 = new TableBuilder()
+ .setDbName(TEST_DB1_NAME)
+ .setTableName(tableName1)
+ .addCol("name", ColumnType.STRING_TYPE_NAME)
+ .addCol("income", ColumnType.INT_TYPE_NAME)
+ .create(client, conf);
+
+ Table tbl2 = new TableBuilder()
+ .setDbName(TEST_DB1_NAME)
+ .setTableName(tableName2)
+ .addCol("name", ColumnType.STRING_TYPE_NAME)
+ .addCol("income", ColumnType.INT_TYPE_NAME)
+ .create(client, conf);
+
+ CreateTableRequest tblReq1 = new CreateTableRequest();
+ tblReq1.setTable(tbl1);
+ tblReq1.setSkipFSWrites(true);
+ String tbl1Location =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db/test_table1";
+
+ CreateTableRequest tblReq2 = new CreateTableRequest();
+ tblReq2.setTable(tbl2);
+ tblReq2.setSkipFSWrites(false);
+ String tbl2Location =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db/test_table2";
+
+ Path dbPath = new Path(db.getLocationUri());
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Table1's file system directory is skipped", fs.exists(new Path(tbl1Location)));
+ fs = FileSystem.get(new Path(mgdLocation).toUri(), conf);
+ assertTrue("Table2's file system directory is not skipped", fs.exists(new Path(tbl2Location)));
+ } catch (Throwable e) {
+ System.err.println(StringUtils.stringifyException(e));
Review Comment:
Same here. Remove the System.err and Log them into the logger object.
--
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] sonarcloud[bot] commented on pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3477:
URL: https://github.com/apache/hive/pull/3477#issuecomment-1219477057
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3477)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3477&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3477&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3477&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3477&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3477&metric=duplicated_lines_density&view=list) No Duplication information
--
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] dengzhhu653 commented on a diff in pull request #3477: Hive 26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r930579455
##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2040,7 +2044,10 @@ struct CreateDatabaseRequest {
9: optional i32 createTime,
10: optional string managedLocationUri,
11: optional string type,
- 12: optional string dataConnectorName
+ 12: optional string dataConnectorName,
+ // use boolean skipFSWrites to decide whether create directory in file system or not
+ 13: optional bool skipFSWrites=false,
+ 14: optional Database database
Review Comment:
Do we need `optional Database database` here? or can we just put the `skipFSWrites` into the struct `Database`, so that the old client can be able to skip create directory with less changes?
--
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] saihemanth-cloudera commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r931896175
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1399,6 +1403,42 @@ public void create_database(final Database db)
}
}
+ public void create_database_req(final CreateDatabaseRequest req)
Review Comment:
Shouldn't this be an overridden method?
--
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] dengzhhu653 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r946831412
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1230,74 +1230,79 @@ private void create_database_core(RawStore ms, final Database db)
Map<String, String> transactionalListenersResponses = Collections.emptyMap();
try {
firePreEvent(new PreCreateDatabaseEvent(db, this));
- //reinstate location uri for metastore db.
- if (skipAuthorization == true){
- db.setLocationUri(dbExtPath.toString());
- if (dbMgdPath != null) {
- db.setManagedLocationUri(dbMgdPath.toString());
- }
- }
- if (db.getCatalogName() != null && !db.getCatalogName().
- equals(Warehouse.DEFAULT_CATALOG_NAME)) {
- if (!wh.isDir(dbExtPath)) {
- LOG.debug("Creating database path " + dbExtPath);
- if (!wh.mkdirs(dbExtPath)) {
- throw new MetaException("Unable to create database path " + dbExtPath +
- ", failed to create database " + db.getName());
+
+ //reinstate location uri for metastore db.
Review Comment:
nit: extra spaces
--
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] dengzhhu653 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r936606173
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,103 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+ public void testIfFSWritesIsSkippedForDatabase() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = new DatabaseBuilder()
+ .setName(TEST_DB1_NAME)
+ .setLocation(dbLocation)
+ .setManagedLocation(mgdLocation)
+ .build(conf);
+ req.setDatabase(db);
+ req.setDatabaseName(TEST_DB1_NAME);
+ client.createDatabase(req);
+
+ Path dbPath = new Path(db.getLocationUri());
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Database's file system directory is skipped", fs.exists(new Path(dbLocation)));
+ fs = FileSystem.get(new Path(mgdLocation).toUri(), conf);
+ assertTrue("Database's managed location is not skipped", fs.exists(new Path(mgdLocation)));
+ } catch (Throwable e) {
+ System.err.println(StringUtils.stringifyException(e));
Review Comment:
how about e.printStackTrace()?
--
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] dengzhhu653 commented on a diff in pull request #3477: Hive 26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r930584920
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -2361,32 +2402,36 @@ private void create_table_core(final RawStore ms, final CreateTableRequest req)
firePreEvent(new PreCreateTableEvent(tbl, db, this));
- if (!TableType.VIRTUAL_VIEW.toString().equals(tbl.getTableType())) {
- if (tbl.getSd().getLocation() == null
- || tbl.getSd().getLocation().isEmpty()) {
- tblPath = wh.getDefaultTablePath(db, tbl.getTableName() + getTableSuffix(tbl), isExternal(tbl));
- } else {
- if (!isExternal(tbl) && !MetaStoreUtils.isNonNativeTable(tbl)) {
- LOG.warn("Location: " + tbl.getSd().getLocation()
- + " specified for non-external table:" + tbl.getTableName());
- }
- tblPath = wh.getDnsPath(new Path(tbl.getSd().getLocation()));
- // ignore suffix if it's already there (direct-write CTAS)
- if (!tblPath.getName().matches("(.*)" + SOFT_DELETE_TABLE_PATTERN)) {
- tblPath = new Path(tblPath + getTableSuffix(tbl));
+ if (!skipFSWrites) {
Review Comment:
how about change the condition to `!skipFSWrites && !TableType.VIRTUAL_VIEW.toString().equals(tbl.getTableType())`
--
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] saihemanth-cloudera commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r931888382
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -3958,12 +4003,16 @@ private Partition append_partition_common(RawStore ms, String catName, String db
throw new AlreadyExistsException("Partition already exists:" + part);
}
- if (!wh.isDir(partLocation)) {
- if (!wh.mkdirs(partLocation)) {
- throw new MetaException(partLocation
- + " is not a directory or unable to create one");
+ if (!skipFSWrites) {
+ if (!wh.isDir(partLocation)) {
+ if (!wh.mkdirs(partLocation)) {
+ throw new MetaException(partLocation
+ + " is not a directory or unable to create one");
+ }
+ madeDir = true;
}
- madeDir = true;
+ } else {
+ LOG.warn("Because skipFSWrites is true, skip creating directories for partitions.");
Review Comment:
Why do you want to set this log to warn? Can we leave this as Log.info?
--
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] saihemanth-cloudera commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r934986137
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,42 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+
+ public void testIfFSWritesIsSkipped() throws Throwable {
+ skipFSWritesTester();
+ }
+ private void skipFSWritesTester() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = req.getDatabase();
Review Comment:
Use the databaseBuilder to create the database object and then set it to CreateDatabaseReq object
--
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] saihemanth-cloudera commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r942048008
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,42 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+
+ public void testIfFSWritesIsSkipped() throws Throwable {
+ skipFSWritesTester();
+ }
+ private void skipFSWritesTester() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = req.getDatabase();
+ db.setName(TEST_DB1_NAME);
+ client.createDatabase(req);
+
+ Path dbPath = new Path(db.getLocationUri());
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Database's file system directory is skipped", fs.exists(new Path(dbLocation)));
+ fs = FileSystem.get(new Path(mgdLocation).toUri(), conf);
Review Comment:
Can you also get the created database object and assert on its location and managed location?
--
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] DanielZhu58 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
DanielZhu58 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r942219799
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -2361,32 +2402,36 @@ private void create_table_core(final RawStore ms, final CreateTableRequest req)
firePreEvent(new PreCreateTableEvent(tbl, db, this));
- if (!TableType.VIRTUAL_VIEW.toString().equals(tbl.getTableType())) {
- if (tbl.getSd().getLocation() == null
- || tbl.getSd().getLocation().isEmpty()) {
- tblPath = wh.getDefaultTablePath(db, tbl.getTableName() + getTableSuffix(tbl), isExternal(tbl));
- } else {
- if (!isExternal(tbl) && !MetaStoreUtils.isNonNativeTable(tbl)) {
- LOG.warn("Location: " + tbl.getSd().getLocation()
- + " specified for non-external table:" + tbl.getTableName());
- }
- tblPath = wh.getDnsPath(new Path(tbl.getSd().getLocation()));
- // ignore suffix if it's already there (direct-write CTAS)
- if (!tblPath.getName().matches("(.*)" + SOFT_DELETE_TABLE_PATTERN)) {
- tblPath = new Path(tblPath + getTableSuffix(tbl));
+ if (!skipFSWrites) {
Review Comment:
These two if condition cannot be combined here because we have an else structure in the second if condition.
##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2025,7 +2027,9 @@ struct CreateTableRequest {
7: optional list<SQLDefaultConstraint> defaultConstraints,
8: optional list<SQLCheckConstraint> checkConstraints,
9: optional list<string> processorCapabilities,
- 10: optional string processorIdentifier
+ 10: optional string processorIdentifier,
+ // use boolean skipFSWrites to decide whether create directory in file system or not
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] dengzhhu653 commented on pull request #3477: Hive 26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on PR #3477:
URL: https://github.com/apache/hive/pull/3477#issuecomment-1196226978
also should we test renaming a table that skips creating the directory?
--
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] sonarcloud[bot] commented on pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3477:
URL: https://github.com/apache/hive/pull/3477#issuecomment-1220220148
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3477)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3477&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3477&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3477&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=CODE_SMELL) [19 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3477&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3477&metric=duplicated_lines_density&view=list) No Duplication information
--
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] saihemanth-cloudera commented on pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on PR #3477:
URL: https://github.com/apache/hive/pull/3477#issuecomment-1210200482
@DanielZhu58 - Can you post the replies for @dengzhhu653's questions? 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] sonarcloud[bot] commented on pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3477:
URL: https://github.com/apache/hive/pull/3477#issuecomment-1220101835
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3477)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3477&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3477&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3477&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3477&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3477&metric=duplicated_lines_density&view=list) No Duplication information
--
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] dengzhhu653 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r936607197
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,103 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+ public void testIfFSWritesIsSkippedForDatabase() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = new DatabaseBuilder()
+ .setName(TEST_DB1_NAME)
+ .setLocation(dbLocation)
+ .setManagedLocation(mgdLocation)
+ .build(conf);
+ req.setDatabase(db);
+ req.setDatabaseName(TEST_DB1_NAME);
+ client.createDatabase(req);
+
+ Path dbPath = new Path(db.getLocationUri());
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Database's file system directory is skipped", fs.exists(new Path(dbLocation)));
+ fs = FileSystem.get(new Path(mgdLocation).toUri(), conf);
+ assertTrue("Database's managed location is not skipped", fs.exists(new Path(mgdLocation)));
+ } catch (Throwable e) {
+ System.err.println(StringUtils.stringifyException(e));
+ System.err.println("testIfFSWritesIsSkippedForDatabase() failed.");
+ throw e;
+ }
+ }
+
+ @Test
+ public void testIfFSWritesIsSkippedForTable() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+ String tableName1 = "test_table1";
+ String tableName2 = "test_table2";
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ Database db = new DatabaseBuilder()
Review Comment:
maybe should create `db` first in HMS
--
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] dengzhhu653 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r936614566
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -41,15 +41,8 @@
import static org.mockito.Mockito.mock;
import com.google.common.collect.Sets;
-import org.apache.hadoop.hive.metastore.api.DataConnector;
-import org.apache.hadoop.hive.metastore.api.DatabaseType;
-import org.apache.hadoop.hive.metastore.api.GetPartitionsFilterSpec;
-import org.apache.hadoop.hive.metastore.api.GetProjectionsSpec;
-import org.apache.hadoop.hive.metastore.api.GetPartitionsRequest;
-import org.apache.hadoop.hive.metastore.api.GetPartitionsResponse;
-import org.apache.hadoop.hive.metastore.api.PartitionSpecWithSharedSD;
-import org.apache.hadoop.hive.metastore.api.PartitionWithoutSD;
-import org.apache.hadoop.hive.metastore.api.SourceTable;
+import org.apache.hadoop.hive.metastore.api.*;
Review Comment:
please do not squash the imports with '*'
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -41,15 +41,8 @@
import static org.mockito.Mockito.mock;
import com.google.common.collect.Sets;
-import org.apache.hadoop.hive.metastore.api.DataConnector;
-import org.apache.hadoop.hive.metastore.api.DatabaseType;
-import org.apache.hadoop.hive.metastore.api.GetPartitionsFilterSpec;
-import org.apache.hadoop.hive.metastore.api.GetProjectionsSpec;
-import org.apache.hadoop.hive.metastore.api.GetPartitionsRequest;
-import org.apache.hadoop.hive.metastore.api.GetPartitionsResponse;
-import org.apache.hadoop.hive.metastore.api.PartitionSpecWithSharedSD;
-import org.apache.hadoop.hive.metastore.api.PartitionWithoutSD;
-import org.apache.hadoop.hive.metastore.api.SourceTable;
+import org.apache.hadoop.hive.metastore.api.*;
Review Comment:
nit: please do not squash the imports with '*'
--
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] dengzhhu653 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r934387518
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,42 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+
+ public void testIfFSWritesIsSkipped() throws Throwable {
+ skipFSWritesTester();
+ }
+ private void skipFSWritesTester() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = req.getDatabase();
Review Comment:
The failed tests seem have relation with the changes, as the `db` is null. you should create `Database` first.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1399,6 +1403,43 @@ public void create_database(final Database db)
}
}
+ @Override
+ public void create_database_req(final CreateDatabaseRequest req)
+ throws AlreadyExistsException, InvalidObjectException, MetaException {
+ Database db = req.getDatabase();
+ boolean skipFSWrites = req.isSkipFSWrites();
+ startFunction("create_database_req", ": " + db.toString());
+ boolean success = false;
+ Exception ex = null;
+ if (!db.isSetCatalogName()) {
+ db.setCatalogName(getDefaultCatalog(conf));
+ }
+ try {
+ try {
+ if (null != get_database_core(db.getCatalogName(), db.getName())) {
+ throw new AlreadyExistsException("Database " + db.getName() + " already exists");
+ }
+ } catch (NoSuchObjectException e) {
+ // expected
+ }
+ create_database_core(getMS(), db, skipFSWrites);
+ success = true;
+ } catch (Exception e) {
+ ex = e;
Review Comment:
the catch clause can be rewritten like:
` ex = e;
throw handleException(e)
.throwIfInstance(MetaException.class, InvalidObjectException.class, AlreadyExistsException.class)
.defaultMetaException();`
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,42 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+
+ public void testIfFSWritesIsSkipped() throws Throwable {
+ skipFSWritesTester();
Review Comment:
seems we do not need introduce another method to perform tests
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,42 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+
+ public void testIfFSWritesIsSkipped() throws Throwable {
+ skipFSWritesTester();
+ }
+ private void skipFSWritesTester() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = req.getDatabase();
+ db.setName(TEST_DB1_NAME);
+ client.createDatabase(req);
+
+ Path dbPath = new Path(db.getLocationUri());
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Database's file system directory is skipped", fs.exists(new Path(dbLocation)));
+ fs = FileSystem.get(new Path(mgdLocation).toUri(), conf);
Review Comment:
maybe we should call `createDatabase` again with Database location setting to `mgdLocation`.
--
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] DanielZhu58 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
DanielZhu58 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r942220131
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -3958,12 +4003,16 @@ private Partition append_partition_common(RawStore ms, String catName, String db
throw new AlreadyExistsException("Partition already exists:" + part);
}
- if (!wh.isDir(partLocation)) {
- if (!wh.mkdirs(partLocation)) {
- throw new MetaException(partLocation
- + " is not a directory or unable to create one");
+ if (!skipFSWrites) {
+ if (!wh.isDir(partLocation)) {
+ if (!wh.mkdirs(partLocation)) {
+ throw new MetaException(partLocation
+ + " is not a directory or unable to create one");
+ }
+ madeDir = true;
}
- madeDir = true;
+ } else {
+ LOG.warn("Because skipFSWrites is true, skip creating directories for partitions.");
Review Comment:
Yes, we can leave this as LOG.info.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -2361,32 +2402,36 @@ private void create_table_core(final RawStore ms, final CreateTableRequest req)
firePreEvent(new PreCreateTableEvent(tbl, db, this));
- if (!TableType.VIRTUAL_VIEW.toString().equals(tbl.getTableType())) {
- if (tbl.getSd().getLocation() == null
- || tbl.getSd().getLocation().isEmpty()) {
- tblPath = wh.getDefaultTablePath(db, tbl.getTableName() + getTableSuffix(tbl), isExternal(tbl));
- } else {
- if (!isExternal(tbl) && !MetaStoreUtils.isNonNativeTable(tbl)) {
- LOG.warn("Location: " + tbl.getSd().getLocation()
- + " specified for non-external table:" + tbl.getTableName());
- }
- tblPath = wh.getDnsPath(new Path(tbl.getSd().getLocation()));
- // ignore suffix if it's already there (direct-write CTAS)
- if (!tblPath.getName().matches("(.*)" + SOFT_DELETE_TABLE_PATTERN)) {
- tblPath = new Path(tblPath + getTableSuffix(tbl));
+ if (!skipFSWrites) {
+ if (!TableType.VIRTUAL_VIEW.toString().equals(tbl.getTableType())) {
+ if (tbl.getSd().getLocation() == null
+ || tbl.getSd().getLocation().isEmpty()) {
+ tblPath = wh.getDefaultTablePath(db, tbl.getTableName() + getTableSuffix(tbl), isExternal(tbl));
+ } else {
+ if (!isExternal(tbl) && !MetaStoreUtils.isNonNativeTable(tbl)) {
+ LOG.warn("Location: " + tbl.getSd().getLocation()
+ + " specified for non-external table:" + tbl.getTableName());
+ }
+ tblPath = wh.getDnsPath(new Path(tbl.getSd().getLocation()));
+ // ignore suffix if it's already there (direct-write CTAS)
+ if (!tblPath.getName().matches("(.*)" + SOFT_DELETE_TABLE_PATTERN)) {
+ tblPath = new Path(tblPath + getTableSuffix(tbl));
+ }
}
+ tbl.getSd().setLocation(tblPath.toString());
}
- tbl.getSd().setLocation(tblPath.toString());
- }
- if (tblPath != null) {
- if (!wh.isDir(tblPath)) {
- if (!wh.mkdirs(tblPath)) {
- throw new MetaException(tblPath
- + " is not a directory or unable to create one");
+ if (tblPath != null) {
+ if (!wh.isDir(tblPath)) {
+ if (!wh.mkdirs(tblPath)) {
+ throw new MetaException(tblPath
+ + " is not a directory or unable to create one");
+ }
+ madeDir = true;
}
- madeDir = true;
}
+ } else {
+ LOG.warn("Because skipFSWrites is true, skip the creation of directories for tables.");
Review Comment:
Yes, we can leave this as LOG.info.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1399,6 +1403,42 @@ public void create_database(final Database db)
}
}
+ public void create_database_req(final CreateDatabaseRequest req)
Review Comment:
Yes. `@Override` is added.
--
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] DanielZhu58 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
DanielZhu58 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r942220459
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,42 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+
+ public void testIfFSWritesIsSkipped() throws Throwable {
+ skipFSWritesTester();
+ }
+ private void skipFSWritesTester() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = req.getDatabase();
Review Comment:
Yes. The changes are completed.
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,42 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+
+ public void testIfFSWritesIsSkipped() throws Throwable {
+ skipFSWritesTester();
+ }
+ private void skipFSWritesTester() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = req.getDatabase();
+ db.setName(TEST_DB1_NAME);
+ client.createDatabase(req);
+
+ Path dbPath = new Path(db.getLocationUri());
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Database's file system directory is skipped", fs.exists(new Path(dbLocation)));
+ fs = FileSystem.get(new Path(mgdLocation).toUri(), conf);
Review Comment:
Sure. Will add it.
--
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] DanielZhu58 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
DanielZhu58 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r942219606
##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -901,7 +901,9 @@ struct AddPartitionsRequest {
4: required bool ifNotExists,
5: optional bool needResult=true,
6: optional string catName,
- 7: optional string validWriteIdList
+ 7: optional string validWriteIdList,
+ // use boolean skipFSWrites to decide whether create directory in file system or not
+ 8: optional bool skipFSWrites=false
Review Comment:
Yes, we do. Because that's one of the requirement of this patch.
##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2040,7 +2044,10 @@ struct CreateDatabaseRequest {
9: optional i32 createTime,
10: optional string managedLocationUri,
11: optional string type,
- 12: optional string dataConnectorName
+ 12: optional string dataConnectorName,
+ // use boolean skipFSWrites to decide whether create directory in file system or not
+ 13: optional bool skipFSWrites=false,
+ 14: optional Database database
Review Comment:
Yes, we do. Because in the create database function with signature CreateDatabaseRequest, we will use getDatabase() function. This will need the `optional Database database`.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1229,74 +1229,78 @@ private void create_database_core(RawStore ms, final Database db)
Map<String, String> transactionalListenersResponses = Collections.emptyMap();
try {
firePreEvent(new PreCreateDatabaseEvent(db, this));
- //reinstate location uri for metastore db.
- if (skipAuthorization == true){
Review Comment:
We can leave it the same way this time. Because this change is not highly related with the HMS directory skipping.
--
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] dengzhhu653 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r941205221
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,104 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+ public void testIfFSWritesIsSkippedForDatabase() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = new DatabaseBuilder()
+ .setName(TEST_DB1_NAME)
+ .setLocation(dbLocation)
+ .setManagedLocation(mgdLocation)
+ .build(conf);
+ req.setDatabase(db);
+ req.setDatabaseName(TEST_DB1_NAME);
+ req.setLocationUri(dbLocation);
+ req.setManagedLocationUri(mgdLocation);
+ client.createDatabase(req);
+
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Database's file system directory is skipped", fs.exists(new Path(dbLocation)));
+ fs = FileSystem.get(new Path(mgdLocation).toUri(), conf);
+ assertFalse("Database's managed location is not skipped", fs.exists(new Path(mgdLocation)));
+ } catch (Throwable e) {
+ System.err.println(StringUtils.stringifyException(e));
+ e.printStackTrace();
+ System.err.println("testIfFSWritesIsSkippedForDatabase() failed.");
+ throw e;
+ }
+ }
+
+ @Test
+ public void testIfFSWritesIsSkippedForTable() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+ String tableName1 = "test_table1";
+ String tableName2 = "test_table2";
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ Database db = new DatabaseBuilder()
+ .setName(TEST_DB1_NAME)
+ .setLocation(dbLocation)
+ .setManagedLocation(mgdLocation)
+ .build(conf);
+ client.createDatabase(db);
+
+ Table tbl1 = new TableBuilder()
+ .setDbName(TEST_DB1_NAME)
+ .setTableName(tableName1)
+ .addCol("name", ColumnType.STRING_TYPE_NAME)
+ .addCol("income", ColumnType.INT_TYPE_NAME)
+ .create(client, conf);
+
+ Table tbl2 = new TableBuilder()
+ .setDbName(TEST_DB1_NAME)
+ .setTableName(tableName2)
+ .addCol("name", ColumnType.STRING_TYPE_NAME)
+ .addCol("income", ColumnType.INT_TYPE_NAME)
+ .create(client, conf);
+
+ CreateTableRequest tblReq1 = new CreateTableRequest();
+ tblReq1.setTable(tbl1);
+ tblReq1.setSkipFSWrites(true);
+ String tbl1Location =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db/test_table1";
+
+ CreateTableRequest tblReq2 = new CreateTableRequest();
+ tblReq2.setTable(tbl2);
+ tblReq2.setSkipFSWrites(false);
+ String tbl2Location =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db/test_table2";
+
+ Path dbPath = new Path(db.getLocationUri());
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
Review Comment:
Should check the table path?
--
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] github-actions[bot] closed pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
URL: https://github.com/apache/hive/pull/3477
--
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] dengzhhu653 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r946841781
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,135 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+ public void testIfFSWritesIsSkippedForDatabase() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with skipFSWrites is true, the directory is not created
+ // with skipFSWrites is false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = new DatabaseBuilder()
+ .setName(TEST_DB1_NAME)
+ .setLocation(dbLocation)
+ .setManagedLocation(mgdLocation)
+ .build(conf);
+ req.setDatabase(db);
+ req.setDatabaseName(TEST_DB1_NAME);
+ req.setLocationUri(dbLocation);
+ req.setManagedLocationUri(mgdLocation);
+ client.createDatabase(req);
+
+ String tblLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db/test_table";
+ String tblmgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db/test_table";
+
+ CreateTableRequest tblReq = new CreateTableRequest();
+ tblReq.setSkipFSWrites(true);
+ String tableName = "test_table";
+ Table table = new TableBuilder()
+ .setDbName(TEST_DB1_NAME)
+ .setTableName(tableName)
+ .addCol("name", ColumnType.STRING_TYPE_NAME)
+ .addCol("income", ColumnType.INT_TYPE_NAME)
+ .create(client, conf);
+ tblReq.setTable(table);
+ client.createTable(tblReq);
+
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Database's file system directory is skipped", fs.exists(new Path(dbLocation)));
+ assertFalse("Table's file system directory is skipped", fs.exists(new Path(tblLocation)));
+
+ fs = FileSystem.get(new Path(mgdLocation).toUri(), conf);
+ assertFalse("Database's managed location directory is skipped", fs.exists(new Path(mgdLocation)));
+ assertFalse("Table's managed location directory is skipped", fs.exists(new Path(tblmgdLocation)));
+
+ String dbLocationUri = "file:" + dbLocation.substring(7);
+ String mgdLocationUri = "file:" + mgdLocation.substring(7);
+ assertTrue("Database's dbLocation has been set", db.getLocationUri().equals(dbLocationUri));
+ assertTrue("Database's managed location has been set", db.getManagedLocationUri().equals(mgdLocationUri));
+ } catch (Throwable e) {
+ LOG.info(StringUtils.stringifyException(e));
+ e.printStackTrace();
+ LOG.info("testIfFSWritesIsSkippedForDatabase() failed.");
+ throw e;
+ }
+ }
+
+ @Test
+ public void testIfFSWritesIsSkippedForTable() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+ String tableName1 = "test_table1";
+ String tableName2 = "test_table2";
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ Database db = new DatabaseBuilder()
+ .setName(TEST_DB1_NAME)
+ .setLocation(dbLocation)
+ .setManagedLocation(mgdLocation)
+ .build(conf);
+ client.createDatabase(db);
+
+ Table tbl1 = new TableBuilder()
+ .setDbName(TEST_DB1_NAME)
+ .setTableName(tableName1)
+ .addCol("name", ColumnType.STRING_TYPE_NAME)
+ .addCol("income", ColumnType.INT_TYPE_NAME)
+ .create(client, conf);
+
+ Table tbl2 = new TableBuilder()
+ .setDbName(TEST_DB1_NAME)
+ .setTableName(tableName2)
+ .addCol("name", ColumnType.STRING_TYPE_NAME)
+ .addCol("income", ColumnType.INT_TYPE_NAME)
+ .create(client, conf);
+
+ CreateTableRequest tblReq1 = new CreateTableRequest();
+ tblReq1.setTable(tbl1);
+ tblReq1.setSkipFSWrites(true);
+ String tbl1Location =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db/test_table1";
+ String tbl1mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db/test_table1";
+
+ CreateTableRequest tblReq2 = new CreateTableRequest();
+ tblReq2.setTable(tbl2);
+ tblReq2.setSkipFSWrites(false);
+ String tbl2Location =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db/test_table2";
+ String tbl2mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db/test_table2";
+
+ Path dbPath = new Path(db.getLocationUri());
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Table1's file system directory is skipped", fs.exists(new Path(tbl1Location)));
+ assertFalse("Table2's file system directory is not skipped", fs.exists(new Path(tbl2Location)));
Review Comment:
assertFalse -> assertTrue?
--
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] saihemanth-cloudera commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r942043946
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1400,6 +1405,43 @@ public void create_database(final Database db)
}
}
+ @Override
+ public void create_database_req(final CreateDatabaseRequest req)
+ throws AlreadyExistsException, InvalidObjectException, MetaException {
+ Database db = req.getDatabase();
+ boolean skipFSWrites = req.isSkipFSWrites();
Review Comment:
Nit: We directly pass this value in L#1427 instead of declaring a variable 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] saihemanth-cloudera commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r931890038
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -2361,32 +2402,36 @@ private void create_table_core(final RawStore ms, final CreateTableRequest req)
firePreEvent(new PreCreateTableEvent(tbl, db, this));
- if (!TableType.VIRTUAL_VIEW.toString().equals(tbl.getTableType())) {
- if (tbl.getSd().getLocation() == null
- || tbl.getSd().getLocation().isEmpty()) {
- tblPath = wh.getDefaultTablePath(db, tbl.getTableName() + getTableSuffix(tbl), isExternal(tbl));
- } else {
- if (!isExternal(tbl) && !MetaStoreUtils.isNonNativeTable(tbl)) {
- LOG.warn("Location: " + tbl.getSd().getLocation()
- + " specified for non-external table:" + tbl.getTableName());
- }
- tblPath = wh.getDnsPath(new Path(tbl.getSd().getLocation()));
- // ignore suffix if it's already there (direct-write CTAS)
- if (!tblPath.getName().matches("(.*)" + SOFT_DELETE_TABLE_PATTERN)) {
- tblPath = new Path(tblPath + getTableSuffix(tbl));
+ if (!skipFSWrites) {
+ if (!TableType.VIRTUAL_VIEW.toString().equals(tbl.getTableType())) {
+ if (tbl.getSd().getLocation() == null
+ || tbl.getSd().getLocation().isEmpty()) {
+ tblPath = wh.getDefaultTablePath(db, tbl.getTableName() + getTableSuffix(tbl), isExternal(tbl));
+ } else {
+ if (!isExternal(tbl) && !MetaStoreUtils.isNonNativeTable(tbl)) {
+ LOG.warn("Location: " + tbl.getSd().getLocation()
+ + " specified for non-external table:" + tbl.getTableName());
+ }
+ tblPath = wh.getDnsPath(new Path(tbl.getSd().getLocation()));
+ // ignore suffix if it's already there (direct-write CTAS)
+ if (!tblPath.getName().matches("(.*)" + SOFT_DELETE_TABLE_PATTERN)) {
+ tblPath = new Path(tblPath + getTableSuffix(tbl));
+ }
}
+ tbl.getSd().setLocation(tblPath.toString());
}
- tbl.getSd().setLocation(tblPath.toString());
- }
- if (tblPath != null) {
- if (!wh.isDir(tblPath)) {
- if (!wh.mkdirs(tblPath)) {
- throw new MetaException(tblPath
- + " is not a directory or unable to create one");
+ if (tblPath != null) {
+ if (!wh.isDir(tblPath)) {
+ if (!wh.mkdirs(tblPath)) {
+ throw new MetaException(tblPath
+ + " is not a directory or unable to create one");
+ }
+ madeDir = true;
}
- madeDir = true;
}
+ } else {
+ LOG.warn("Because skipFSWrites is true, skip the creation of directories for tables.");
Review Comment:
Why do you want to set this log to warn? Can we leave this as Log.info?
--
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] sonarcloud[bot] commented on pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3477:
URL: https://github.com/apache/hive/pull/3477#issuecomment-1220477287
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3477)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3477&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3477&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3477&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=CODE_SMELL) [19 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3477&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3477&metric=duplicated_lines_density&view=list) No Duplication information
--
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] sonarcloud[bot] commented on pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3477:
URL: https://github.com/apache/hive/pull/3477#issuecomment-1219730170
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3477)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3477&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3477&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3477&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3477&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3477&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3477&metric=duplicated_lines_density&view=list) No Duplication information
--
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] dengzhhu653 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r936605923
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,103 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+ public void testIfFSWritesIsSkippedForDatabase() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = new DatabaseBuilder()
+ .setName(TEST_DB1_NAME)
+ .setLocation(dbLocation)
+ .setManagedLocation(mgdLocation)
+ .build(conf);
+ req.setDatabase(db);
+ req.setDatabaseName(TEST_DB1_NAME);
+ client.createDatabase(req);
+
+ Path dbPath = new Path(db.getLocationUri());
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Database's file system directory is skipped", fs.exists(new Path(dbLocation)));
+ fs = FileSystem.get(new Path(mgdLocation).toUri(), conf);
+ assertTrue("Database's managed location is not skipped", fs.exists(new Path(mgdLocation)));
Review Comment:
should `mgdLocation` not be created as `skipFsWrites` is true?
--
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] saihemanth-cloudera commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r931856910
##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2025,7 +2027,9 @@ struct CreateTableRequest {
7: optional list<SQLDefaultConstraint> defaultConstraints,
8: optional list<SQLCheckConstraint> checkConstraints,
9: optional list<string> processorCapabilities,
- 10: optional string processorIdentifier
+ 10: optional string processorIdentifier,
+ // use boolean skipFSWrites to decide whether create directory in file system or not
Review Comment:
Add "HIVE-26012" in the comment so that if anyone needs more details on this, they can review the 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.
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] dengzhhu653 commented on a diff in pull request #3477: Hive 26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r930575169
##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -901,7 +901,9 @@ struct AddPartitionsRequest {
4: required bool ifNotExists,
5: optional bool needResult=true,
6: optional string catName,
- 7: optional string validWriteIdList
+ 7: optional string validWriteIdList,
+ // use boolean skipFSWrites to decide whether create directory in file system or not
+ 8: optional bool skipFSWrites=false
Review Comment:
as far as I know, Hive only allows native table to add partitions, do we need this for partitioned tables?
--
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] dengzhhu653 commented on a diff in pull request #3477: Hive 26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r930582433
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1229,74 +1229,78 @@ private void create_database_core(RawStore ms, final Database db)
Map<String, String> transactionalListenersResponses = Collections.emptyMap();
try {
firePreEvent(new PreCreateDatabaseEvent(db, this));
- //reinstate location uri for metastore db.
- if (skipAuthorization == true){
Review Comment:
could we extract the creating directory logic to a standalone method, so it's clear? just a suggestion.
--
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] saihemanth-cloudera commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r942043042
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1399,6 +1403,43 @@ public void create_database(final Database db)
}
}
+ @Override
+ public void create_database_req(final CreateDatabaseRequest req)
+ throws AlreadyExistsException, InvalidObjectException, MetaException {
+ Database db = req.getDatabase();
+ boolean skipFSWrites = req.isSkipFSWrites();
+ startFunction("create_database_req", ": " + db.toString());
+ boolean success = false;
+ Exception ex = null;
+ if (!db.isSetCatalogName()) {
+ db.setCatalogName(getDefaultCatalog(conf));
+ }
+ try {
+ try {
+ if (null != get_database_core(db.getCatalogName(), db.getName())) {
+ throw new AlreadyExistsException("Database " + db.getName() + " already exists");
+ }
+ } catch (NoSuchObjectException e) {
+ // expected
+ }
+ create_database_core(getMS(), db, skipFSWrites);
+ success = true;
+ } catch (Exception e) {
+ ex = e;
Review Comment:
@DanielZhu58 - Can you implement the above suggestion?
--
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] DanielZhu58 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
DanielZhu58 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r942222476
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,104 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+ public void testIfFSWritesIsSkippedForDatabase() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = new DatabaseBuilder()
+ .setName(TEST_DB1_NAME)
+ .setLocation(dbLocation)
+ .setManagedLocation(mgdLocation)
+ .build(conf);
+ req.setDatabase(db);
+ req.setDatabaseName(TEST_DB1_NAME);
+ req.setLocationUri(dbLocation);
+ req.setManagedLocationUri(mgdLocation);
+ client.createDatabase(req);
+
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Database's file system directory is skipped", fs.exists(new Path(dbLocation)));
+ fs = FileSystem.get(new Path(mgdLocation).toUri(), conf);
+ assertFalse("Database's managed location is not skipped", fs.exists(new Path(mgdLocation)));
+ } catch (Throwable e) {
+ System.err.println(StringUtils.stringifyException(e));
+ e.printStackTrace();
+ System.err.println("testIfFSWritesIsSkippedForDatabase() failed.");
+ throw e;
+ }
+ }
+
+ @Test
+ public void testIfFSWritesIsSkippedForTable() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+ String tableName1 = "test_table1";
+ String tableName2 = "test_table2";
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ Database db = new DatabaseBuilder()
+ .setName(TEST_DB1_NAME)
+ .setLocation(dbLocation)
+ .setManagedLocation(mgdLocation)
+ .build(conf);
+ client.createDatabase(db);
+
+ Table tbl1 = new TableBuilder()
+ .setDbName(TEST_DB1_NAME)
+ .setTableName(tableName1)
+ .addCol("name", ColumnType.STRING_TYPE_NAME)
+ .addCol("income", ColumnType.INT_TYPE_NAME)
+ .create(client, conf);
+
+ Table tbl2 = new TableBuilder()
+ .setDbName(TEST_DB1_NAME)
+ .setTableName(tableName2)
+ .addCol("name", ColumnType.STRING_TYPE_NAME)
+ .addCol("income", ColumnType.INT_TYPE_NAME)
+ .create(client, conf);
+
+ CreateTableRequest tblReq1 = new CreateTableRequest();
+ tblReq1.setTable(tbl1);
+ tblReq1.setSkipFSWrites(true);
+ String tbl1Location =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db/test_table1";
+
+ CreateTableRequest tblReq2 = new CreateTableRequest();
+ tblReq2.setTable(tbl2);
+ tblReq2.setSkipFSWrites(false);
+ String tbl2Location =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db/test_table2";
+
+ Path dbPath = new Path(db.getLocationUri());
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Table1's file system directory is skipped", fs.exists(new Path(tbl1Location)));
+ fs = FileSystem.get(new Path(mgdLocation).toUri(), conf);
+ assertTrue("Table2's file system directory is not skipped", fs.exists(new Path(tbl2Location)));
+ } catch (Throwable e) {
+ System.err.println(StringUtils.stringifyException(e));
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] DanielZhu58 commented on a diff in pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
DanielZhu58 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r936963722
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,103 @@ public void testDropDataConnectorIfNotExistsTrue() throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+ public void testIfFSWritesIsSkippedForDatabase() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) + "/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = new DatabaseBuilder()
+ .setName(TEST_DB1_NAME)
+ .setLocation(dbLocation)
+ .setManagedLocation(mgdLocation)
+ .build(conf);
+ req.setDatabase(db);
+ req.setDatabaseName(TEST_DB1_NAME);
+ client.createDatabase(req);
+
+ Path dbPath = new Path(db.getLocationUri());
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Database's file system directory is skipped", fs.exists(new Path(dbLocation)));
+ fs = FileSystem.get(new Path(mgdLocation).toUri(), conf);
+ assertTrue("Database's managed location is not skipped", fs.exists(new Path(mgdLocation)));
+ } catch (Throwable e) {
+ System.err.println(StringUtils.stringifyException(e));
Review Comment:
Sure, I can change it to e.printStackTrace().
--
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] github-actions[bot] closed pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #3477: HIVE-26012: HMS APIs to be enhanced for metadata replication
URL: https://github.com/apache/hive/pull/3477
--
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