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

keyspace: keyspace meta accounts for 65.28% of in-use memory of PD/API leader in dev env #6733

Closed
binshi-bing opened this issue Jul 1, 2023 · 2 comments · Fixed by #6793
Closed
Assignees
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@binshi-bing
Copy link
Contributor

Enhancement Task

There are more >20K keyspaces in dev. Is this much memory usage by keyspace meta expected? Do we need to keep all keyspace meta in memory?

image
@binshi-bing binshi-bing added the type/enhancement The issue or PR belongs to an enhancement. label Jul 1, 2023
@binshi-bing binshi-bing assigned binshi-bing and nolouch and unassigned binshi-bing Jul 1, 2023
@lhy1024
Copy link
Contributor

lhy1024 commented Jul 4, 2023

every client call WatchKeyspaces to maintain a watch-loop, but tidb only need to get all keyspace.

func (s *KeyspaceServer) WatchKeyspaces(request *keyspacepb.WatchKeyspacesRequest, stream keyspacepb.Keyspace_WatchKeyspacesServer) error {
    .....
    go watcher.StartWatchLoop()
    if err := watcher.WaitLoad(); err != nil {
        cancel() // cancel context to stop watcher
        return err
    }
    <-ctx.Done() // wait for context done
    return nil
}
func (w *GCWorker) getAllKeyspace(ctx context.Context) []*keyspacepb.KeyspaceMeta {
    ctx, cancel := context.WithCancel(context.Background())
    defer cancel()
    watchChan, err := w.pdClient.WatchKeyspaces(ctx)
    if err != nil {
        logutil.Logger(ctx).Error("WatchKeyspaces error")
    }
    initialLoaded := <-watchChan
    return initialLoaded
}

I think we only provide GetAllKeyspace in client is enough and maintain only one watch-loop in server
And we also can LoadKeyspace by this server watch-loop and avoid to unnecessary load by etcd.

@lhy1024 lhy1024 assigned lhy1024 and unassigned nolouch Jul 4, 2023
@lhy1024
Copy link
Contributor

lhy1024 commented Jul 12, 2023

After discussion, @ystaticy will fix it

ti-chi-bot bot added a commit that referenced this issue Jul 13, 2023
close #6733

Signed-off-by: ystaticy <y_static_y@sina.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants