append log for upgrade and sqlExecutoer

[qingxinhome]

User description

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue https://github.com/matrixorigin/MO-Cloud/issues/3404

What this PR does / why we need it:

append log for upgrader and sqlExecutor

---

PR Type

Bug fix, Enhancement

---

Description

• Added error and info logging for pre-sql, upgrade sql, and post-sql execution in upgrade_strategy.go.
• Added timing and info logging for tenant and cluster upgrade entries in v1_2_1/upgrade.go.
• Added logging for received SQL execute requests in sql_executor.go.

---

Changes walkthrough 📝

Relevant files

Enhancement

upgrade_strategy.go Add logging for SQL execution in upgrade strategy                pkg/bootstrap/versions/upgrade_strategy.go Added error logging for pre-sql, upgrade sql, and post-sql execution. Added info logging for successful execution of pre-sql, upgrade sql, and post-sql. +5/-0      upgrade.go Add timing and logging for tenant and cluster upgrades      pkg/bootstrap/versions/v1_2_1/upgrade.go Added timing for tenant and cluster upgrade entries. Added info logging for completion of tenant and cluster upgrade entries. +18/-0    sql_executor.go Add logging for received SQL execute requests                        pkg/sql/compile/sql_executor.go Added logging for received SQL execute requests. +4/-0

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Review 🔍

⏱️ Estimated effort to review [1-5]	2, because the changes are straightforward and mainly involve adding logging statements to existing functions. The logic of the code remains largely unchanged, making it easier to review.
🧪 Relevant tests	No
⚡ Possible issues	Possible Performance Issue: The addition of logging, especially with detailed information like SQL statements and transaction IDs, could potentially impact performance, especially if these logs are generated at a high frequency in a production environment.
🔒 Security concerns	No

[codiumai-pr-agent-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Use defer to ensure that res.Close() is always called To ensure that the res.Close() method is always called, even if an error occurs, it would be better to use a defer statement right after the txn.Exec call. pkg/bootstrap/versions/upgrade_strategy.go [144-149] res, err := txn.Exec(u.PreSql, executor.StatementOption{}.WithAccountID(accountId)) if err != nil { getLogger().Error("execute upgrade entry pre-sql error", zap.Error(err), zap.String("upgrade entry", u.String())) return err } -res.Close() +defer res.Close() Suggestion importance[1-10]: 8 Why: Using defer for res.Close() right after the txn.Exec call ensures the resource is always properly closed, even if an error occurs, which is a significant improvement in resource management.	8
Possible bug	Add nil checks before accessing nested fields to prevent potential nil pointer dereference To avoid potential nil pointer dereference, check if exec.opts.Txn() and exec.opts.Txn().Txn() are not nil before calling hex.EncodeToString(exec.opts.Txn().Txn().ID). pkg/sql/compile/sql_executor.go [259-261] +txnID := "" +if exec.opts.Txn() != nil && exec.opts.Txn().Txn() != nil { + txnID = hex.EncodeToString(exec.opts.Txn().Txn().ID) +} logutil.Info("Received SQL execute request", zap.String("sql", sql), - zap.String("txn-id", hex.EncodeToString(exec.opts.Txn().Txn().ID))) + zap.String("txn-id", txnID)) Suggestion importance[1-10]: 7 Why: Adding nil checks before dereferencing pointers is crucial to prevent runtime errors in Go. This suggestion correctly identifies a potential bug and offers a solution to improve code robustness.	7
Maintainability	Extract logging logic into a reusable function to avoid code duplication To avoid code duplication and improve maintainability, extract the logging logic into a separate function that can be reused. pkg/bootstrap/versions/upgrade_strategy.go [146-150] -getLogger().Error("execute upgrade entry pre-sql error", zap.Error(err), zap.String("upgrade entry", u.String())) -getLogger().Info("execute upgrade pre-sql completed", zap.String("upgrade entry", u.String())) +logUpgradeResult := func(err error, stage string) { + if err != nil { + getLogger().Error("execute upgrade entry " + stage + " error", zap.Error(err), zap.String("upgrade entry", u.String())) + } else { + getLogger().Info("execute upgrade " + stage + " completed", zap.String("upgrade entry", u.String())) + } +} +logUpgradeResult(err, "pre-sql") Suggestion importance[1-10]: 6 Why: Extracting repeated logging logic into a separate function enhances maintainability and reduces code duplication. This is a good practice, especially in larger codebases where similar logging patterns are used frequently.	6
Enhancement	Rename the variable duration to elapsedTime for better readability To improve readability and avoid potential confusion, rename the variable duration to elapsedTime to better reflect its purpose. pkg/bootstrap/versions/v1_2_1/upgrade.go [71-76] -duration := time.Since(start) +elapsedTime := time.Since(start) getLogger().Info("tenant upgrade entry complete", zap.String("upgrade entry", upgEntry.String()), - zap.Int64("time cost", duration.Milliseconds()), + zap.Int64("time cost", elapsedTime.Milliseconds()), zap.Int32("tenantId", tenantID), zap.String("toVersion", v.Metadata().Version)) Suggestion importance[1-10]: 4 Why: While renaming duration to elapsedTime improves readability slightly, it's a minor enhancement and doesn't impact functionality or performance.	4