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][client] Avert extensive time consumption during table view construction #21270

Merged
merged 10 commits into from
Nov 6, 2023

Conversation

liangyepianzhou
Copy link
Contributor

Reopen #21170

Motivation

If a topic persistently experiences a substantial quantity of data inputs, the act of reading all the messages present in this topic to build a TableView can take an excessive amount of time.

Modification

In the process of constructing the TableView, initially, the last message ID of the current topic is procured. Consequently, once this last message ID has been reached, the creation ensues to its completion.

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:

…truction

### Motivation
If a topic persistently experiences a substantial quantity of data inputs,  the act of reading all the messages present in this topic to build a TableView can take an excessive amount of time.
### Modification
In the process of constructing the TableView, initially, the last message ID of the current topic is procured. Consequently, once this last message ID has been reached, the creation ensues to its completion.
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 28, 2023
@liangyepianzhou liangyepianzhou self-assigned this Sep 28, 2023
@codelipenghui codelipenghui added this to the 3.2.0 milestone Nov 3, 2023
@codelipenghui codelipenghui added area/client category/reliability The function does not work properly in certain specific environments or failures. e.g. data lost labels Nov 3, 2023
@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

readAllExistingMessages(reader, future, startTime, messagesRead);
if (maxMessageIds.isEmpty()) {
future.complete(reader);
readTailMessages(reader);
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of adding readTailMessages here, it would be more clear to add a thenAccept to the future in readAllExistingMessages(Reader<T> reader).

future.thenAccept(__ -> readTailMessages(reader));

Otherwise, if future.complete could be called somewhere else in future, the author might forget to call readTailMessages.

@liangyepianzhou liangyepianzhou merged commit 957337b into apache:master Nov 6, 2023
@liangyepianzhou liangyepianzhou deleted the tableview_build branch November 6, 2023 07:55
Technoboy- pushed a commit that referenced this pull request Nov 10, 2023
…truction (#21270)

Reopen #21170
### Motivation
If a topic persistently experiences a substantial quantity of data inputs,  the act of reading all the messages present in this topic to build a TableView can take an excessive amount of time.
### Modification
In the process of constructing the TableView, initially, the last message ID of the current topic is procured. Consequently, once this last message ID has been reached, the creation ensues to its completion.
Technoboy- pushed a commit that referenced this pull request Nov 10, 2023
…truction (#21270)

Reopen #21170
### Motivation
If a topic persistently experiences a substantial quantity of data inputs,  the act of reading all the messages present in this topic to build a TableView can take an excessive amount of time.
### Modification
In the process of constructing the TableView, initially, the last message ID of the current topic is procured. Consequently, once this last message ID has been reached, the creation ensues to its completion.
liangyepianzhou added a commit that referenced this pull request Nov 13, 2023
liangyepianzhou added a commit that referenced this pull request Nov 13, 2023
nborisov pushed a commit to nborisov/pulsar that referenced this pull request Nov 13, 2023
…truction (apache#21270)

Reopen apache#21170
### Motivation
If a topic persistently experiences a substantial quantity of data inputs,  the act of reading all the messages present in this topic to build a TableView can take an excessive amount of time.
### Modification
In the process of constructing the TableView, initially, the last message ID of the current topic is procured. Consequently, once this last message ID has been reached, the creation ensues to its completion.
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 20, 2023
…truction (apache#21270)

Reopen apache#21170
### Motivation
If a topic persistently experiences a substantial quantity of data inputs,  the act of reading all the messages present in this topic to build a TableView can take an excessive amount of time.
### Modification
In the process of constructing the TableView, initially, the last message ID of the current topic is procured. Consequently, once this last message ID has been reached, the creation ensues to its completion.
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 20, 2023
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 20, 2023
…truction (apache#21270)

Reopen apache#21170
### Motivation
If a topic persistently experiences a substantial quantity of data inputs,  the act of reading all the messages present in this topic to build a TableView can take an excessive amount of time.
### Modification
In the process of constructing the TableView, initially, the last message ID of the current topic is procured. Consequently, once this last message ID has been reached, the creation ensues to its completion.
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 20, 2023
mukesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Mar 1, 2024
…iew construction (apache#21270)"

This reverts commit c46ed2d.

(cherry picked from commit ceafb3b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/client category/reliability The function does not work properly in certain specific environments or failures. e.g. data lost cherry-picked/branch-3.1 doc-not-needed Your PR changes do not impact docs ready-to-test release/3.1.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants