[ https://issues.apache.org/jira/browse/HIVE-26012?focusedWorklogId=796847&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-796847 ]
ASF GitHub Bot logged work on HIVE-26012: ----------------------------------------- Author: ASF GitHub Bot Created on: 01/Aug/22 10:46 Start Date: 01/Aug/22 10:46 Worklog Time Spent: 10m Work Description: 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`. Issue Time Tracking ------------------- Worklog Id: (was: 796847) Time Spent: 50m (was: 40m) > HMS APIs to be enhanced for metadata replication > ------------------------------------------------ > > Key: HIVE-26012 > URL: https://issues.apache.org/jira/browse/HIVE-26012 > Project: Hive > Issue Type: Improvement > Components: Metastore > Affects Versions: 3.1.0 > Reporter: Naveen Gangam > Assignee: Hongdan Zhu > Priority: Major > Labels: pull-request-available > Attachments: HMS APIs to be enhanced for metadata replication.docx > > Time Spent: 50m > Remaining Estimate: 0h > > HMS currently has APIs like these that automatically create/delete the > directories on the associated DFS. > [create/drop]_database > [create/drop]_table* > [add/append/drop]_partition* > This is expected and should be this way when query processors use this APIs. > However, when tools that replicate hive metadata use this APIs on the target > cluster, creating these dirs on target side which cause the replication of > DFS-snapshots to fail. > So we if provide an option to bypass this creation of dirs, dfs replications > will be smoother. In the future we will need to restrict users that can use > these APIs. So we will have some sort of an authorization policy. -- This message was sent by Atlassian Jira (v8.20.10#820010)