Skip to content

[feat][client] Introduce Refresh API in the TableView#21417

Merged
liangyepianzhou merged 11 commits into
apache:masterfrom
liangyepianzhou:refresh
Mar 15, 2024
Merged

[feat][client] Introduce Refresh API in the TableView#21417
liangyepianzhou merged 11 commits into
apache:masterfrom
liangyepianzhou:refresh

Conversation

@liangyepianzhou

Copy link
Copy Markdown
Contributor

Master #21271

Motivation

The proposal will introduce a new API to refresh the table view with the latest written data on the topic, ensuring that all subsequent reads are based on the refreshed data.

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Oct 23, 2023
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated

@BewareMyPower BewareMyPower 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.

Remove meaningless lines

Comment thread pulsar-broker/src/test/java/org/apache/pulsar/client/impl/TableViewTest.java Outdated
Comment thread pulsar-broker/src/test/java/org/apache/pulsar/client/impl/TableViewTest.java Outdated
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
Comment thread pulsar-client-api/src/main/java/org/apache/pulsar/client/api/TableView.java Outdated
Comment thread pulsar-client-api/src/main/java/org/apache/pulsar/client/api/TableView.java Outdated
Comment thread pulsar-broker/src/test/java/org/apache/pulsar/client/impl/TableViewTest.java Outdated
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
1. Add refresh API.
2. Add a linkedHashMap storing the refresh request.
2.1 Add request when refresh
2.2 Complete request when read completely.
2.3 Complete request exceptionally when reader close.
3. Add a map storing the current read position.
4. Check the current read position when building new refresh request.
Comment thread pulsar-broker/src/test/java/org/apache/pulsar/client/impl/TableViewTest.java Outdated
Comment thread pulsar-broker/src/test/java/org/apache/pulsar/client/impl/TableViewTest.java Outdated
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
Comment thread pulsar-broker/src/test/java/org/apache/pulsar/client/impl/TableViewTest.java Outdated
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
Comment thread pulsar-broker/src/test/java/org/apache/pulsar/client/impl/TableViewTest.java Outdated
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
Comment thread pulsar-broker/src/test/java/org/apache/pulsar/client/impl/TableViewTest.java Outdated
@BewareMyPower

Copy link
Copy Markdown
Contributor

@codelipenghui Could you take a second review again?

Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java Outdated
@codecov-commenter

codecov-commenter commented Mar 15, 2024

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.36066% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.64%. Comparing base (bbc6224) to head (7fc7a34).
⚠️ Report is 1260 commits behind head on master.

Files with missing lines Patch % Lines
...a/org/apache/pulsar/client/impl/TableViewImpl.java 98.36% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #21417      +/-   ##
============================================
+ Coverage     73.57%   73.64%   +0.06%     
+ Complexity    32624    32258     -366     
============================================
  Files          1877     1887      +10     
  Lines        139502   139436      -66     
  Branches      15299    15292       -7     
============================================
+ Hits         102638   102681      +43     
+ Misses        28908    28820      -88     
+ Partials       7956     7935      -21     
Flag Coverage Δ
inttests 26.88% <29.50%> (+2.30%) ⬆️
systests 24.37% <0.00%> (+0.04%) ⬆️
unittests 72.91% <98.36%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...a/org/apache/pulsar/client/impl/TableViewImpl.java 88.82% <98.36%> (+3.51%) ⬆️

... and 161 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@liangyepianzhou liangyepianzhou merged commit 95d24ac into apache:master Mar 15, 2024
@liangyepianzhou liangyepianzhou deleted the refresh branch March 15, 2024 08:57
hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
Master apache#21271
### Motivation
The proposal will introduce a new API to refresh the table view with the latest written data on the topic, ensuring that all subsequent reads are based on the refreshed data.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants