This is an automated email from the ASF dual-hosted git repository.
yuqi4733 pushed a commit to branch branch-1.1
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.1 by this push:
new c3790b2889 [#9474] improvement(server): Reduce the number of calling
`catalogInuse` (#9558)
c3790b2889 is described below
commit c3790b2889c54e8fd1ce0dc74bb24c327be847c0
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Dec 25 21:09:16 2025 +0800
[#9474] improvement(server): Reduce the number of calling `catalogInuse`
(#9558)
### What changes were proposed in this pull request?
User proxy to reduce the number of calling `catalogInuse`.
### Why are the changes needed?
To improve API performance.
Fix: #9474
### Does this PR introduce _any_ user-facing change?
N/A.
### How was this patch tested?
Existing test.
Co-authored-by: Mini Yu <[email protected]>
---
.../java/org/apache/gravitino/GravitinoEnv.java | 57 ++++++++--
.../gravitino/catalog/OperationDispatcher.java | 4 -
.../catalog/OperationDispatcherInterceptor.java | 116 +++++++++++++++++++++
3 files changed, 166 insertions(+), 11 deletions(-)
diff --git a/core/src/main/java/org/apache/gravitino/GravitinoEnv.java
b/core/src/main/java/org/apache/gravitino/GravitinoEnv.java
index 34d5506420..aa58ddf855 100644
--- a/core/src/main/java/org/apache/gravitino/GravitinoEnv.java
+++ b/core/src/main/java/org/apache/gravitino/GravitinoEnv.java
@@ -19,6 +19,7 @@
package org.apache.gravitino;
import com.google.common.base.Preconditions;
+import java.lang.reflect.Proxy;
import org.apache.gravitino.audit.AuditLogManager;
import org.apache.gravitino.authorization.AccessControlDispatcher;
import org.apache.gravitino.authorization.AccessControlManager;
@@ -37,6 +38,7 @@ import
org.apache.gravitino.catalog.FilesetOperationDispatcher;
import org.apache.gravitino.catalog.ModelDispatcher;
import org.apache.gravitino.catalog.ModelNormalizeDispatcher;
import org.apache.gravitino.catalog.ModelOperationDispatcher;
+import org.apache.gravitino.catalog.OperationDispatcherInterceptor;
import org.apache.gravitino.catalog.PartitionDispatcher;
import org.apache.gravitino.catalog.PartitionNormalizeDispatcher;
import org.apache.gravitino.catalog.PartitionOperationDispatcher;
@@ -537,14 +539,28 @@ public class GravitinoEnv {
SchemaOperationDispatcher schemaOperationDispatcher =
new SchemaOperationDispatcher(catalogManager, entityStore,
idGenerator);
- SchemaHookDispatcher schemaHookDispatcher = new
SchemaHookDispatcher(schemaOperationDispatcher);
+ SchemaDispatcher schemaDispatcherProxy =
+ (SchemaDispatcher)
+ Proxy.newProxyInstance(
+ SchemaDispatcher.class.getClassLoader(),
+ new Class[] {SchemaDispatcher.class},
+ new OperationDispatcherInterceptor(
+ schemaOperationDispatcher, catalogManager, entityStore));
+ SchemaHookDispatcher schemaHookDispatcher = new
SchemaHookDispatcher(schemaDispatcherProxy);
SchemaNormalizeDispatcher schemaNormalizeDispatcher =
new SchemaNormalizeDispatcher(schemaHookDispatcher, catalogManager);
this.schemaDispatcher = new SchemaEventDispatcher(eventBus,
schemaNormalizeDispatcher);
TableOperationDispatcher tableOperationDispatcher =
new TableOperationDispatcher(catalogManager, entityStore, idGenerator);
- TableHookDispatcher tableHookDispatcher = new
TableHookDispatcher(tableOperationDispatcher);
+ TableDispatcher tableDispatcherProxy =
+ (TableDispatcher)
+ Proxy.newProxyInstance(
+ TableDispatcher.class.getClassLoader(),
+ new Class[] {TableDispatcher.class},
+ new OperationDispatcherInterceptor(
+ tableOperationDispatcher, catalogManager, entityStore));
+ TableHookDispatcher tableHookDispatcher = new
TableHookDispatcher(tableDispatcherProxy);
TableNormalizeDispatcher tableNormalizeDispatcher =
new TableNormalizeDispatcher(tableHookDispatcher, catalogManager);
this.tableDispatcher = new TableEventDispatcher(eventBus,
tableNormalizeDispatcher);
@@ -553,28 +569,55 @@ public class GravitinoEnv {
// partition doesn't have ownership, so we don't need it now.
PartitionOperationDispatcher partitionOperationDispatcher =
new PartitionOperationDispatcher(catalogManager, entityStore,
idGenerator);
+ PartitionDispatcher partitionDispatcherProxy =
+ (PartitionDispatcher)
+ Proxy.newProxyInstance(
+ PartitionDispatcher.class.getClassLoader(),
+ new Class[] {PartitionDispatcher.class},
+ new OperationDispatcherInterceptor(
+ partitionOperationDispatcher, catalogManager,
entityStore));
PartitionNormalizeDispatcher partitionNormalizeDispatcher =
- new PartitionNormalizeDispatcher(partitionOperationDispatcher,
catalogManager);
+ new PartitionNormalizeDispatcher(partitionDispatcherProxy,
catalogManager);
this.partitionDispatcher = new PartitionEventDispatcher(eventBus,
partitionNormalizeDispatcher);
FilesetOperationDispatcher filesetOperationDispatcher =
new FilesetOperationDispatcher(catalogManager, entityStore,
idGenerator);
- FilesetHookDispatcher filesetHookDispatcher =
- new FilesetHookDispatcher(filesetOperationDispatcher);
+ FilesetDispatcher filesetDispatcherProxy =
+ (FilesetDispatcher)
+ Proxy.newProxyInstance(
+ FilesetDispatcher.class.getClassLoader(),
+ new Class[] {FilesetDispatcher.class},
+ new OperationDispatcherInterceptor(
+ filesetOperationDispatcher, catalogManager, entityStore));
+ FilesetHookDispatcher filesetHookDispatcher = new
FilesetHookDispatcher(filesetDispatcherProxy);
FilesetNormalizeDispatcher filesetNormalizeDispatcher =
new FilesetNormalizeDispatcher(filesetHookDispatcher, catalogManager);
this.filesetDispatcher = new FilesetEventDispatcher(eventBus,
filesetNormalizeDispatcher);
TopicOperationDispatcher topicOperationDispatcher =
new TopicOperationDispatcher(catalogManager, entityStore, idGenerator);
- TopicHookDispatcher topicHookDispatcher = new
TopicHookDispatcher(topicOperationDispatcher);
+ TopicDispatcher topicDispatcherProxy =
+ (TopicDispatcher)
+ Proxy.newProxyInstance(
+ TopicDispatcher.class.getClassLoader(),
+ new Class[] {TopicDispatcher.class},
+ new OperationDispatcherInterceptor(
+ topicOperationDispatcher, catalogManager, entityStore));
+ TopicHookDispatcher topicHookDispatcher = new
TopicHookDispatcher(topicDispatcherProxy);
TopicNormalizeDispatcher topicNormalizeDispatcher =
new TopicNormalizeDispatcher(topicHookDispatcher, catalogManager);
this.topicDispatcher = new TopicEventDispatcher(eventBus,
topicNormalizeDispatcher);
ModelOperationDispatcher modelOperationDispatcher =
new ModelOperationDispatcher(catalogManager, entityStore, idGenerator);
- ModelHookDispatcher modelHookDispatcher = new
ModelHookDispatcher(modelOperationDispatcher);
+ ModelDispatcher modelDispatcherProxy =
+ (ModelDispatcher)
+ Proxy.newProxyInstance(
+ ModelDispatcher.class.getClassLoader(),
+ new Class[] {ModelDispatcher.class},
+ new OperationDispatcherInterceptor(
+ modelOperationDispatcher, catalogManager, entityStore));
+ ModelHookDispatcher modelHookDispatcher = new
ModelHookDispatcher(modelDispatcherProxy);
ModelNormalizeDispatcher modelNormalizeDispatcher =
new ModelNormalizeDispatcher(modelHookDispatcher, catalogManager);
this.modelDispatcher = new ModelEventDispatcher(eventBus,
modelNormalizeDispatcher);
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/OperationDispatcher.java
b/core/src/main/java/org/apache/gravitino/catalog/OperationDispatcher.java
index a9e4d73ef1..809efa926f 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/OperationDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/OperationDispatcher.java
@@ -92,8 +92,6 @@ public abstract class OperationDispatcher {
protected <R, E extends Throwable> R doWithCatalog(
NameIdentifier ident, ThrowableFunction<CatalogManager.CatalogWrapper,
R> fn, Class<E> ex)
throws E {
- catalogManager.checkCatalogInUse(store, ident);
-
try {
CatalogManager.CatalogWrapper c =
catalogManager.loadCatalogAndWrap(ident);
return fn.apply(c);
@@ -114,8 +112,6 @@ public abstract class OperationDispatcher {
Class<E1> ex1,
Class<E2> ex2)
throws E1, E2 {
- catalogManager.checkCatalogInUse(store, ident);
-
try {
CatalogManager.CatalogWrapper c =
catalogManager.loadCatalogAndWrap(ident);
return fn.apply(c);
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/OperationDispatcherInterceptor.java
b/core/src/main/java/org/apache/gravitino/catalog/OperationDispatcherInterceptor.java
new file mode 100644
index 0000000000..89d9ec958a
--- /dev/null
+++
b/core/src/main/java/org/apache/gravitino/catalog/OperationDispatcherInterceptor.java
@@ -0,0 +1,116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.catalog;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import org.apache.gravitino.EntityStore;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.lock.LockType;
+import org.apache.gravitino.lock.TreeLockUtils;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+
+/**
+ * {@code OperationDispatcherInterceptor} is an invocation handler that
intercepts method calls to
+ * an operation dispatcher to perform catalog usage checks before proceeding
with the actual method
+ * invocation.
+ *
+ * <p>Note: This interceptor will only intercept methods in
+ *
+ * <p>SchemaDispatcher
+ *
+ * <p>TableDispatch
+ *
+ * <p>FilesetDispatch
+ *
+ * <p>ModelDispatch
+ *
+ * <p>TopicDispatch
+ *
+ * <p>PartitionDispatch
+ */
+public class OperationDispatcherInterceptor implements InvocationHandler {
+ private final Object dispatcher;
+ private final CatalogManager catalogManager;
+ private final EntityStore store;
+
+ /**
+ * An {@link InvocationHandler} implementation that intercepts method calls
on dispatcher objects
+ * in the Gravitino catalog system. This class is used as part of the
dynamic proxy pattern to
+ * wrap dispatcher instances, enabling pre-processing logic such as catalog
existence checks and
+ * tree-based locking before delegating the actual method invocation to the
underlying dispatcher.
+ *
+ * <p>For each intercepted method call, if the first argument is a {@link
NameIdentifier} or
+ * {@link Namespace}, the interceptor extracts the catalog identifier and
acquires a read lock on
+ * the catalog using {@link TreeLockUtils}. It then checks if the catalog is
in use via the {@link
+ * CatalogManager}. Only after these checks and locks does it invoke the
original method on the
+ * dispatcher.
+ *
+ * <p>This mechanism ensures that all dispatcher operations are performed
safely and consistently
+ * with respect to catalog state and concurrency requirements.
+ */
+ public OperationDispatcherInterceptor(
+ Object dispatcher, CatalogManager catalogManager, EntityStore store) {
+ this.dispatcher = dispatcher;
+ this.catalogManager = catalogManager;
+ this.store = store;
+ }
+
+ @Override
+ public Object invoke(Object proxy, Method method, Object[] args) throws
Throwable {
+ if (args != null && args.length > 0) {
+ NameIdentifier catalogIdent = null;
+ if (args[0] instanceof NameIdentifier ident) {
+ catalogIdent = NameIdentifierUtil.getCatalogIdentifier(ident);
+ } else if (args[0] instanceof Namespace ns) {
+ if (ns.length() >= 2) {
+ catalogIdent = NameIdentifier.of(ns.level(0), ns.level(1));
+ }
+ }
+
+ if (catalogIdent != null) {
+ final NameIdentifier finalCatalogIdent = catalogIdent;
+ // Note: In this implementation, the catalog-in-use check is performed
separately
+ // under a tree lock before invoking the main operation. In the
original code,
+ // this check may have been performed as part of a single, monolithic
operation.
+ // This separation ensures that the catalog's state is validated under
the appropriate
+ // lock, improving thread safety and consistency. However, it
introduces a trade-off:
+ // the check and the main operation are not atomic with respect to
each other, so there
+ // is a small window where the catalog's state could change between
the check and the
+ // operation. This approach was chosen to avoid holding locks during
potentially
+ // long-running operations, balancing safety and performance.
+ TreeLockUtils.doWithTreeLock(
+ catalogIdent,
+ LockType.READ,
+ () -> {
+ catalogManager.checkCatalogInUse(store, finalCatalogIdent);
+ return null;
+ });
+ }
+ }
+
+ try {
+ return method.invoke(dispatcher, args);
+ } catch (InvocationTargetException e) {
+ throw e.getTargetException();
+ }
+ }
+}
