Skip to content

Conversation

@rleungx
Copy link
Member

@rleungx rleungx commented Jul 22, 2024

The purpose of this PR is that we want to persist the leader information on the PD side so that if the PDs are reloaded, it won't affect the leader distribution in the cluster with a large number of regions. Because the heartbeat may be not processed in time and there will be redundant scheduling.
Besides, the region cache can get the leader ASAP.

Signed-off-by: Ryan Leung <rleungx@gmail.com>
@ti-chi-bot ti-chi-bot bot requested a review from you06 July 22, 2024 03:22
@ti-chi-bot
Copy link

ti-chi-bot bot commented Jul 22, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign overvenus for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot added the size/L label Jul 22, 2024
bool is_in_flashback = 7;
// The start_ts that the current flashback progress is using.
uint64 flashback_start_ts = 8;
Peer leader = 9;
Copy link
Member

Choose a reason for hiding this comment

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

It's better to add a new protobuf message in pdpb.proto that wraps metapb.Region and metapb.Peer for better extensibility.
metapb.Region must only include states that need to be persisted in TiKV side, so "leader" should not be added here.

Copy link
Member Author

Choose a reason for hiding this comment

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

It will cause breaking changes on the PD side. BTW, I think the purpose of metapb.Region are not the same on PD and TiKV.

Copy link
Member

Choose a reason for hiding this comment

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

+1, store leader in region meta is a little weird from the perspective of TiKV. Can PD store a separate key value for leader info?

Copy link
Member Author

Choose a reason for hiding this comment

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

leader won't assign any value on the TiKV side.

Copy link
Member

Choose a reason for hiding this comment

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

I know, if there is no other better solution. I can accept reluctantly.

@rleungx rleungx removed the request for review from you06 July 22, 2024 06:00
@rleungx rleungx changed the title metapb: persist region leader persist region leader Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants