-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fixed UDF jar metadata handling in UDFInfo when multiple UDFs share the same jar
#17732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,6 +68,7 @@ | |
|
|
||
| private final UDFTable udfTable; | ||
| private final Map<String, String> existedJarToMD5; | ||
| private final Map<String, Integer> existedJarToReferenceCount; | ||
|
|
||
| private final UDFExecutableManager udfExecutableManager; | ||
|
|
||
|
|
@@ -78,6 +79,7 @@ | |
| public UDFInfo() throws IOException { | ||
| udfTable = new UDFTable(); | ||
| existedJarToMD5 = new HashMap<>(); | ||
| existedJarToReferenceCount = new HashMap<>(); | ||
| udfExecutableManager = | ||
| UDFExecutableManager.setupAndGetInstance( | ||
| CONFIG_NODE_CONF.getUdfTemporaryLibDir(), CONFIG_NODE_CONF.getUdfDir()); | ||
|
|
@@ -135,7 +137,7 @@ | |
| final UDFInformation udfInformation = physicalPlan.getUdfInformation(); | ||
| udfTable.addUDFInformation(udfInformation.getFunctionName(), udfInformation); | ||
| if (udfInformation.isUsingURI()) { | ||
| existedJarToMD5.put(udfInformation.getJarName(), udfInformation.getJarMD5()); | ||
| addJarReference(udfInformation.getJarName(), udfInformation.getJarMD5()); | ||
| if (physicalPlan.getJarFile() != null) { | ||
| udfExecutableManager.saveToInstallDir( | ||
| ByteBuffer.wrap(physicalPlan.getJarFile().getValues()), udfInformation.getJarName()); | ||
|
|
@@ -185,7 +187,10 @@ | |
|
|
||
| public TSStatus dropFunction(Model model, String functionName) { | ||
| if (udfTable.containsUDF(model, functionName)) { | ||
| existedJarToMD5.remove(udfTable.getUDFInformation(model, functionName).getJarName()); | ||
| final UDFInformation udfInformation = udfTable.getUDFInformation(model, functionName); | ||
| if (udfInformation.isUsingURI()) { | ||
| removeJarReference(udfInformation.getJarName()); | ||
| } | ||
| udfTable.removeUDFInformation(model, functionName); | ||
| } | ||
| return new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode()); | ||
|
|
@@ -248,6 +253,7 @@ | |
| deserializeExistedJarToMD5(fileInputStream); | ||
|
|
||
| udfTable.deserializeUDFTable(fileInputStream); | ||
| rebuildJarMetadataFromUDFTable(); | ||
| } finally { | ||
| releaseUDFTableLock(); | ||
| } | ||
|
|
@@ -272,6 +278,32 @@ | |
|
|
||
| public void clear() { | ||
| existedJarToMD5.clear(); | ||
| existedJarToReferenceCount.clear(); | ||
| udfTable.clear(); | ||
| } | ||
|
|
||
| private void addJarReference(String jarName, String jarMD5) { | ||
| existedJarToMD5.putIfAbsent(jarName, jarMD5); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. putIfAbsent keeps the first MD5 for a jar name but always increments the reference count. That is fine when validate() runs before addUDFInTable, but if addJarReference is ever called without that check, ref count and MD5 could diverge. Consider rejecting a conflicting MD5 inside addJarReference (same check as in validate() at lines 107-115) so this helper is safe on its own. |
||
| existedJarToReferenceCount.merge(jarName, 1, Integer::sum); | ||
| } | ||
|
|
||
| private void removeJarReference(String jarName) { | ||
| final Integer referenceCount = existedJarToReferenceCount.get(jarName); | ||
| if (referenceCount == null || referenceCount <= 1) { | ||
| existedJarToReferenceCount.remove(jarName); | ||
| existedJarToMD5.remove(jarName); | ||
| return; | ||
| } | ||
| existedJarToReferenceCount.put(jarName, referenceCount - 1); | ||
| } | ||
|
|
||
| private void rebuildJarMetadataFromUDFTable() { | ||
|
Check warning on line 300 in iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/UDFInfo.java
|
||
| existedJarToMD5.clear(); | ||
| existedJarToReferenceCount.clear(); | ||
| for (UDFInformation udfInformation : udfTable.getAllInformationList()) { | ||
| if (udfInformation.isUsingURI()) { | ||
| addJarReference(udfInformation.getJarName(), udfInformation.getJarMD5()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After deserializeExistedJarToMD5, rebuildJarMetadataFromUDFTable() clears both maps and rebuilds from udfTable. That makes udfTable the source of truth on load.
A short comment here explaining that deserialized existedJarToMD5 is intentionally discarded would help future readers.