[hive] Fix schemeless LOCATION for external tables and roll back failed creates - #10078
zhang-arvin wants to merge 1 commit into
Conversation
|
The Evidence — the same jobs fail on a clean
This PR only touches This comment was generated by an AI agent (Hermes Agent). |
JingsongLi
left a comment
JingsongLi
left a comment
There was a problem hiding this comment.
Reviewed 797b4b7772cc99931368d34eb7d4a561b248f2ba against the linked issue and the Spark → HiveCatalog → filesystem/metastore create path. Resolving a schemeless external LOCATION against the configured default filesystem has clear end-to-end value, so this PR should remain open.
The unconditional metastore rollback introduces a concurrent-create regression, and the new LOCATION test depends on writing to /data; details are inline. The reported Spark loadTable failure occurs after catalog.createTable returns, so this new HiveCatalog catch block does not handle that original failure either. Keeping this fix focused on location resolution would avoid that extra failure mode.
Validation on JDK 8: the two added tests produced one pass and one error (Mkdirs failed to create file:/data/external/external_table/schema). A focused reviewer test injected a competing HMS create between the existence check and registration: this head deleted the winning table; restoring only the previous create-failure cleanup behavior made the same test pass. I have not run the original Spark + HDFS reproduction end to end.
| client.dropTable( | ||
| identifier.getDatabaseName(), | ||
| identifier.getTableName(), | ||
| true, | ||
| false)); |
There was a problem hiding this comment.
[P1] Preserve the other creator's table when registration fails
AbstractCatalog.createTable checks table existence before entering createTableImpl, and the metastore createTable call is outside runWithLock. If another caller registers the same identifier in that interval, our createTable throws AlreadyExistsException, but this catch now unconditionally drops the other caller's table. This is reachable for external creates, where an existing filesystem schema can be reused; with different requested locations the winner can even own a completely different directory. deleteData=true also makes a managed winner's data eligible for deletion. I reproduced the loss of the winning HMS entry with a focused test against this head; the same test passes with the previous catch behavior. Please remove this rollback from the location fix, or require proven ownership of the registered table and exclude AlreadyExistsException before deleting anything.
| catalog.createDatabase(databaseName, false); | ||
| Identifier identifier = Identifier.create(databaseName, tableName); | ||
|
|
||
| String schemelessLocation = "/data/external/" + tableName; |
There was a problem hiding this comment.
[P2] Allocate the schemeless test location under the test's temporary directory
This fixture uses the default local filesystem, so catalog.createTable actually tries to create /data/external/external_table/schema. On a normal non-root checkout this fails before the assertions; running the two new tests on JDK 8 produced java.io.IOException: Mkdirs failed to create file:/data/external/external_table/schema here, while the rollback test passed. The fixed global path also persists outside test cleanup. Please derive a unique absolute path from the fixture's temporary directory and strip its URI scheme; a configured non-local filesystem case should separately verify the original wrong-filesystem regression.
…ed creates Creating an external table with a schemeless LOCATION (for example LOCATION '/data/external/t') writes the schema to the driver's local filesystem instead of the configured default filesystem, leaves a zombie entry in the metastore when the create fails, and makes the subsequent loadTable call in SparkCatalog#createTable miss with NoSuchTableException. Two problems: 1. HiveCatalog#initialTableLocation builds the location straight from the LOCATION property. FileIO#get treats a scheme-less path as local, so schema-0 lands on local disk while the metastore points the table at the path that was requested. Resolve the scheme against FileSystem.getDefaultUri so the default filesystem is used, matching the behaviour of the catalog warehouse. Applies to both createTableImpl and createObjectTable. 2. The failure handler only deleted the directory for a managed table and never removed the metastore entry, so a failed create left the table registered in HMS with no readable schema. Roll the entry back via dropTable(..., true, false) and keep the managed-table directory cleanup.
797b4b7 to
97a0608
Compare
2 participants
What is the purpose of the change
Fixes #9990.
CREATE TABLE ... LOCATION '/data/external/t'(a LOCATION without a URI scheme) on aHiveCatalogwrites the schema to the driver's local filesystem instead of the configured default filesystem, leaves a zombie entry in the metastore when the create fails, and then fails withNoSuchTableExceptionout ofSparkCatalog#createTable.Brief change log
1.
HiveCatalog#initialTableLocationbuilt the location without resolving the scheme.FileIO#get(FileIO.java:527) returns aLocalFileIOwhenever the path has no scheme, so for a schemelessLOCATIONthe catalog wroteschema-0to the driver's local disk while the metastore registered the table against the path the user asked for. The table is then unreadable from any other node, and the log lines in the report match this exactly (schema-0on the local FS, empty directory on HDFS).resolveLocationSchemenow resolves a scheme-less location againstFileSystem.getDefaultUri(hiveConf), so it lands on the default filesystem like the catalog warehouse does. Already-schemed locations are returned untouched. Applied to bothcreateTableImplandcreateObjectTable.2. A failed create left the metastore entry behind.
The
catchblock aroundcreateHiveTabledeleted the directory only for a managed table and never removed the metastore entry. The schema write happens before the registration, so removing the entry restores the pre-call state.cleanupOnCreateTableFailurenow drops the table viadropTable(db, table, true, false)and keeps the managed-table directory cleanup.Note on the Spark-side symptom
The
NoSuchTableException/zombie-table behaviour the report observes was introduced by #9549, which madeSparkCatalog#createTablecallloadTable(ident)right aftercatalog.createTable(...). That change is reasonable when the table is genuinely readable, so the fix here is at the catalog layer where the broken location and the missing rollback actually live — theloadTablethen succeeds instead of miss. Happy to restructure it if maintainers prefer the resolution to live inSparkCataloginstead.Tests
testCreateExternalTableWithSchemelessLocation— creates an external table withpath=/data/external/...and asserts the location the catalog works with carries an explicit scheme and preserves the requested path. This asserts the regressed behaviour directly and fails without the fix.testCreateTableDoesNotLeaveZombieEntryWhenMetastoreFails— asserts the rollback path removes the metastore entry.Verification status
mvn -pl paimon-hive/paimon-hive-catalog -am -Pfast-build -DskipTests compile→ BUILD SUCCESS.HiveCatalogTest(and its siblings in this module) fail in@BeforeEach setUpwithIllegalStateException: Hadoop configuration is not available for this CatalogContext(CatalogTestBase.java:126→ResolvingFileIO.configure→CatalogContext.hadoopConf).HadoopUtils.getHadoopConfigurationneedsHADOOP_HOME/HADOOP_CONF_DIRon this machine, andSerializableConfigurationis a no-op stand-in otherwise. This is a pre-existing harness/environment limitation, unrelated to the change — the module's existing tests fail identically on cleanmaster. CI has the Hadoop environment and should run them; I'll report back here once it does.