-
-
Notifications
You must be signed in to change notification settings - Fork 532
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
[no-release-notes] go: sqle: dprocedures: dolt_gc: Implement a session-aware GC safepoint controller. #8798
Changes from 7 commits
366e466
915e392
ef954d0
bdc8ff1
fc3217e
13c9ddf
819136d
153d46b
5826451
7c1cff7
f79db4a
b9f6a56
52b625f
b44469d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,6 @@ | |
package binlogreplication | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"strings" | ||
"sync" | ||
|
@@ -158,8 +157,6 @@ func (d *doltBinlogReplicaController) StartReplica(ctx *sql.Context) error { | |
// created and locked to disable log ins, and if it does exist, but is missing super privs or is not | ||
// locked, it will be given superuser privs and locked. | ||
func (d *doltBinlogReplicaController) configureReplicationUser(ctx *sql.Context) { | ||
sql.SessionCommandBegin(ctx.Session) | ||
defer sql.SessionCommandEnd(ctx.Session) | ||
mySQLDb := d.engine.Analyzer.Catalog.MySQLDb | ||
ed := mySQLDb.Editor() | ||
defer ed.Close() | ||
|
@@ -417,8 +414,10 @@ func (d *doltBinlogReplicaController) setSqlError(errno uint, message string) { | |
// replication is not configured, hasn't been started, or has been stopped before the server was | ||
// shutdown, then this method will not start replication. This method should only be called during | ||
// the server startup process and should not be invoked after that. | ||
func (d *doltBinlogReplicaController) AutoStart(_ context.Context) error { | ||
runningState, err := loadReplicationRunningState(d.ctx) | ||
func (d *doltBinlogReplicaController) AutoStart(ctx *sql.Context) error { | ||
sql.SessionCommandBegin(ctx.Session) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noted this on the other PR, I don't think we are generally very disciplined about sql session lifecycle management. Maybe it only matters in a few key places for GC There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, but we will have to get better where it matters... |
||
defer sql.SessionCommandEnd(ctx.Session) | ||
runningState, err := loadReplicationRunningState(ctx) | ||
if err != nil { | ||
logrus.Errorf("Unable to load replication running state: %s", err.Error()) | ||
return err | ||
|
@@ -430,7 +429,7 @@ func (d *doltBinlogReplicaController) AutoStart(_ context.Context) error { | |
} | ||
|
||
logrus.Info("auto-starting binlog replication from source...") | ||
return d.StartReplica(d.ctx) | ||
return d.StartReplica(ctx) | ||
} | ||
|
||
// Release all resources, such as replication threads, associated with the replication. | ||
|
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.
The logic in this method seems like it would be more naturally contained by the WorkingSet type