Skip to content

Conversation

@walkinggo
Copy link
Contributor

Main Issue: #23723

Motivation

see #23723 . The main work of this PR is to add a method for setting the memory limit of the Pulsar client in a Pulsar Function.

Modifications

1.Memory Limit Support:
Added MemoryLimit class to define memory constraints for Pulsar clients, which includes both an absolute value and a percentage of the maximum direct memory.
This memory configuration is applied across several components including FunctionConfig, WorkerConfig, InstanceConfig, and ThreadRuntimeFactory.
2.Class Updates:
Introduced new fields in configuration classes such as FunctionConfig, WorkerConfig, and InstanceConfig to hold memory limit settings.
Updated ThreadRuntimeFactory to handle the memory limit during initialization, ensuring that the client memory limit is respected.
3.Proto File Changes:
Updated Function.proto to include a new MemoryLimit field, allowing the configuration of memory limits directly via function details.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: walkinggo#8

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Feb 11, 2025
@lhotari
Copy link
Member

lhotari commented Feb 12, 2025

@walkinggo Please first make sure that the CI build passes in your own fork. I see that walkinggo#8 referenced in the PR description isn't passing.

@walkinggo
Copy link
Contributor Author

@walkinggo Please first make sure that the CI build passes in your own fork. I see that walkinggo#8 referenced in the PR description isn't passing.

The error in my PR is related to json-smart, but my code doesn't involve modifications to this component. I suspect there might be an issue with Maven Central, and I also noticed that this PR is updating the version of json-smart.

@lhotari
Copy link
Member

lhotari commented Feb 12, 2025

@walkinggo Please first make sure that the CI build passes in your own fork. I see that walkinggo#8 referenced in the PR description isn't passing.

The error in my PR is related to json-smart, but my code doesn't involve modifications to this component. I suspect there might be an issue with Maven Central, and I also noticed that this PR is updating the version of json-smart.

@walkinggo PR #23966 has been merged. Please merge origin/master to your PR to get the changes included.

@walkinggo
Copy link
Contributor Author

@walkinggo Please first make sure that the CI build passes in your own fork. I see that walkinggo#8 referenced in the PR description isn't passing.

The error in my PR is related to json-smart, but my code doesn't involve modifications to this component. I suspect there might be an issue with Maven Central, and I also noticed that this PR is updating the version of json-smart.

@walkinggo PR #23966 has been merged. Please merge origin/master to your PR to get the changes included.

The PR has been successfully merged and passed the CI checks.

@walkinggo walkinggo requested a review from lhotari February 13, 2025 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants