shibd opened a new pull request, #433: URL: https://github.com/apache/pulsar-client-node/pull/433
### 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](https://github.com/shibd/pulsar-client-node/pull/44) 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) - [x] `doc-not-needed` (Please explain why) - [ ] `doc` (Your PR contains doc changes) - [ ] `doc-complete` (Docs have been already added) -- 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]
