-
-
Notifications
You must be signed in to change notification settings - Fork 481
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
Perf improvements for remote read replicas #5005
Conversation
…chmu/replica-perf
…ged since the last pull
…chmu/replica-perf
srcDBCommit, err = actions.FetchRemoteBranch(fetchCtx, rrd.tmpDir, rrd.remote, rrd.srcDB, rrd.ddb, branch, actions.NoopRunProgFuncs, actions.NoopStopProgFuncs) | ||
if err != nil { | ||
return nil, err | ||
err = rrd.ddb.PullChunks(ctx, rrd.tmpDir, rrd.srcDB, srcRoot, nil, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: It seems more correct, clearer and very marginally more performance to make an interface that will pull more than one addr at once and then to pass all the addrs in remoteRefs
to it.
…e a slice of hashes to pull, not just one. This allows client code to be more efficient when it needs to pull multiple hashes.
…eckout in the same SQL session as the commit)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for making that change.
This significantly speeds up the process of syncing a remote-based replica by 1) pulling chunks once, rather than once per branch in the case of replicas that want everything, and 2) by eliminating a bunch of duplicated and wasted work. Latency for up-to-date case is now <100 ms.