Skip to content

Comments

Ignore nonexistent include in database_global.json#1041

Merged
qiluo-msft merged 11 commits intosonic-net:masterfrom
yue-fred-gao:ignore_nonexist_include_global
Aug 8, 2025
Merged

Ignore nonexistent include in database_global.json#1041
qiluo-msft merged 11 commits intosonic-net:masterfrom
yue-fred-gao:ignore_nonexist_include_global

Conversation

@yue-fred-gao
Copy link
Contributor

why

SonicDBConfig::initializeGlobalConfig aborts parsing global config if it encounters an include of a database config that doesn't exist. The global config file includes all possible includes in the switch. However, not all includes are relevant to a client. For example, in smartswitch, dash-ha container only cares NPU databases and databases of the DPU managed by the container. The current behavior requires a dash-ha container mounting all the DPU database instances.

what this PR does

  1. Add ignore_nonexistent flag, with default value false, to function parseDatabaseConfig. If it is set to true, it will not throw error if the included database config file doesn't exist.
  2. initializeGlobalConfig sets ignore_nonexistent to true when calling parseDatabaseConfig. If inst_entry, db_entry and separator_entry are empty, it will ignore the key.

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Pterosaur
Pterosaur previously approved these changes Jul 15, 2025
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yue-fred-gao
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yue-fred-gao
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yue-fred-gao
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yue-fred-gao
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yue-fred-gao
Copy link
Contributor Author

hi @liuh-80 , is anybody investigating the test failure in PerformancetimerTest.basic?

tests/performancetimer_ut.cpp:42: Failure
Expected equality of these values:
  output
    Which is: "{\"API\":\"basic_set_name\",\"RPS[k]\":4.9,\"Tasks\":3000,\"Total[ms]\":607,\"busy[ms]\":400,\"idle[ms]\":207,\"m_gaps\":[7,200],\"m_incs\":[1000,2000],\"m_intervals\":[100,300]}"
  expected
    Which is: "{\"API\":\"basic_set_name\",\"RPS[k]\":5.0,\"Tasks\":3000,\"Total[ms]\":600,\"busy[ms]\":400,\"idle[ms]\":200,\"m_gaps\":[0,200],\"m_incs\":[1000,2000],\"m_intervals\":[100,300]}"
[  FAILED  ] PerformancetimerTest.basic (708 ms)

@mssonicbld
Copy link
Collaborator

/azp run

@yue-fred-gao yue-fred-gao dismissed stale reviews from Pterosaur and zjswhhh via b07af70 August 8, 2025 17:01
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@qiluo-msft qiluo-msft enabled auto-merge (squash) August 8, 2025 18:03
@qiluo-msft qiluo-msft merged commit 1484a85 into sonic-net:master Aug 8, 2025
15 checks passed

/// Rust wrapper around `swss::SonicDBConfig::initializeGlobalConfig`.
pub fn sonic_db_config_initialize_global(path: &str) -> Result<(), Exception> {
pub fn sonic_db_config_initialize_global(path: &str, ignore_nonexistent: bool) -> Result<(), Exception> {
Copy link
Contributor

Choose a reason for hiding this comment

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

sonic_db_config_initialize_global

This will break existing Rust application using this function. Since Rust does not support default parameter value like Python/C++, does not support function overloading. There are 2 options:

  1. keep original function unchanged, and add anotehr function like sonic_db_config_initialize_global_ignore_nonexistent usiing more parameters.
  2. treat it as breaking change, and update all the Rust applications.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hi @qiluo-msft, I plan to commit the change in dash-ha at the same time. When we update sonic-buildimage, it needs to go in at the same time as this. Are there other rust applications using this function? If there is and hard to coordinate, I can implement 1 to break the dependency.

Copy link
Contributor

@qiluo-msft qiluo-msft Aug 12, 2025

Choose a reason for hiding this comment

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

As far as I know, only dash-ha is using it.

Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I know, only dash-ha is using it.

Hi @qiluo-msft , I'm using this rest API as well in the swss.
sonic-net/sonic-swss#3796

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Pterosaur , is it in the committed code that needs to be updated at the same time?

@mssonicbld
Copy link
Collaborator

Cherry-pick PR to msft-202506: Azure/sonic-swss-common.msft#53

@mssonicbld
Copy link
Collaborator

Cherry-pick PR to msft-202412: Azure/sonic-swss-common.msft#64

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.

6 participants