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

Fix: SendAsync callback was not invoked when producer is in reconnecting #1345

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nodece
Copy link
Member

@nodece nodece commented Mar 12, 2025

Fixes #1332

Motivation

When the producer reconnects, the batching/non-batching messages can not be added to the pending queue, so the timeout error can not be triggered. This leads to situations where SendAsync callbacks are never invoked when the producer is in a reconnecting state, potentially causing resource leaks and unresponsive applications.

Modifications

  1. Refactored batch processing:
    • Moved batch processing logic to a dedicated goroutine to eliminate race conditions.
    • Proper synchronization was added for batch operations when calling the flush method.
  2. Separated data writing from message processing, means the message should be added to the queue, and then write the message to the broker by the event loop.
  3. Fixed payload size handling in TestProducerSendWithContext test:
    time="2025-03-16T09:05:13Z" level=error msg="Single message serialize failed %!s(<nil>)" error="encryptedPayload 
    exceeds MaxMessageSize, size: 1048607, MaxMessageSize: 1048576" producerID=1 producer_name=standalone-0- 
    358 topic="persistent://public/default/my-topic-857833034"
    
  4. Added a comprehensive test case TestProducerKeepReconnectingAndThenCallSendAsync that verifies:
    • SendAsync callbacks are properly invoked with appropriate errors during reconnection.
    • The fix works correctly with both batching enabled and disabled configurations.

@nodece nodece marked this pull request as draft March 12, 2025 17:36
@nodece nodece force-pushed the fix-SendAsync-callback branch 3 times, most recently from d207f52 to f38a7ad Compare March 16, 2025 07:47
@nodece nodece force-pushed the fix-SendAsync-callback branch from f38a7ad to 61794b2 Compare March 16, 2025 07:53
@nodece nodece marked this pull request as ready for review March 16, 2025 15:04
@nodece nodece self-assigned this Mar 16, 2025
@nodece
Copy link
Member Author

nodece commented Mar 16, 2025

@gunli Could you have a chance to review?

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.

[Bug][Producer] The callback was not invoked during reconnecting.
1 participant