utafrali commented on code in PR #14264:
URL: https://github.com/apache/gravitino/pull/14264#discussion_r4380088477
##########
iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableOperationExecutor.java:
##########
@@ -164,7 +165,21 @@ public void dropTable(
metadata.metadataFileLocation(),
wrapper.fileIOImpl(),
wrapper.fileIOProperties(),
- context.userName()));
+ context.userName());
+ wrapper.dropTable(tableIdentifier);
+ try {
+ manager.addJob(job);
+ } catch (RuntimeException e) {
+ // The table is already dropped from the catalog, so the drop
succeeded. Its files are
+ // orphaned; the metadata location lets an operator purge them.
+ LOG.error(
+ "Dropped table {} in catalog {} but failed to enqueue its
async purge; its data"
+ + " files are not deleted. Purge them from metadata {}",
+ tableIdentifier,
+ context.catalogName(),
+ metadata.metadataFileLocation(),
Review Comment:
`metadata.metadataFileLocation()` is invoked a second time here, but `job`
already holds the same value via `job.metadataLocation()` (Lombok fluent
getter). Using the job's captured field is slightly cleaner and makes it
explicit that the logged location is identical to what was submitted to the
failed enqueue:
```java
LOG.error(
"Dropped table {} in catalog {} but failed to enqueue its async purge;
its data"
+ " files are not deleted. Purge them from metadata {}",
tableIdentifier,
context.catalogName(),
job.metadataLocation(),
e);
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
