Skip to content

fix: poll publish results from all producers in SolaceIO writer - #39729

Open
waterWang wants to merge 1 commit into
apache:masterfrom
waterWang:fix/solaceio-publish-all-producers
Open

fix: poll publish results from all producers in SolaceIO writer#39729
waterWang wants to merge 1 commit into
apache:masterfrom
waterWang:fix/solaceio-publish-all-producers

Conversation

@waterWang

Copy link
Copy Markdown

Description

The UnboundedSolaceWriter.publishResults() method only polls the publish results queue for the current bundle's producer. When multiple producers are configured (producersMapCardinality > 1), messages published by other producers have their publish results left in their respective queues, never drained or output. This causes downstream transforms attached to the SolaceIO.Write output to receive only a subset of publish results.

Root Cause

In publishResults():

Queue<PublishResult> publishResultsQueue =
    solaceSessionServiceWithProducer().getPublishedResultsQueue();

solaceSessionServiceWithProducer() returns the session for currentBundleProducerIndex (set at @StartBundle time). Other producers' queues are never polled.

Fix

Iterate over all producers (0 to producersMapCardinality - 1) and poll each one's publish results queue. This ensures all publish results are drained and output, regardless of which producer published the message.

Related Issues

Fixes #39588
Also related to #39589 (batch pipeline output loss in SolaceIO writer).

CC @stankiewicz @iht

@github-actions

Copy link
Copy Markdown
Contributor

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

@chamikaramj chamikaramj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

Queue<PublishResult> publishResultsQueue = session.getPublishedResultsQueue();
Solace.PublishResult result = publishResultsQueue.poll();

if (result != null) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm wondering if this can result in data duplication. What if two competing bundles get access to the same queue with data and push them. Does the underlying library guarantee that data only get pushed once ?

if (result != null) {
if (getCurrentBundleTimestamp() == null) {
setCurrentBundleTimestamp(Instant.now());
for (int producerIndex = 0; producerIndex < producersMapCardinality; producerIndex++) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you considered potential perf implications due to all bundles going through all queues ? I would still merge this to fix the correctness issue. But I would at least file a bug to improve perf if this is a concern.

if (result != null) {
if (getCurrentBundleTimestamp() == null) {
setCurrentBundleTimestamp(Instant.now());
for (int producerIndex = 0; producerIndex < producersMapCardinality; producerIndex++) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you please add a unit test ?

@stankiewicz

Copy link
Copy Markdown
Contributor

hi, #39612 is being tested to fix #39589 @waterWang

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: SolaceIO write transform does not output all publish results

3 participants