Skip to content
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

[improve][fn] Cherry-picked commit Optimize Function Worker startup apache#22122 #228

Closed
wants to merge 1 commit into from

Conversation

nikhil-ctds
Copy link
Collaborator

Motivation

Cherry-picking commit from Apache for optimizing functions worker

Master Issue: apache#22122

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (yes / no)
  • The public API: (yes / no)
  • The schema: (yes / no / don't know)
  • The default values of configurations: (yes / no)
  • The wire protocol: (yes / no)
  • The rest endpoints: (yes / no)
  • The admin cli options: (yes / no)
  • Anything that affects deployment: (yes / no / don't know)

Documentation

Check the box below or label this PR directly (if you have committer privilege).

Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

…rect zip/bytecode access (apache#22122)

(cherry picked from commit bbc6224)
(cherry picked from commit 597cfa1)

(cherry picked from commit d608bfc)

 Conflicts:
	pulsar-broker/pom.xml
	pulsar-functions/localrun/src/main/java/org/apache/pulsar/functions/LocalRunner.java
	pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/thread/ThreadRuntime.java
	pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/thread/ThreadRuntimeFactory.java
	pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/worker/FunctionsManager.java
	pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils/FunctionCommon.java
	pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils/FunctionConfigUtils.java
	pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils/SinkConfigUtils.java
	pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils/functions/FunctionArchive.java
	pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils/functions/FunctionUtils.java
	pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils/io/ConnectorUtils.java
	pulsar-functions/utils/src/test/java/org/apache/pulsar/functions/utils/FunctionCommonTest.java
	pulsar-functions/utils/src/test/java/org/apache/pulsar/functions/utils/FunctionConfigUtilsTest.java
	pulsar-functions/utils/src/test/java/org/apache/pulsar/functions/utils/SinkConfigUtilsTest.java
	pulsar-functions/worker/pom.xml
	pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/rest/api/ComponentImpl.java
	pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/rest/api/FunctionsImpl.java
	pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/rest/api/SinksImpl.java
	pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/rest/api/SourcesImpl.java
	pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/FunctionsImplTest.java
	pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/v2/FunctionApiV2ResourceTest.java
	pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/v3/FunctionApiV3ResourceTest.java
	pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/v3/SinkApiV3ResourceTest.java
@nikhil-ctds nikhil-ctds self-assigned this Mar 5, 2024
@@ -119,24 +118,24 @@ private static ClassLoader getFunctionClassLoader(InstanceConfig instanceConfig,
String narExtractionDirectory,
FunctionCacheManager fnCache,
Optional<ConnectorsManager> connectorsManager,
Optional<FunctionsManager> functionsManager,
Function.FunctionDetails.ComponentType componentType)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicoloboschi , On apache 2_10 component type is initialised in the function and not using the argument.
Can you have a look and let me know if its okay to remove the componentType parameter from the function.

The 2 places where this function getFunctionClassLoader is called are -

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as long as it comes from the same logic, it's okay to drop the function arg

@@ -399,8 +404,7 @@ public static SinkConfig convertFromDetails(FunctionDetails functionDetails) {
}

public static ExtractedSinkDetails validateAndExtractDetails(SinkConfig sinkConfig,
ClassLoader sinkClassLoader,
ClassLoader functionClassLoader,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicoloboschi , In apache we are not validating functionClassLoader,
FunctionClassLoader and functionClass name is used to resolve typeArg and inputClassLoader, but in apache, we are using typeArg is defined and we are using inputFunction.getTypePool() instead of inputClassLoader at the validation points.

Can you have a look if removing the param breaks any functionality?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This difference comes from the fact that LS 2.10 has PIP-193 in it while AP 2.10 hasn't (it's only available since 3.0.0)

We should apply the same fix to both the functions (both the sink and the pre-processing)
In this case the sinkClassLoader is the sink and functionClassLoader is the preprocessing function

@nikhil-ctds nikhil-ctds closed this Mar 7, 2024
@nikhil-ctds nikhil-ctds deleted the 2.10_ds_cherry-pick branch April 24, 2024 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants