Skip to content

Conversation

@shibd
Copy link
Member

@shibd shibd commented Sep 25, 2025

Motivation

To support a custom router in the Node.js client, the most direct approach would be to call the user-provided router function from within the C++ callback. However, because the C++ client's sendAsync method has a synchronous component for partition selection before it becomes fully asynchronous, this implementation causes a deadlock between the Node.js main thread and the C++ callback thread. This is due to the single-threaded nature of the Node.js event loop.

https://github.com/apache/pulsar-client-cpp/blob/90ea3695b80660f837785d98e96047d90de3f64f/lib/PartitionedProducerImpl.cc#L209-L220

You can understand by reviewing the PR and running the associated tests.

Modifications

This PR achieves the goal by calling the user's router function at the JS layer and then passing the resulting partition index to the C++ layer via message properties.

  1. The send method in Producer.js is wrapped. It now first calls the user's router function to asynchronously determine the target partition.
  2. The resulting partition index is added to the message's properties under the special key: __partition__.
  3. Inside the N-API implementation, a fixed, synchronous C++ function (internalCppMessageRouter) is now used, which simply reads this property and returns the partition index to the underlying C++ client.

Points for Discussion

This implementation introduces two points for discussion:

  1. We need to maintain a cache for the number of partitions (numPartitions) in Producer.js, which is then passed to the user's router function. (Currently, this cache has a 60-second TTL. A potential future improvement is to expose a method from the C++ producer to get the partition count directly. This would allow us to remove the JS-side cache, as the C++ client internally handles partition metadata updates.)
  2. This introduces a special property (__partition__) in the message properties, which could potentially be confusing for users. However, I believe this is not a major issue at the moment.

Verifying this change

  • The Message Routing test case has been added to cover this change.

Documentation

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@shibd shibd self-assigned this Sep 25, 2025
@shibd
Copy link
Member Author

shibd commented Sep 26, 2025

I will push a separate PR to fix failed CI.

@shibd shibd marked this pull request as ready for review September 26, 2025 03:35
@BewareMyPower BewareMyPower changed the title Support custome router for producer Support custom router for producer Sep 26, 2025
Copy link
Contributor

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

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

I have a better idea for this implementation, block this PR first. Please wait for my PR,

@shibd
Copy link
Member Author

shibd commented Sep 26, 2025

Support on PR: #435

@shibd shibd closed this Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants