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

Refactor migration assessment codepath: flatten reported issues #2153

Merged
merged 16 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .github/workflows/misc-migtests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,6 @@ jobs:
echo "127.0.0.1 yb-master-n1" | sudo tee -a /etc/hosts
psql "postgresql://yugabyte@yb-tserver-n1:5433/yugabyte" -c "SELECT version();"

echo "TEST: PG sample schemas (omnibus)"
migtests/scripts/run-schema-migration.sh pg/omnibus

echo "Setup the gcp credentials"
migtests/scripts/gcs/create_gcs_credentials_file

Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/pg-17-migtests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ jobs:
if: ${{ !cancelled() && matrix.test_group == 'offline' }}
run: migtests/scripts/run-schema-migration.sh pg/osm

- name: "TEST: PG sample schemas (omnibus)"
sanyamsinghal marked this conversation as resolved.
Show resolved Hide resolved
if: ${{ !cancelled() && matrix.test_group == 'offline' }}
run: migtests/scripts/run-schema-migration.sh pg/omnibus

- name: "TEST: PG sample schemas (adventureworks)"
if: ${{ !cancelled() && matrix.test_group == 'offline' }}
run: migtests/scripts/run-schema-migration.sh pg/adventureworks
Expand Down
21 changes: 17 additions & 4 deletions migtests/scripts/functions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -877,18 +877,31 @@ normalize_json() {
# Normalize JSON with jq; use --sort-keys to avoid the need to keep the same sequence of keys in expected vs actual json
jq --sort-keys 'walk(
if type == "object" then
.ObjectNames? |= (if type == "string" then split(", ") | sort | join(", ") else . end) |
.ObjectNames? |= (
if type == "string" then
split(", ") | sort | join(", ")
else
.
end
) |
.VoyagerVersion? = "IGNORED" |
.TargetDBVersion? = "IGNORED" |
.DbVersion? = "IGNORED" |
.FilePath? = "IGNORED" |
.OptimalSelectConnectionsPerNode? = "IGNORED" |
.OptimalInsertConnectionsPerNode? = "IGNORED" |
.RowCount? = "IGNORED" |
.SqlStatement? |= (if type == "string" then gsub("\\n"; " ") else . end)
# Replace newline characters in SqlStatement with spaces
.SqlStatement? |= (
if type == "string" then
gsub("\\n"; " ")
else
.
end
)
elif type == "array" then
sort_by(tostring)
else
sort_by(tostring)
else
.
end
)' "$input_file" > "$temp_file"
Expand Down
121 changes: 62 additions & 59 deletions yb-voyager/cmd/analyzeSchema.go
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In this file, changes are mostly due to update in the reportCase function signature to have impact also as an argument.

Large diffs are not rendered by default.

310 changes: 220 additions & 90 deletions yb-voyager/cmd/assessMigrationCommand.go

Large diffs are not rendered by default.

16 changes: 11 additions & 5 deletions yb-voyager/cmd/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -1195,13 +1195,15 @@ type AssessmentReport struct {
MigrationCaveats []UnsupportedFeature `json:"MigrationCaveats"`
}

// Fields apart from Category, CategoryDescription, TypeName and Impact will be populated only if/when available
type AssessmentIssue struct {
Category string
Category string // expected values: feature, query_constrcuts, migration_caveats, plpgsql_objects, datatytpe
CategoryDescription string
TypeName string
TypeDescription string
Impact string
ObjectType string
Type string // Ex: GIN_INDEXES, SECURITY_INVOKER_VIEWS, STORED_GENERATED_COLUMNS
Name string // Ex: "Stored generated columns are not supported."
Description string
Impact string // Level-1, Level-2, Level-3 (default: Level-1 ??)
ObjectType string // For datatype category, ObjectType will be datatype (for eg "geometry")
ObjectName string
SqlStatement string
DocsLink string
Expand Down Expand Up @@ -1313,6 +1315,10 @@ func ParseJSONToAssessmentReport(reportPath string) (*AssessmentReport, error) {
return &report, nil
}

func (ar *AssessmentReport) AppendIssues(issues ...AssessmentIssue) {
ar.Issues = append(ar.Issues, issues...)
}

func (ar *AssessmentReport) GetShardedTablesRecommendation() ([]string, error) {
if ar.Sizing == nil {
return nil, fmt.Errorf("sizing report is null, can't fetch sharded tables")
Expand Down
68 changes: 43 additions & 25 deletions yb-voyager/cmd/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ limitations under the License.
*/
package cmd

import (
"github.com/yugabyte/yb-voyager/yb-voyager/src/constants"
"github.com/yugabyte/yb-voyager/yb-voyager/src/utils"
)

const (
KB = 1024
MB = 1024 * 1024
Expand Down Expand Up @@ -104,7 +109,8 @@ const (

UNSUPPORTED_FEATURES = "unsupported_features"
UNSUPPORTED_DATATYPES = "unsupported_datatypes"
UNSUPPORTED_PLPGSQL_OBEJCTS = "unsupported_plpgsql_objects"
UNSUPPORTED_PLPGSQL_OBJECTS = "unsupported_plpgsql_objects"
MIGRATION_CAVEATS = "migration_caveats"
REPORT_UNSUPPORTED_QUERY_CONSTRUCTS = "REPORT_UNSUPPORTED_QUERY_CONSTRUCTS"

HTML = "html"
Expand Down Expand Up @@ -173,21 +179,14 @@ const (
List of all the features we are reporting as part of Unsupported features and Migration caveats
*/
const (
// AssessmentIssue types used in YugabyteD payload
FEATURE = "feature"
DATATYPE = "datatype"
QUERY_CONSTRUCT = "query_construct" // confused: in json for some values we are using space separated and for some snake_case
MIGRATION_CAVEATS = "migration_caveats"
PLPGSQL_OBJECT = "plpgsql_object"

// Description
FEATURE_ISSUE_TYPE_DESCRIPTION = "Features of the source database that are not supported on the target YugabyteDB."
DATATYPE_ISSUE_TYPE_DESCRIPTION = "Data types of the source database that are not supported on the target YugabyteDB."
MIGRATION_CAVEATS_TYPE_DESCRIPTION = "Migration Caveats highlights the current limitations with the migration workflow."
UNSUPPORTED_QUERY_CONSTRUTS_DESCRIPTION = "Source database queries not supported in YugabyteDB, identified by scanning system tables."
UNSUPPPORTED_PLPGSQL_OBJECT_DESCRIPTION = "Source schema objects having unsupported statements on the target YugabyteDB in PL/pgSQL code block"
SCHEMA_SUMMARY_DESCRIPTION = "Objects that will be created on the target YugabyteDB."
SCHEMA_SUMMARY_DESCRIPTION_ORACLE = SCHEMA_SUMMARY_DESCRIPTION + " Some of the index and sequence names might be different from those in the source database."
FEATURE_CATEGORY_DESCRIPTION = "Features of the source database that are not supported on the target YugabyteDB."
DATATYPE_CATEGORY_DESCRIPTION = "Data types of the source database that are not supported on the target YugabyteDB."
MIGRATION_CAVEATS_CATEGORY_DESCRIPTION = "Migration Caveats highlights the current limitations with the migration workflow."
UNSUPPORTED_QUERY_CONSTRUCTS_CATEGORY_DESCRIPTION = "Source database queries not supported in YugabyteDB, identified by scanning system tables."
UNSUPPPORTED_PLPGSQL_OBJECT_CATEGORY_DESCRIPTION = "Source schema objects having unsupported statements on the target YugabyteDB in PL/pgSQL code block"
SCHEMA_SUMMARY_DESCRIPTION = "Objects that will be created on the target YugabyteDB."
SCHEMA_SUMMARY_DESCRIPTION_ORACLE = SCHEMA_SUMMARY_DESCRIPTION + " Some of the index and sequence names might be different from those in the source database."

//Unsupported Features

Expand Down Expand Up @@ -222,16 +221,16 @@ const (
// Migration caveats

//POSTGRESQL
ALTER_PARTITION_ADD_PK_CAVEAT_FEATURE = "Alter partitioned tables to add Primary Key"
FOREIGN_TABLE_CAVEAT_FEATURE = "Foreign tables"
POLICIES_CAVEAT_FEATURE = "Policies"
UNSUPPORTED_DATATYPES_LIVE_CAVEAT_FEATURE = "Unsupported Data Types for Live Migration"
UNSUPPORTED_DATATYPES_LIVE_WITH_FF_FB_CAVEAT_FEATURE = "Unsupported Data Types for Live Migration with Fall-forward/Fallback"
UNSUPPORTED_DATATYPES_FOR_LIVE_MIGRATION_ISSUE = "There are some data types in the schema that are not supported by live migration of data. These columns will be excluded when exporting and importing data in live migration workflows."
UNSUPPORTED_DATATYPES_FOR_LIVE_MIGRATION_WITH_FF_FB_ISSUE = "There are some data types in the schema that are not supported by live migration with fall-forward/fall-back. These columns will be excluded when exporting and importing data in live migration workflows."
DESCRIPTION_ADD_PK_TO_PARTITION_TABLE = `After export schema, the ALTER table should be merged with CREATE table for partitioned tables as alter of partitioned tables to add primary key is not supported.`
DESCRIPTION_FOREIGN_TABLES = `During the export schema phase, SERVER and USER MAPPING objects are not exported. These should be manually created to make the foreign tables work.`
DESCRIPTION_POLICY_ROLE_ISSUE = `There are some policies that are created for certain users/roles. During the export schema phase, USERs and GRANTs are not exported. Therefore, they will have to be manually created before running import schema.`
ALTER_PARTITION_ADD_PK_CAVEAT_FEATURE = "Alter partitioned tables to add Primary Key"
FOREIGN_TABLE_CAVEAT_FEATURE = "Foreign tables"
POLICIES_CAVEAT_FEATURE = "Policies"
UNSUPPORTED_DATATYPES_LIVE_CAVEAT_FEATURE = "Unsupported Data Types for Live Migration"
UNSUPPORTED_DATATYPES_LIVE_WITH_FF_FB_CAVEAT_FEATURE = "Unsupported Data Types for Live Migration with Fall-forward/Fallback"
UNSUPPORTED_DATATYPES_FOR_LIVE_MIGRATION_DESCRIPTION = "There are some data types in the schema that are not supported by live migration of data. These columns will be excluded when exporting and importing data in live migration workflows."
UNSUPPORTED_DATATYPES_FOR_LIVE_MIGRATION_WITH_FF_FB_DESCRIPTION = "There are some data types in the schema that are not supported by live migration with fall-forward/fall-back. These columns will be excluded when exporting and importing data in live migration workflows."
DESCRIPTION_ADD_PK_TO_PARTITION_TABLE = `After export schema, the ALTER table should be merged with CREATE table for partitioned tables as alter of partitioned tables to add primary key is not supported.`
DESCRIPTION_FOREIGN_TABLES = `During the export schema phase, SERVER and USER MAPPING objects are not exported. These should be manually created to make the foreign tables work.`
DESCRIPTION_POLICY_ROLE_DESCRIPTION = `There are some policies that are created for certain users/roles. During the export schema phase, USERs and GRANTs are not exported. Therefore, they will have to be manually created before running import schema.`
)

var supportedSourceDBTypes = []string{ORACLE, MYSQL, POSTGRESQL, YUGABYTEDB}
Expand All @@ -244,3 +243,22 @@ var validSSLModes = map[string][]string{
}

var EVENT_BATCH_MAX_RETRY_COUNT = 50

// returns the description for a given assessment issue category
func GetCategoryDescription(category string) string {
switch category {
case constants.FEATURE:
return FEATURE_CATEGORY_DESCRIPTION
case constants.DATATYPE:
return DATATYPE_CATEGORY_DESCRIPTION
case constants.QUERY_CONSTRUCT:
return UNSUPPORTED_QUERY_CONSTRUCTS_CATEGORY_DESCRIPTION
case constants.PLPGSQL_OBJECT:
return UNSUPPPORTED_PLPGSQL_OBJECT_CATEGORY_DESCRIPTION
case constants.MIGRATION_CAVEATS:
return MIGRATION_CAVEATS_CATEGORY_DESCRIPTION
default:
utils.ErrExit("unsupported assessment issue category %q", category)
}
return ""
}
14 changes: 14 additions & 0 deletions yb-voyager/src/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,24 @@ const (
// Database Object types
TABLE = "table"
FUNCTION = "function"
COLUMN = "column"
Copy link
Contributor

Choose a reason for hiding this comment

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

db object types are mostly used all in caps in code TABLE, FUNCTION or COLUMN

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it has been there like that before, lets change it later. Don't want to risk the scope of this PR.


// Source DB Types
YUGABYTEDB = "yugabytedb"
POSTGRESQL = "postgresql"
ORACLE = "oracle"
MYSQL = "mysql"

// AssessmentIssue Categoes - used by YugabyteD payload and Migration Complexity Explainability
// TODO: soon to be renamed as SCHEMA, SCHEMA_PLPGSQL, DML_QUERY, MIGRATION_CAVEAT, "DATATYPE"
FEATURE = "feature"
DATATYPE = "datatype"
QUERY_CONSTRUCT = "query_construct"
MIGRATION_CAVEATS = "migration_caveats"
PLPGSQL_OBJECT = "plpgsql_object"

// constants for the Impact Buckets
IMPACT_LEVEL_1 = "LEVEL_1" // Represents minimal impact like only the schema ddl
IMPACT_LEVEL_2 = "LEVEL_2" // Represents moderate impact like dml queries which might impact a lot of implementation/assumption in app layer
IMPACT_LEVEL_3 = "LEVEL_3" // Represent significant impact like TABLE INHERITANCE, which doesn't have any simple workaround but can impact multiple objects/apps
)
46 changes: 44 additions & 2 deletions yb-voyager/src/issue/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@ limitations under the License.
package issue

import (
"github.com/samber/lo"
"github.com/yugabyte/yb-voyager/yb-voyager/src/constants"
"github.com/yugabyte/yb-voyager/yb-voyager/src/ybversion"
)

type Issue struct {
Type string // (advisory_locks, index_not_supported, etc)
TypeName string // for display
TypeDescription string
Name string // for display
Description string
Impact string
Suggestion string
GH string
DocsLink string
Expand All @@ -40,3 +43,42 @@ func (i Issue) IsFixedIn(v *ybversion.YBVersion) (bool, error) {
}
return v.GreaterThanOrEqual(minVersionFixedInSeries), nil
}

func (i Issue) GetImpact() string {
// Default value as IMPACT_LEVEL_1 if not defined in issue
return lo.Ternary(i.Impact != "", i.Impact, constants.IMPACT_LEVEL_1)
}

/*
Dynamic Impact Determination (TODO)
- We can define the impact calculator function based on issue type wherever/whenever needed
- Map will have functions only for issue type with dynamic impact determination

For example:

type ImpactCalcFunc func(issue QueryIssue, stats *PgStats) string

var impactCalculators = map[string]ImpactCalcFunc{
INHERITED_TABLE: inheritedTableImpactCalc,
// etc...
}

// Example dynamic function
func inheritedTableImpactCalc(i QueryIssue, stats *PgStats) string {
usage := stats.GetUsage(i.ObjectName) // e.g. how many reads/writes
if usage.WritesPerDay > 1000 {
return "LEVEL_2"
}
return "LEVEL_3"
}

// Update existing GetImpact() method
func (i Issue) GetImpact(stats *PgStats) string {
sanyamsinghal marked this conversation as resolved.
Show resolved Hide resolved
if calc, ok := impactCalculators[i.Type]; ok {
return calc(i, stats)
}

return lo.Ternary(i.Impact != "", i.Impact, constants.IMPACT_LEVEL_1)
}

*/
Loading