This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 233359d503 [#10218] Improvement(server): Fix misleading success log
when metalake drop returns false (#10813)
233359d503 is described below
commit 233359d503ba45dacd64d648445f35ac27166fa5
Author: Gaurav Rudra <[email protected]>
AuthorDate: Fri Apr 24 00:36:07 2026 -0700
[#10218] Improvement(server): Fix misleading success log when metalake drop
returns false (#10813)
### What changes were proposed in this pull request?
The success log statement in drop/purge REST handlers for Metalake,
Table, Fileset, Topic, and Partition was unconditionally emitted after
returning Utils.ok(new DropResponse(dropped)), regardless of whether the
operation actually succeeded. This PR restructures the logging in all
four handlers so that:
- An INFO log is emitted only when dropped == true (the entity was
actually removed).
- A WARN log is emitted when dropped == false (the entity was not
found).
- The intermediate Response response variable is eliminated, returning
Utils.ok(...) inline.
Files changed: MetalakeOperations.java, FilesetOperations.java,
PartitionOperations.java, TableOperations.java, TopicOperations.java.
### Why are the changes needed?
Previously, the success log (e.g. "Fileset dropped: ...") was always
printed after a drop request — even when the dispatcher returned false,
meaning the entity did not exist and nothing was actually dropped. This
produced misleading log output where operators would see a success
message alongside a warning, making it hard to diagnose whether an
entity was truly removed. The fix ensures log messages accurately
reflect the outcome of the operation.
### Does this PR introduce _any_ user-facing change?
No. This is a server-side logging fix only.
fix: #10218
fix: #10216
### How was this patch tested?
Existing unit and integration tests cover the drop path. No new behavior
was introduced, so no new tests were added.
---
.../server/web/rest/FilesetOperations.java | 8 ++++----
.../server/web/rest/MetalakeOperations.java | 8 ++++----
.../server/web/rest/PartitionOperations.java | 23 +++++++++++-----------
.../gravitino/server/web/rest/TableOperations.java | 20 +++++++++----------
.../gravitino/server/web/rest/TopicOperations.java | 9 ++++-----
5 files changed, 34 insertions(+), 34 deletions(-)
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java
index 5dcc3833c6..eda03954fd 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java
@@ -337,13 +337,13 @@ public class FilesetOperations {
() -> {
NameIdentifier ident = NameIdentifierUtil.ofFileset(metalake,
catalog, schema, fileset);
boolean dropped = dispatcher.dropFileset(ident);
- if (!dropped) {
+ if (dropped) {
+ LOG.info("Fileset dropped: {}.{}.{}.{}", metalake, catalog,
schema, fileset);
+ } else {
LOG.warn("Cannot find to be dropped fileset {} under schema {}",
fileset, schema);
}
- Response response = Utils.ok(new DropResponse(dropped));
- LOG.info("Fileset dropped: {}.{}.{}.{}", metalake, catalog,
schema, fileset);
- return response;
+ return Utils.ok(new DropResponse(dropped));
});
} catch (Exception e) {
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java
index 4424d6b133..a02eea506b 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java
@@ -257,13 +257,13 @@ public class MetalakeOperations {
() -> {
NameIdentifier identifier =
NameIdentifierUtil.ofMetalake(metalakeName);
boolean dropped = metalakeDispatcher.dropMetalake(identifier,
force);
- if (!dropped) {
+ if (dropped) {
+ LOG.info("Metalake dropped: {}", metalakeName);
+ } else {
LOG.warn("Failed to drop metalake by name {}", metalakeName);
}
- Response response = Utils.ok(new DropResponse(dropped));
- LOG.info("Metalake dropped: {}", metalakeName);
- return response;
+ return Utils.ok(new DropResponse(dropped));
});
} catch (Exception e) {
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/PartitionOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/PartitionOperations.java
index 85aebb6fdd..d96bf34863 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/PartitionOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/PartitionOperations.java
@@ -245,23 +245,24 @@ public class PartitionOperations {
purge
? dispatcher.purgePartition(tableIdent, partition)
: dispatcher.dropPartition(tableIdent, partition);
- if (!dropped) {
+ if (dropped) {
+ LOG.info(
+ "Partition {} {} in table {}.{}.{}.{}",
+ partition,
+ purge ? "purged" : "dropped",
+ metalake,
+ catalog,
+ schema,
+ table);
+ } else {
LOG.warn(
"Failed to drop partition {} under table {} under schema {}",
partition,
table,
schema);
}
- Response response = Utils.ok(new DropResponse(dropped));
- LOG.info(
- "Partition {} {} in table {}.{}.{}.{}",
- partition,
- purge ? "purged" : "dropped",
- metalake,
- catalog,
- schema,
- table);
- return response;
+
+ return Utils.ok(new DropResponse(dropped));
});
} catch (Exception e) {
return ExceptionHandlers.handlePartitionException(OperationType.DROP,
"", table, e);
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java
index e0c807bd52..0940abb458 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java
@@ -266,19 +266,19 @@ public class TableOperations {
() -> {
NameIdentifier ident = NameIdentifierUtil.ofTable(metalake,
catalog, schema, table);
boolean dropped = purge ? dispatcher.purgeTable(ident) :
dispatcher.dropTable(ident);
- if (!dropped) {
+ if (dropped) {
+ LOG.info(
+ "Table {}: {}.{}.{}.{}",
+ purge ? "purge" : "drop",
+ metalake,
+ catalog,
+ schema,
+ table);
+ } else {
LOG.warn("Cannot find to be dropped table {} under schema {}",
table, schema);
}
- Response response = Utils.ok(new DropResponse(dropped));
- LOG.info(
- "Table {}: {}.{}.{}.{}",
- purge ? "purge" : "drop",
- metalake,
- catalog,
- schema,
- table);
- return response;
+ return Utils.ok(new DropResponse(dropped));
});
} catch (Exception e) {
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.java
index dfa8f322e2..beedcac793 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.java
@@ -257,14 +257,13 @@ public class TopicOperations {
LOG.info("Dropping topic under schema: {}.{}.{}", metalake,
catalog, schema);
NameIdentifier ident = NameIdentifierUtil.ofTopic(metalake,
catalog, schema, topic);
boolean dropped = dispatcher.dropTopic(ident);
-
- if (!dropped) {
+ if (dropped) {
+ LOG.info("Topic dropped: {}.{}.{}.{}", metalake, catalog,
schema, topic);
+ } else {
LOG.warn("Cannot find to be dropped topic {} under schema {}",
topic, schema);
}
- Response response = Utils.ok(new DropResponse(dropped));
- LOG.info("Topic dropped: {}.{}.{}.{}", metalake, catalog, schema,
topic);
- return response;
+ return Utils.ok(new DropResponse(dropped));
});
} catch (Exception e) {
return ExceptionHandlers.handleTopicException(OperationType.DROP, topic,
schema, e);
