github-actions[bot] commented on code in PR #67046:
URL: https://github.com/apache/doris/pull/67046#discussion_r3839585987
##########
fe/be-java-extensions/java-common/src/main/java/org/apache/doris/common/classloader/ScannerLoader.java:
##########
@@ -126,59 +128,82 @@ public void loadAllScannerJars() {
LOG.info("Finished loading scanner JARs");
}
- public static UdfClassCache getUdfClassLoader(String functionSignature) {
- return udfLoadedClasses.get(functionSignature);
+ private static class UdfClassCacheEntry {
+ private final String functionSignature;
+ private final UdfClassCache classCache;
+
+ UdfClassCacheEntry(String functionSignature, UdfClassCache classCache)
{
+ this.functionSignature = functionSignature;
+ this.classCache = classCache;
+ }
+ }
+
+ public static UdfClassCache getUdfClassLoader(long functionId) {
+ UdfClassCacheEntry entry = udfLoadedClasses.get(functionId);
+ return entry == null ? null : entry.classCache;
}
/**
- * Cache the UDF class metadata for the given function signature.
+ * Cache the UDF class metadata for the given catalog function id.
*
- * <p>Insertion is atomic via {@link Map#putIfAbsent}: if another executor
thread has
- * already published a cache entry for {@code functionSignature}, the
{@code classCache}
+ * <p>Insertion is atomic via {@link Map#putIfAbsent}: if another executor
+ * thread has already published a cache entry for {@code functionId}, the
{@code classCache}
* argument is treated as a redundant build and closed here (it has not
yet been handed
* to any executor, so closing its URLClassLoader is safe). The
already-published entry
* is returned to the caller so the current executor can switch to it.</p>
*
* <p>The {@code expirationTime} parameter is kept for backward
compatibility with the
* existing call sites and DDL property {@code expiration_time}, but is no
longer used:
* cached entries are not evicted by time. Removal happens only via
- * {@link #cleanUdfClassLoader(String)} on DROP FUNCTION.</p>
+ * {@link #cleanUdfClassLoader(String, long)} on DROP FUNCTION.</p>
*
* @return the {@link UdfClassCache} actually held in the map after this
call —
* either {@code classCache} (we won the race) or the pre-existing
entry
* (another thread won; {@code classCache} has been closed and
must not be used).
*/
- public static UdfClassCache cacheClassLoader(String functionSignature,
UdfClassCache classCache,
- long expirationTime) {
- LOG.info("Cache UDF for: " + functionSignature);
- UdfClassCache existing =
udfLoadedClasses.putIfAbsent(functionSignature, classCache);
+ public static UdfClassCache cacheClassLoader(String functionSignature,
long functionId,
+ UdfClassCache classCache, long expirationTime) {
+ LOG.info("Cache UDF for function signature: {}, function id: {}",
functionSignature, functionId);
+ UdfClassCacheEntry newEntry = new
UdfClassCacheEntry(functionSignature, classCache);
+ UdfClassCacheEntry existing = udfLoadedClasses.putIfAbsent(functionId,
newEntry);
if (existing == null) {
return classCache;
}
// Lost the race against a concurrent first-time load. The cache we
just built has
// never been exposed to any executor, so closing its URLClassLoader
here cannot
// affect anyone. Do NOT touch `existing` — another executor may
already be using it.
- try {
- classCache.close();
- } catch (Exception e) {
- LOG.warn("Failed to close redundant UdfClassCache for " +
functionSignature, e);
+ closeUdfClassLoader(functionId, newEntry);
+ return existing.classCache;
+ }
+
+ public void cleanUdfClassLoader(String functionSignature, long functionId)
{
+ boolean dropByFunctionId = functionId > 0;
+ LOG.info("cleanUdfClassLoader for function signature: {}, function id:
{}, drop by function id: {}",
+ functionSignature, functionId, dropByFunctionId);
+ if (dropByFunctionId) {
+ closeUdfClassLoader(functionId,
udfLoadedClasses.remove(functionId));
Review Comment:
[P2] Use the ID of the generation actually removed
This branch assumes `functionId` is the function that `DROP` just removed,
but `DropFunctionCommand` obtains the ID and calls `dropFunction()` in two
separate synchronized manager calls. D1 can read V1/ID 10; another session can
drop V1, create and execute V2/ID 11; then D1 drops V2 while still submitting
ID 10. This removes only V1's entry, while V2 has no metadata left to trigger
another cleanup and, with expiration disabled, its classloader remains for the
BE lifetime. Please make the catalog/global drop return the exact removed ID or
IDs atomically and build cleanup tasks from that result.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
