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 adding message to list potential issue #14377

Merged
merged 1 commit into from
Feb 18, 2022

Conversation

Technoboy-
Copy link
Contributor

@Technoboy- Technoboy- commented Feb 18, 2022

Motivation

There may exist potential issue when adding MessageIdData into data, because when the future is triggered, the data object was not guarantee to be already filled in.

Documentation

  • no-need-doc

@Technoboy- Technoboy- self-assigned this Feb 18, 2022
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Feb 18, 2022
@BewareMyPower BewareMyPower added release/2.8.4 release/2.9.3 type/bug The PR fixed a bug or issue reported a bug release/2.10.1 labels Feb 18, 2022
@BewareMyPower BewareMyPower added this to the 2.11.0 milestone Feb 18, 2022
@merlimat merlimat merged commit b22445f into apache:master Feb 18, 2022
michaeljmarshall pushed a commit that referenced this pull request Feb 24, 2022
@michaeljmarshall michaeljmarshall added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Feb 24, 2022
@codelipenghui codelipenghui modified the milestones: 2.11.0, 2.10.0 Feb 25, 2022
codelipenghui pushed a commit that referenced this pull request Feb 25, 2022
nicoloboschi pushed a commit to nicoloboschi/pulsar that referenced this pull request Mar 1, 2022
gaoran10 pushed a commit that referenced this pull request Mar 1, 2022
@gaoran10 gaoran10 added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Mar 2, 2022
@@ -1907,18 +1907,17 @@ protected void completeOpBatchReceive(OpBatchReceive<T> op) {
return CompletableFuture.completedFuture(Collections.emptyList());
}
List<MessageIdData> data = new ArrayList<>(messageIds.size());
Copy link
Contributor

Choose a reason for hiding this comment

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

There are seem another potential race condition, use ArrayList in possible multithread scenario is unsafe.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've create #14687 to continue this discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yes.

Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022
@Technoboy- Technoboy- deleted the fix-potential-dlq-issu branch August 10, 2022 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/client cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.3 release/2.9.2 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants