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

[WIP]broadcast region when we use etcdBaseKV to saveRegion #1466

Closed
bradyjoestar opened this issue Mar 19, 2019 · 5 comments
Closed

[WIP]broadcast region when we use etcdBaseKV to saveRegion #1466

bradyjoestar opened this issue Mar 19, 2019 · 5 comments
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. type/enhancement The issue or PR belongs to an enhancement.

Comments

@bradyjoestar
Copy link
Contributor

I am reading the core/kv.go and found the following code

// SaveRegion saves one region to KV.
func (kv *KV) SaveRegion(region *metapb.Region) error {
	if atomic.LoadInt32(&kv.useRegionKV) > 0 {
		return kv.regionKV.SaveRegion(region)
	}
	return saveProto(kv.KVBase, regionPath(region.GetId()), region)
}

If useRegionKV=0, we will use kv.KVBase to saveRegion, but at the same time region_syncer is still runing in the
https://github.com/pingcap/pd/blob/59968e186cf23bdc172cd7be3c3c7e22aeb600f0/server/cluster.go#L128

And the leader will broadcast the region to member due to
https://github.com/pingcap/pd/blob/59968e186cf23bdc172cd7be3c3c7e22aeb600f0/server/cluster_info.go#L563

Maybe it could be skipped when we dont use leveldb as kvstore to decrease the network flow?

@nolouch
Copy link
Contributor

nolouch commented Mar 19, 2019

@bradyjoestar It's not always broadcast the changed, only if the client has established the stream with the leader. and if not use leveldb, it will not establish stream with the leader.

@nolouch
Copy link
Contributor

nolouch commented Mar 19, 2019

But there is a problem that the config change only know if PD restart or leader changed, maybe you can help us to improve it.

@nolouch nolouch added type/question The issue belongs to a question. type/enhancement The issue or PR belongs to an enhancement. labels Mar 19, 2019
@bradyjoestar
Copy link
Contributor Author

@nolouch my pleasure if it is not hurry . 😁

@nolouch
Copy link
Contributor

nolouch commented Mar 19, 2019

@bradyjoestar Thanks, you can have a try. Now, the config of the follower only reload when the lead changed, what we want is when the config of the leader changed, we can also apply the change to the follower server. and now the related code is here:
https://github.com/pingcap/pd/blob/59968e186cf23bdc172cd7be3c3c7e22aeb600f0/server/leader.go#L298-L307

@nolouch nolouch added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. and removed type/question The issue belongs to a question. labels Mar 19, 2019
@bradyjoestar
Copy link
Contributor Author

@nolunch I will try to handle it. 👌

@bradyjoestar bradyjoestar changed the title broadcast region when we use etcdBaseKV to saveRegion [WIP]broadcast region when we use etcdBaseKV to saveRegion Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

No branches or pull requests

3 participants