Restrict live migration to single table-list and added guardrails for that around table-list flags #2354

priyanshi-yb · 2025-02-24T08:20:20Z

Describe the changes in this pull request

During resumption in the command export data from source we are now using the stored table list in MSR TableListExportedFromSource and SourceExportedTableListWithLeafPartitions based on the source type as for PostgreSQL source we allow filtering the leaf partitions as well so storing them in MSR.
In the command export data from target, we use the stored table list in MSR TableListExportedFromSource to populate the leaf partitions from the target for all the partitioned tables. We then store this information in the TargetExportedTableListWithLeafPartitions for further runs to avoid fetching new leaf partitions from the target.
Added guardrails around the table-list and exclude-table-list flags to check for any discrepancy between these on resumption and the initial lists.
1. If any new tables are passed via these flags, commands error out saying Unknown tables found.
2. If any table-list changes are found, commands report them on the console output of the commands, like Missing tables ... / Extra tables found ... than the initial list and command error out saying Re-run with the table-list passed in the initial run
For the case of the partition, if any new leaf tables are added during the migration on the partitioned tables that are getting migrated. On resumption, we will give a prompt to the user that some new leaf partitions are found and will not be considered for the migration if they are fine to continue or else restart the migration

Describe if there are any user-facing changes

Yes, as mentioned above there will be user-facing changes for some guardrails around table-list etc.. now user won't be able to change the table-list during the live migration

Behaviour in these scenarios

New table is added on database either source/target -

With or without table-list flags: export data from source/target commands will be unaware of newly added tables and won’t consider it either during the run or re-run.
If a newly added table is passed through the table-list/exclude-table-list - commands error out saying unknown table, valid tables names are […].

➜ yb-voyager export data --export-dir ... --export-type snapshot-and-changes  --table-list test_partitions_quences                           
...
Continuing streaming from where we left off...
Unknown table names [test_partitions_quences] in the include list
Valid table names are: [public.foo_bar public.sales_region public.test_seq1 public.try public.sequence_check1 public.london public.sydney public.boston public.test_partitions_sequences_b public.test_partitions_sequences_l public.test_partitions_sequences_s public.sequence_check2 public.se..]

New leaf table is added on partitioned table in database either source/target -

With or without table-list flags:
-- During the run, export data from source/target commands will be unaware of newly added leaf tables and won’t consider it.
-- On re-run, export data from source/target commands will report to the user that new leaf tables are detected for partitioned tables with below msg -

Detected new partition tables for the following partitioned tables. These will not be considered during migration:
Root table: public.test_partitions_sequences, new leaf partitions: public.test_partitions_sequences_h
Do you want to continue?? [Y/N]: Y
num tables to export: 1
table list for data export: [test_partitions_sequences (test_partitions_sequences_s)]

If a new leaf partition table is passed through the table-list/exclude-table-list flags - commands error out saying unknown table; valid tables names are […].

➜  yb-voyager export data --export-dir .... --export-type snapshot-and-changes  --table-list test_partitions_sequences_h
....
Continuing streaming from where we left off...
Unknown table names [test_partitions_sequences_h] in the include list
Valid table names are: [public.foo_bar public.sales_region public.test_seq1 public.try public.sequence_check1 public.london public.sydney public.boston public.test_partitions_sequences_b public.test_partitions_sequences_l public.test_partitions_sequences_s public.sequence_check2 publi.....]

Table-list changes during resumption via flags

In case some tables are removed from the initial list in resumption - commands will report that there is discrepancy found and error out as mentioned below

  TARGET_DB_PASSWORD=*** yb-voyager export data from target --export-dir ... --table-list public."accounts_list_partitioned",public."empty_partition_table2",public."orders_interval_partition" --exclude-table-list empty_partition_table2
migrationID: 822cd9e3-00bc-4c7f-8d8c-195cdd3e24b7
Note: Live migration is a TECH PREVIEW feature.
export of data for source type as 'yugabytedb'
Continuing streaming from where we left off...
Changing the table list during live-migration is not allowed.
Missing tables in the current run compared to the initial list: empty_partition_table2_p_west,empty_partition_table2_p_east
Table list passed in the initial run of migration - [public.accounts_list_partitioned_p_northwest .... public.orders_interval_partition_interval_partition_less_than_2018 public.orders_interval_partition]. 
Re-run the command with the table list passed in the initial run of migration.

In case some tables are added from the initial list in resumption - commands will report that there is discrepancy found and error out as mentioned below

 TARGET_DB_PASSWORD=*** yb-voyager export data from target --export-dir /home/centos/code/yb-voyager/migtests/tests/oracle/partitions/oracle_partitions_fallb_export-dir --table-list public."accounts_list_partitioned",public."empty_partition_table2",public."orders_interval_partition",empty_partition_table ...
migrationID: 822cd9e3-00bc-4c7f-8d8c-195cdd3e24b7
Note: Live migration is a TECH PREVIEW feature.
export of data for source type as 'yugabytedb'
Continuing streaming from where we left off...
Changing the table list during live-migration is not allowed.
Extra tables in the current run compared to the initial list: empty_partition_table2_p_extra,empty_partition_table_p_west,empty_partition_table_p_east,empty_partition_table
Table list passed in the initial run of migration - [public.accounts_list_partitioned_p_northwest .... public.orders_interval_partition]. 
Re-run the command with the table list passed in the initial run of migration.

How was this pull request tested?

Added unit tests with test container PG source for various cases.

Validating the table-list in the first-run
validating guardrails check prompts in subsequent runs
validating new leaf additions
validating unknown table case in table-list via error exit (any dummy table name or new table added on source)

Does your PR have changes that can cause upgrade issues?

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

…es on tables that are not part of migrations

… one

…ctName and then re-using the code to first check if it is leaf table else lookup

…ags storing list with leaf in msr directly

… msr new fields with nil etc..

yb-voyager/cmd/common.go

yb-voyager/cmd/exportData.go

makalaaneesh · 2025-02-26T10:53:04Z

yb-voyager/cmd/exportData.go

+	}
+
+	if len(lo.Keys(newLeafTables)) > 0 {
+		utils.PrintAndLog("Detected new partition tables for the following partitioned tables. These will not be considered during migration:")


Should we prompt?

Not sure if a prompt is required, I think there is nothing we want users to do here right? so this might just FYI?

makalaaneesh

Went over it at a high level, looks good!
Will give it a more detailed look soon along with the tests.

yb-voyager/cmd/exportData.go

… DDL changes on source, subsequent run with table-list/exclude-table-list flags. Some TODOs remaining to added assertions for

yb-voyager/cmd/exportData_test.go

makalaaneesh · 2025-02-27T15:43:45Z

yb-voyager/cmd/exportData_test.go

+
+	// getInitialTableList for subsequent run with  start-clean false and basic without table-list flags so no guardrails
+	startClean = false
+	getInitialTableistAndAssertExpectedResult(t, expectedTableList, expectedPartitionsToRootMap)


you added a new partition leaf above. This should error out no?

We are not errorring out in this case just reporting to the user that new leafs found for these root tables and these will not be considered during migration,

Ticket for having a guardrail check during the ongoing live migration for new leaf tables #2356

yb-voyager/cmd/exportData_test.go

…ls and new leafs addition. Added tests for subsequent run validating the guardrails of missing and extra tables and new leaf tables addition

…ion as a function variable to be able to override it in the test

…s by adding a new field in nameRegfor Sequences detemination

yb-voyager/cmd/exportData.go

makalaaneesh · 2025-03-03T07:25:13Z

yb-voyager/cmd/exportData.go

 	} else {
-		tableList = fullTableList
+		//this is only for filtering the non-leaf and non-root tables - the mid level partitioned table from fullTableList


To remove non-leaf and non-root, you're still calling addLeafPartitionsInTableList? That sounds a bit weird. Does that function ignore all leafs, and then re-add them?

addLeafPartitionsInTableList has a boolean parameter addAllLeafPartitions in case this is false, that function will consider that the table-list passed is already exhaustive with all the tables (root, middle partition, leaf partitions, so on..) and it will just do the filtering to remove the middle level partitions and just the keep the leaf and root tables in the list.

nit: write a separate specific function to remove the non-leaf/non-root tables.
Add a TODO if defering.

yb-voyager/cmd/exportData.go

yb-voyager/src/namereg/namereg.go

yb-voyager/cmd/exportData_test.go

makalaaneesh · 2025-03-03T07:47:32Z

yb-voyager/cmd/exportData_test.go

+// Tests the unknown table case by over ridding the utils.ErrExit function to assert the error msg
+func testUnknownTableCaseForTableListFlags(t *testing.T, expectedUnknownErrorMsg string) {
+	previousFunction := utils.ErrExit
+	//changing the error exit function to test the unknown table scenario


oo, this does not seem like the best way to do it. To be fair, it essentially is just like "mocking" a function, so it's okay, but I think the right way would be:
If we write our functions in a way such that they return a list of guardrail failures, then we can just assert that, instead of having to mocking the errExit function.

Is this too much work given our current state?

It's not a lot of work. I think it's more of a choice between these ways -

Either we directly display the problem/guardrail and error out

Return as an error and then display it on the console eventually with the whole context.

e.g.

➜ yb-voyager export data ... --export-type snapshot-and-changes --table-list asdfa migrationID: a1194e55-3d95-4d37-95e3-d5221eed976d .... Unknown table names in the include list: [asdfa] Valid table names are: [...] ➜ yb-voyager export data ... --export-type snapshot-and-changes --table-list asdfa migrationID: a1194e55-3d95-4d37-95e3-d5221eed976d ... error getting initial table list: error applying table list flags for current list and remove roots: error in apply table list filter on registered list for the flags in current run: Unknown table names in the include list: [asdfa] Valid table names are: [...]

I think the first one is better, where we directly display the problem/guardrails because it's not an actual error per say.
Let me know what do you think?

yb-voyager/cmd/exportData_test.go

yb-voyager/cmd/exportData.go

yb-voyager/cmd/exportData_test.go

makalaaneesh · 2025-03-03T14:43:06Z

yb-voyager/cmd/exportData.go

+		}
+
+		//Filtering the new leaf tables if present in filteredListWithoutRootTable as we have reported above
+		if isNewLeafTable(t) {


Why/how would the new leaf appear in currentRunTableListFilteredViaFlags?

While applying include/exclude flags, we also add leaf partitions in case only the root is passed in flags.The addLeafPartitions function goes to db for that case

makalaaneesh

LGTM! pls update the PR description to reflect error vs prompt and if any other changes

priyanshi-yb added 5 commits February 20, 2025 23:53

Initial changes for hanlding the table names for supporting ddl chang…

245034c

…es on tables that are not part of migrations

in case includetable list is not used then use stored list as include…

28736f3

… one

changed logic of getting getting registered list to first return obje…

0fe89a2

…ctName and then re-using the code to first check if it is leaf table else lookup

minor fix

81ebdc7

add leaf partititons in include list for guardrails

e07e69f

priyanshi-yb changed the title ~~Restrict live migration to single table-list from export data from source and guardrail for that around table-list flags~~ Restrict live migration to single table-list and guardrail for that around table-list flags Feb 24, 2025

handle the case where only subset of leaf partitions are passed in fl…

ce05787

…ags storing list with leaf in msr directly

priyanshi-yb force-pushed the priyanshi/name-reg-table-name branch from 850a85b to ce05787 Compare February 24, 2025 13:03

priyanshi-yb added 2 commits February 24, 2025 21:48

fix the export data from target case after testing

f275d35

minor cleanup

f09e16f

priyanshi-yb force-pushed the priyanshi/name-reg-table-name branch from bf3e4e0 to f09e16f Compare February 24, 2025 16:35

priyanshi-yb requested a review from makalaaneesh February 25, 2025 07:58

priyanshi-yb added 2 commits February 25, 2025 17:06

Report the new leaf tables added upfront on re-run in the command output

65fd658

minor cleanup

c760d92

priyanshi-yb marked this pull request as ready for review February 25, 2025 12:26

priyanshi-yb changed the title ~~Restrict live migration to single table-list and guardrail for that around table-list flags~~ Restrict live migration to single table-list and added guardrails for that around table-list flags Feb 25, 2025

Minor fix to handle upgrade and resumption in export data, hanlde the…

f8869de

… msr new fields with nil etc..

makalaaneesh reviewed Feb 26, 2025

View reviewed changes

priyanshi-yb added 2 commits February 27, 2025 00:00

restructured the code in smaller re-usable functions for table-list area

5da3738

minor change

2d132cb

priyanshi-yb requested a review from makalaaneesh February 27, 2025 07:27

minor misss for first run in export data from target

fa0370d

makalaaneesh reviewed Feb 27, 2025

View reviewed changes

yb-voyager/cmd/exportData.go Show resolved Hide resolved

yb-voyager/cmd/exportData.go Show resolved Hide resolved

yb-voyager/cmd/exportData.go Show resolved Hide resolved

priyanshi-yb added 5 commits February 27, 2025 17:53

added unit tests for fetchTablesNamesFromSourceAndFilterTableList

90e2dd7

fix git tag

36e02eb

clean db after each test

d042d3d

test for the subsequent run basic

bdd9b39

more tests on Subsequent run with basic start-clean true/false cases,…

e64f29f

… DDL changes on source, subsequent run with table-list/exclude-table-list flags. Some TODOs remaining to added assertions for

makalaaneesh reviewed Feb 27, 2025

View reviewed changes

Restructured the code a bit to be able to add more tests for guardrai…

b842a60

…ls and new leafs addition. Added tests for subsequent run validating the guardrails of missing and extra tables and new leaf tables addition

priyanshi-yb added 5 commits February 28, 2025 19:02

Added Unknown table case as well, by changing the utils.ErrExit funct…

25d97fc

…ion as a function variable to be able to override it in the test

Merge branch 'main' into priyanshi/name-reg-table-name

0cd8597

minor change

ccc7316

minor change

e4f8a89

fixed the 'GetRegisteredTableList' function to return only table name…

b034db2

…s by adding a new field in nameRegfor Sequences detemination

priyanshi-yb force-pushed the priyanshi/name-reg-table-name branch from 3ed77c3 to b034db2 Compare March 3, 2025 07:05

minor miss

51db76b

makalaaneesh reviewed Mar 3, 2025

View reviewed changes

yb-voyager/cmd/exportData_test.go Show resolved Hide resolved

priyanshi-yb added 3 commits March 3, 2025 14:56

some review comments

a61d355

minor comment

0325e8b

minor change

be7e023

priyanshi-yb requested a review from makalaaneesh March 3, 2025 14:38

makalaaneesh reviewed Mar 3, 2025

View reviewed changes

priyanshi-yb added 2 commits March 4, 2025 15:09

review comments

083bf4a

some comments

7d40f76

priyanshi-yb requested a review from makalaaneesh March 4, 2025 09:46

makalaaneesh approved these changes Mar 4, 2025

View reviewed changes

priyanshi-yb merged commit 7e0bfa4 into main Mar 4, 2025
66 checks passed

priyanshi-yb deleted the priyanshi/name-reg-table-name branch March 4, 2025 12:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restrict live migration to single table-list and added guardrails for that around table-list flags #2354

Restrict live migration to single table-list and added guardrails for that around table-list flags #2354

priyanshi-yb commented Feb 24, 2025 •

edited

Loading

makalaaneesh Feb 26, 2025

priyanshi-yb Feb 27, 2025

makalaaneesh left a comment

makalaaneesh Feb 27, 2025

priyanshi-yb Mar 3, 2025

priyanshi-yb Mar 4, 2025

makalaaneesh Mar 3, 2025

priyanshi-yb Mar 3, 2025 •

edited

Loading

makalaaneesh Mar 4, 2025

makalaaneesh Mar 3, 2025

priyanshi-yb Mar 3, 2025 •

edited

Loading

makalaaneesh Mar 3, 2025

priyanshi-yb Mar 3, 2025 •

edited

Loading

makalaaneesh left a comment

Restrict live migration to single table-list and added guardrails for that around table-list flags #2354

Restrict live migration to single table-list and added guardrails for that around table-list flags #2354

Conversation

priyanshi-yb commented Feb 24, 2025 • edited Loading

Describe the changes in this pull request

Describe if there are any user-facing changes

How was this pull request tested?

Does your PR have changes that can cause upgrade issues?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

makalaaneesh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

priyanshi-yb Mar 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

priyanshi-yb Mar 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

priyanshi-yb Mar 3, 2025 • edited Loading

Choose a reason for hiding this comment

makalaaneesh left a comment

Choose a reason for hiding this comment

priyanshi-yb commented Feb 24, 2025 •

edited

Loading

priyanshi-yb Mar 3, 2025 •

edited

Loading

priyanshi-yb Mar 3, 2025 •

edited

Loading

priyanshi-yb Mar 3, 2025 •

edited

Loading