[fix][client] Complete table view refresh after applying messages - #26566
Merged
Conversation
Publish read positions only after applying or retaining the message value, while preserving refresh calls from update and skipped-message callbacks. Add deterministic coverage for refresh during decoding.
lhotari
requested review from
Technoboy-,
dao-jun,
david-streamlio,
merlimat and
nodece
September 12, 2026 21:22
merlimat
approved these changes
Sep 13, 2026
lhotari
added a commit
that referenced
this pull request
Sep 14, 2026
…ViewImplTest The backport of #26566 (branch-4.0 commit 1450675, cherry-picked from 172b7c3 via branch-4.2 commit 078f7b2) added testRefreshWaitsForMessageToBeApplied, which uses `var` and `List.of(...)`. The pulsar-client module on branch-4.0 is compiled with pulsar.client.compiler.release=8, so the test sources fail to compile ("cannot find symbol: class var", "method of()"). Replace `var` with explicit types and `List.of(...)` with `Collections.emptyList()` / `Collections.singletonList(...)`. The test logic is unchanged. Validation: TableViewImplTest passes (3 tests, both data-provider variants).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
TableView.refreshAsync()can complete while a received message is still being decoded, before its value has been applied to the table. The message position was published before the map update, allowing a concurrent refresh to treat that message as already applied. Callers that immediately read or scan the refreshed table can therefore see stale data.Modifications
Publish the read position after updating the table and before notifying listeners. Skipped and keyless messages still advance the position. This preserves refresh calls from listeners while ensuring that refresh completion reflects the updated table.
Add a deterministic regression test that pauses message decoding, verifies that refresh remains pending, then releases decoding and checks the value and refreshes initiated by normal and skipped-message callbacks.
Verifying this change
Make sure that the change passes the CI checks.
The deterministic regression fails against the original implementation because refresh completes before message application.
Repeated both regression cases with temporary
invocationCount=10: 20/20 passed, plus the existing client unit test.Repeated the existing ownership-cleanup test ten times for each table-view implementation: 20/20 passed without changing its assertions or timeouts.
Final source, with temporary repetitions removed: all 8 selected tests passed, covering
TableViewImplTest,ServiceUnitStateChannelTest.testCleanupDrainsAssignmentsAndRejectsNewOnes, and the existingTableViewTestrefresh and reader-close cases../gradlew spotlessCheck checkstyleMain checkstyleTestpassed across all modules.Tests ran with retries disabled. Local review checked refresh visibility and callback compatibility.
Does this pull request potentially affect one of the following parts: