-
Notifications
You must be signed in to change notification settings - Fork 186
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
feat: if enable backup sqle will backup for every sql #2760
base: main
Are you sure you want to change the base?
Conversation
BackupStrategy string `gorm:"type:enum('none','reverse_sql','origin_row','manual');column:backup_strategy;size:20"` // 备份任务支持的备份策略 | ||
BackupStrategyTip string `gorm:"column:backup_strategy_tip;size:255"` // 推荐备份任务的原因 | ||
BackupStatus string `gorm:"type:enum('waiting_for_execution','executing','failed','succeed');column:backup_status;size:20"` // 备份任务的执行状态 | ||
BackupExecInfo string `gorm:"column:backup_exec_info;size:255"` // 备份任务的执行信息,如错误原因 |
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.
info表义太宽泛,建议换个词
sqle/model/task.go
Outdated
@@ -445,7 +446,7 @@ func (s *Storage) GetTasksByIds(taskIds []uint) (tasks []*Task, foundAllIds bool | |||
func (s *Storage) GetTaskDetailById(taskId string) (*Task, bool, error) { | |||
task := &Task{} | |||
err := s.db.Where("id = ?", taskId). | |||
Preload("ExecuteSQLs").Preload("RollbackSQLs").First(task).Error | |||
Preload("ExecuteSQLs").Preload("ExecuteSQLs.BackupTask").First(task).Error |
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.
确认一下preload调整后是否有较大的开销
e_sql.audit_results, e_sql.audit_level, e_sql.audit_status, e_sql.exec_result, e_sql.exec_status | ||
var taskSQLsQueryTpl = `SELECT e_sql.id,e_sql.number, e_sql.description, e_sql.content AS exec_sql, e_sql.source_file AS sql_source_file, e_sql.start_line AS sql_start_line, e_sql.sql_type, r_sql.content AS rollback_sql, | ||
e_sql.audit_results, e_sql.audit_level, e_sql.audit_status, e_sql.exec_result, e_sql.exec_status, | ||
IFNULL(backup_tasks.backup_strategy, '') AS backup_strategy, |
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.
SQL优化:在model上定义not null,不用ifnull,减少性能开销
backupTasks := make([]*model.BackupTask, 0, len(a.task.ExecuteSQLs)) | ||
for _, sql := range a.task.ExecuteSQLs { | ||
affectedRows, err := a.plugin.EstimateSQLAffectRows(context.TODO(), sql.Content) | ||
if err == 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.
err != nil没有处理
if a.task.EnableBackup { | ||
backupTasks := make([]*model.BackupTask, 0, len(a.task.ExecuteSQLs)) | ||
for _, sql := range a.task.ExecuteSQLs { | ||
affectedRows, err := a.plugin.EstimateSQLAffectRows(context.TODO(), sql.Content) |
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.
EstimateSQLAffectRows性能影响极大,需要减少影响范围
assign in @ColdWaterLW
关联的 issue
https://github.com/actiontech/sqle-ee/issues/1956
测试 https://github.com/actiontech/sqle-ee/issues/1956#issuecomment-2477842021
按序提交,需要在以下PR合并后,再复审。有一部分commit在下面的PR中。
#2756
https://github.com/actiontech/sqle-ee/pull/1978
描述你的变更
确认项(pr提交后操作)
Tip
请在指定复审人之前,确认并完成以下事项,完成后✅
not_compatible
need_update_doc