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

implement committed entries pagination #440

Merged
merged 9 commits into from
Jun 4, 2021
Merged

Conversation

hicqu
Copy link
Contributor

@hicqu hicqu commented May 31, 2021

Signed-off-by: qupeng qupeng@pingcap.com

This is a re-implement for #356, except such things:

  • the new configuration max_committed_size_per_ready isn't pushed into RaftLog
  • new test cases are simplified into one, and it covers Ready and LightReady

hicqu added 3 commits May 31, 2021 14:08
Signed-off-by: qupeng <qupeng@pingcap.com>
Signed-off-by: qupeng <qupeng@pingcap.com>
Signed-off-by: qupeng <qupeng@pingcap.com>
@hicqu hicqu requested review from gengliqi and BusyJay May 31, 2021 09:48
@BusyJay
Copy link
Member

BusyJay commented May 31, 2021

Why not set it by Config?

@hicqu
Copy link
Contributor Author

hicqu commented May 31, 2021

Why not set it by Config?

Using an Options makes it configurable. For example, we can set it to 8M generally, however if there is no available memory, we can set it to 0 to fetch nothing temporarely.

@BusyJay
Copy link
Member

BusyJay commented May 31, 2021

How about adding an API to mutate the configuration instead of adding a new parameter?

Signed-off-by: qupeng <qupeng@pingcap.com>
@BusyJay
Copy link
Member

BusyJay commented May 31, 2021

I recall there is already a PR implementing the feature #356, can you update the PR and pick to master?

Signed-off-by: qupeng <qupeng@pingcap.com>
@hicqu hicqu changed the title Add ReadyOptions for generating Ready add max_committed_size_per_ready into config Jun 1, 2021
@hicqu hicqu changed the title add max_committed_size_per_ready into config implement committed entries pagination Jun 1, 2021
Copy link
Member

@BusyJay BusyJay left a comment

Choose a reason for hiding this comment

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

test_commit_pagination_after_restart should also be ported.

src/raft.rs Outdated Show resolved Hide resolved
hicqu added 4 commits June 1, 2021 16:00
Signed-off-by: qupeng <qupeng@pingcap.com>
Signed-off-by: qupeng <qupeng@pingcap.com>
Signed-off-by: qupeng <qupeng@pingcap.com>
Signed-off-by: qupeng <qupeng@pingcap.com>
@hicqu hicqu merged commit b84afe9 into tikv:master Jun 4, 2021
@hicqu hicqu deleted the ready-with-options branch June 4, 2021 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants