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

Modify error message strings. #2164

Merged
merged 13 commits into from
Jan 9, 2025
Merged

Conversation

makalaaneesh
Copy link
Collaborator

Describe the changes in this pull request

Modify error strings to include user-specific details only after a ":". This is because as part of sending error messages to callhome, we sanitize them by picking the segment of the error message before the first ":"

Describe if there are any user-facing changes

How was this pull request tested?

Does your PR have changes that can cause upgrade issues?

Component Breaking changes?
MetaDB Yes/No
Name registry json Yes/No
Data File Descriptor Json Yes/No
Export Snapshot Status Json Yes/No
Import Data State Yes/No
Export Status Json Yes/No
Data .sql files of tables Yes/No
Export and import data queue Yes/No
Schema Dump Yes/No
AssessmentDB Yes/No
Sizing DB Yes/No
Migration Assessment Report Json Yes/No
Callhome Json Yes/No
YugabyteD Tables Yes/No
TargetDB Metadata Tables Yes/No

@@ -869,7 +869,7 @@ func exportDataOffline(ctx context.Context, cancel context.CancelFunc, finalTabl
cancel() //will cancel/stop both dump tool and progress bar
time.Sleep(time.Second * 5) //give sometime for the cancel to complete before this function returns
utils.ErrExit("yb-voyager encountered internal error. "+
"Check %s/logs/yb-voyager-export-data.log for more details.", exportDir)
"Check: %s/logs/yb-voyager-export-data.log for more details.", exportDir)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: utils.ErrExit("yb-voyager encountered internal error: "+
"Check %s/logs/yb-voyager-export-data.log for more details.", exportDir)

@@ -419,7 +419,7 @@ func applyTableListFilter(importFileTasks []*ImportFileTask) []*ImportFileTask {
}
}
if len(unqualifiedTables) > 0 {
utils.ErrExit("Qualify following table names %v in the %s list with schema-name.", unqualifiedTables, listName)
utils.ErrExit("Qualify following table names: %v in the %s list with schema-name.", unqualifiedTables, listName)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utils.ErrExit("Qualify following table names in the %s list with schema-name: %v", listName, unqualifiedTables)

listName is string include / exclude.

@@ -1104,14 +1104,14 @@ func splitFilesForTable(state *ImportDataState, filePath string, t sqlname.NameT
if tconf.TargetDBType == YUGABYTEDB {
ybSpecificMsg = ", but should be strictly lower than the the rpc_max_message_size on YugabyteDB (default 267386880 bytes)"
}
utils.ErrExit("record num=%d for table %q in file %s is larger than the max batch size %d bytes Max Batch size can be changed using env var MAX_BATCH_SIZE_BYTES%s", numLinesTaken, t.ForOutput(), filePath, tdb.MaxBatchSizeInBytes(), ybSpecificMsg)
utils.ErrExit("record larger than max batch size: record num=%d for table %q in file %s is larger than the max batch size %d bytes Max Batch size can be changed using env var MAX_BATCH_SIZE_BYTES%s", numLinesTaken, t.ForOutput(), filePath, tdb.MaxBatchSizeInBytes(), ybSpecificMsg)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we also add currentBytesRead in error msg -

utils.ErrExit("record of size %d larger than max batch size: record num=%d for table %q in file %s is larger than the max batch size %d bytes Max Batch size can be changed using env var MAX_BATCH_SIZE_BYTES%s", currentBytesRead, numLinesTaken, t.ForOutput(), filePath, tdb.MaxBatchSizeInBytes(), ybSpecificMsg)

@@ -94,7 +94,7 @@ func (ms *MySQL) GetTableApproxRowCount(tableName sqlname.NameTuple) int64 {
log.Infof("Querying '%s' approx row count of table %q", query, tableName.String())
err := ms.db.QueryRow(query).Scan(&approxRowCount)
if err != nil {
utils.ErrExit("Failed to query %q for approx row count of %q: %s", query, tableName.String(), err)
utils.ErrExit("Failed to query for approx row count of %q: %q %s", tableName.String(), query, err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utils.ErrExit("Failed to query for approx row count: %q: %q %s", tableName.String(), query, err)

@@ -160,7 +160,7 @@ func (pg *PostgreSQL) GetTableApproxRowCount(tableName sqlname.NameTuple) int64
log.Infof("Querying '%s' approx row count of table %q", query, tableName.String())
err := pg.db.QueryRow(query).Scan(&approxRowCount)
if err != nil {
utils.ErrExit("Failed to query %q for approx row count of %q: %s", query, tableName.String(), err)
utils.ErrExit("Failed to query for approx row count of %q: %q: %s", tableName.String(), query, err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utils.ErrExit("Failed to query for approx row count: %q: %q: %s", tableName.String(), query, err)

Copy link
Contributor

@priyanshi-yb priyanshi-yb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Base automatically changed from aneesh/callhome-error to main January 9, 2025 09:10
@makalaaneesh makalaaneesh marked this pull request as ready for review January 9, 2025 09:14
@makalaaneesh makalaaneesh merged commit 073fdac into main Jan 9, 2025
67 checks passed
@makalaaneesh makalaaneesh deleted the aneesh/callhome-error-modify branch January 9, 2025 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants