github-actions[bot] commented on code in PR #63897:
URL: https://github.com/apache/doris/pull/63897#discussion_r3338606000
##########
fe/be-java-extensions/java-common/src/main/java/org/apache/doris/common/classloader/ScannerLoader.java:
##########
@@ -116,15 +130,57 @@ public static UdfClassCache getUdfClassLoader(String
functionSignature) {
return udfLoadedClasses.get(functionSignature);
}
- public static synchronized void cacheClassLoader(String functionSignature,
UdfClassCache classCache,
+ /**
+ * Cache the UDF class metadata for the given function signature.
+ *
+ * <p>Insertion is atomic via {@link Map#putIfAbsent}: if another executor
thread has
+ * already published a cache entry for {@code functionSignature}, 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>
+ *
+ * @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);
- udfLoadedClasses.put(functionSignature, classCache, expirationTime *
60 * 1000L);
+ LOG.info("Cache UDF for: " + functionSignature);
+ UdfClassCache existing =
udfLoadedClasses.putIfAbsent(functionSignature, classCache);
+ if (existing == null) {
Review Comment:
`putIfAbsent` is atomic against another first-time loader, but it is not
coordinated with `cleanUdfClassLoader()`. A DROP FUNCTION clean task can run
after an executor has observed a miss and started building the old static UDF
cache, but before this line publishes it; `cleanUdfClassLoader()` then removes
nothing, and this later `putIfAbsent` resurrects the dropped function's
`UdfClassCache`. Since this PR removes time-based eviction, that stale
classloader can remain indefinitely and be reused if the same signature is
created again with a different jar/checksum. Please make DROP cleanup establish
a generation/tombstone or otherwise prevent a cache built before the clean from
being inserted after the clean has completed.
--
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]
