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

Revert "Revert "Fix for fast/cold-boot: call db_migrator only after old config is loaded"" #15684

Merged

Conversation

vaibhavhd
Copy link
Contributor

Reverts #15464

Adds original PR back: #14933

Why I did it

Fix the issue where db_migrator is called before DB is loaded w/ config. This leads to db_migrator:

  1. Not finding anything, and resumes to incorrectly migrate every missing config
    This is not expected. migration should happen after the old config is loaded and only new schema changes need migration.
  2. Since DB does not have anything when migrator is called, db_migrator fails when some APIs return None.

The reason for incorrect call is that:

  1. database service starts db_migrator as part of startup sequence.
  2. config-setup service loads data from old-config/minigraph. However, since it has Requires=database.service.
  3. Hence, config-setup starts only when database service is started. And database service is started when db_migrator is completed.

Fixed by:

  1. Check if this is first time boot by checking pending_config_migration flag.
  2. If pending_config_migration is enabled, then do not call db_migrator as part of database service startup.
  3. Let database service start which triggers config-setup service to start.
  4. Now call db_migrator after when config-setup service loads old-config/minigraph

Error that's being fixed:

May  2 20:35:04 sonic database.sh[649]: Creating new database container
May  2 20:35:04 sonic database.sh[663]: 99e8edba01ed0c7581f0d61dd2fa78374fa4f23e636a957004dd03a6f68eea86
May  2 20:35:04 sonic root: Starting database service...
May  2 20:35:06 sonic database.sh[690]: database
May  2 20:35:10 sonic database.sh[926]: True

May  2 20:35:10 sonic database.sh[928]:   File "/usr/local/bin/db_migrator.py", line 714, in common_migration_ops
May  2 20:35:10 sonic database.sh[928]:   File "/usr/local/bin/db_migrator.py", line 741, in migrate
May  2 20:35:10 sonic database.sh[928]:   File "/usr/local/bin/db_migrator.py", line 782, in main
May  2 20:35:10 sonic database.sh[928]: Traceback (most recent call last):
May  2 20:35:10 sonic database.sh[928]: TypeError: argument of type 'NoneType' is not iterable
May  2 20:35:10 sonic database.sh[928]: argument of type 'NoneType' is not iterable
May  2 20:35:10 sonic database.sh[928]: optional arguments:
May  2 20:35:10 sonic database.sh[928]: usage: db_migrator.py [-h] [-o operation migrate, set_version, get_version]
May  2 20:35:10 sonic db_migrator: :- operator(): DB '{APPL_DB}' is empty with pattern 'COPP_TABLE:*'!
May  2 20:35:10 sonic db_migrator: :- operator(): DB '{APPL_DB}' is empty with pattern 'INTF_TABLE:*'!
May  2 20:35:10 sonic db_migrator: :- operator(): Key 'BUFFER_MAX_PARAM_TABLE|global' field 'mmu_size' unavailable in database 'STATE_DB'
May  2 20:35:10 sonic db_migrator: :- operator(): Key 'WARM_RESTART_ENABLE_TABLE|system' field 'enable' unavailable in database 'STATE_DB'
May  2 20:35:10 sonic db_migrator: Caught exception: argument of type 'NoneType' is not iterable

May  2 20:35:11 sonic config-setup[935]: Copying SONiC configuration minigraph.xml ...
May  2 20:35:11 sonic config-setup[935]: Reloading minigraph...
May  2 20:35:11 sonic config-setup[935]: Use minigraph.xml from old system...

May  2 20:35:11 sonic root: Started database service...
Work item tracking
  • Microsoft ADO (number only): 15829809

How I did it

How to verify it

ested on physical platform. Rebooted from 202012 to master image (new install):

db_migrator now gets delayed during database service bring up: Delaying db_migrator until config migration is over
db_migrator now gets called when config-setup service loads DB. The issue of migrating on empty DB is fixed and is evident as the previous error of hwsku being None is not seen anymore:

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@vaibhavhd vaibhavhd requested a review from yxieca July 6, 2023 05:46
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202211: #15745

@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202205: #15746

@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202305: #16134

@mssonicbld
Copy link
Collaborator

@vaibhavhd PR conflicts with 202211 branch

@vaibhavhd
Copy link
Contributor Author

@qiluo-msft , please pick this PR to 202012 branch. Please let me know if a separate PR is needed.

@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202012: #16223

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Aug 21, 2023
mssonicbld added a commit that referenced this pull request Aug 21, 2023
sonic-otn pushed a commit to sonic-otn/sonic-buildimage that referenced this pull request Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants