Skip to content

[fix][tableview] Fixed ack failure in ReaderImpl due to null messageId - #17828

Merged
Jason918 merged 1 commit into
apache:branch-2.10from
heesung-sohn:branch-2.10-fix-reader-ack
Sep 25, 2022
Merged

[fix][tableview] Fixed ack failure in ReaderImpl due to null messageId#17828
Jason918 merged 1 commit into
apache:branch-2.10from
heesung-sohn:branch-2.10-fix-reader-ack

Conversation

@heesung-sohn

Copy link
Copy Markdown
Contributor

This PR is a cherry-pick from the master requested by #17728 (comment)

@github-actions

Copy link
Copy Markdown

@heesung-sn Please provide a correct documentation label for your PR.
Instructions see Pulsar Documentation Label Guide.

@heesung-sohn
heesung-sohn force-pushed the branch-2.10-fix-reader-ack branch from e7b5fdd to 44aedee Compare September 24, 2022 00:29
@Jason918 Jason918 changed the title [fix][tableview] fixed ack failure in ReaderImpl due to null messageI… [fix][tableview] Fixed ack failure in ReaderImpl due to null messageId Sep 24, 2022
assertEquals(tv.get("key2"), "value2");
}


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.

We don't need the testAck method in #17728 ?

@heesung-sohn heesung-sohn Sep 24, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see an internal data structure change for the reader member in TableView. The testAck in the master branch will fail in 2.10 branch.

2.10
private final ConcurrentMap<String, Reader<T>> readers;

master
private final CompletableFuture<Reader<T>> reader;

We could add the test when merging this update in branch-2.10.

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.

We could add the test when merging this update in branch-2.10.

I see, it's introduced in #15589, which is a refactor and not necessary for patch releases.

@Jason918

Copy link
Copy Markdown
Contributor

/pulsarbot run-failure-checks

@Jason918 Jason918 added release/2.10.2 doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Sep 24, 2022
@github-actions github-actions Bot added doc-label-missing and removed doc-not-needed Your PR changes do not impact docs labels Sep 24, 2022
@Jason918
Jason918 merged commit bbccb62 into apache:branch-2.10 Sep 25, 2022
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Sep 28, 2022
@heesung-sohn
heesung-sohn deleted the branch-2.10-fix-reader-ack branch April 2, 2024 17:44
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.

3 participants