[orchagent, cfgmgr] Add response publisher and state recording by bocon13 · Pull Request #1992 · sonic-net/sonic-swss

bocon13 · 2021-10-28T05:23:46Z

Add response publisher.
Add APPL STATE DB recording.
Add response path into vrforch.

Submission containing materials of a third party:
Copyright Google LLC; Licensed under Apache 2.0

Co-authored-by: Runming Wu runmingwu@google.com
Co-authored-by: Yilan Ji yilanji@google.com
Co-authored-by: Akarsh Gupta akarshgupta@google.com
Co-authored-by: Jay Hu jiarenhu@google.com

Signed-off-by: Don Newton don@opennetworking.org

Depends on pins/sonic-swss-common-public#2

See the Response path HLD for more details:
https://github.com/pins/SONiC/blob/master/doc/pins/appl_state_db_response_path_hld.md

bocon13 · 2021-10-28T05:24:12Z

cc: @mint570 @prsunny

orchagent/vrforch.cpp

orchagent/orch.cpp

lgtm-com · 2021-11-02T17:18:34Z

This pull request introduces 1 alert and fixes 4 when merging 988c7ff into e92c1df - view on LGTM.com

new alerts:

1 for Syntax error

fixed alerts:

2 for Unused local variable
2 for Unused import

lgtm-com · 2021-11-02T18:33:43Z

This pull request introduces 1 alert and fixes 4 when merging e4658d0 into e92c1df - view on LGTM.com

new alerts:

1 for Syntax error

fixed alerts:

2 for Unused local variable
2 for Unused import

orchagent/orch.cpp

orchagent/main.cpp

orchagent/orch.cpp

orchagent/orch.h

orchagent/response_publisher.cpp

mint570 · 2021-11-05T19:17:23Z

@prsunny , all comments have been addressed now. Please take a look. Thanks.

cfgmgr/buffermgrd.cpp

donNewtonAlpha · 2021-11-13T00:35:40Z

/azpw run

orchagent/orch.cpp

donNewtonAlpha · 2021-11-17T23:30:15Z

I think Brian and I will need to revisit this tomorrow. We obviously pulled forward some stuff when we squashed and rebased

…

On Wed, Nov 17, 2021 at 6:27 PM StephenWangGoogle ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In orchagent/orch.cpp <#1992 (comment)>: > } catch (const std::logic_error& e) { - SWSS_LOG_ERROR("Logic error: %s", e.what()); + rc = ReturnCode(StatusCode::SWSS_RC_INTERNAL) + << "Logic error: " << e.what(); + SWSS_LOG_ERROR("%s", rc.message().c_str()); + SWSS_RAISE_CRITICAL_STATE(rc.message()); @mint570 <https://github.com/mint570> - is this intended? Did we add the critical state API in? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1992 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJ4SJMXNHDKJ7J376MY4ILUMQ27VANCNFSM5G36W3CA> .

orchagent/orch.cpp

StephenWangGoogle · 2021-11-18T19:21:16Z

Hi @prsunny , the PR is updated and all comments should be addressed. It can't be tested now since the swss-common 545 PR is not in yet but can you do another pass of review now?

prsunny · 2021-11-19T22:02:53Z

please revert the last commit.. it has introduced a lot of unwanted changes.. Please fix the few lines thats newly added to main.cpp

donNewtonAlpha · 2021-11-19T22:17:24Z

do you want the new files fixed as well?

prsunny · 2021-11-22T19:07:51Z

/azp run

azure-pipelines · 2021-11-22T19:08:02Z

Azure Pipelines successfully started running 1 pipeline(s).

prsunny · 2021-11-22T21:45:11Z

@bocon13 , i'm not sure why this PR has consistent failure on the VS tests - Can you please take a look at this pipeline and check if its due to code change?

bocon13 · 2021-11-23T07:45:30Z

@prsunny I wasn't able to reproduce this today, but here's the error:

OSError: [Errno 24] Too many open files:

It might be related to the responsepublisher files that get created for each test? It might be as simple as bumping up the file limit. I'll continue to look...

prsunny · 2021-11-23T15:16:27Z

@prsunny I wasn't able to reproduce this today, but here's the error:
OSError: [Errno 24] Too many open files:
It might be related to the responsepublisher files that get created for each test? It might be as simple as bumping up the file limit. I'll continue to look...

But I thought its not enabled by default.

prsunny · 2021-11-23T15:16:38Z

/azp run

azure-pipelines · 2021-11-23T15:16:48Z

Azure Pipelines successfully started running 1 pipeline(s).

cfgmgr/Makefile.am

bocon13 · 2021-11-23T23:53:37Z

vs tests are failing because we are missing the APPL_STATE_DB from the database_config.json:
https://github.com/Azure/sonic-buildimage/blob/91a8432b6793cc70391cac0b8c4814627397acd4/platform/vs/docker-sonic-vs/database_config.json#L81-L85

This PR depends on: sonic-net/sonic-buildimage#9082

Update: rebased to master and tests are re-running

bocon13 · 2021-11-24T19:27:06Z

/azpw run

mssonicbld · 2021-11-24T19:27:07Z

/AzurePipelines run

azure-pipelines · 2021-11-24T19:27:17Z

Azure Pipelines successfully started running 1 pipeline(s).

bocon13 · 2021-11-24T21:33:07Z

/azpw run

mssonicbld · 2021-11-24T21:33:08Z

/AzurePipelines run

azure-pipelines · 2021-11-24T21:33:19Z

Azure Pipelines successfully started running 1 pipeline(s).

prsunny · 2021-11-25T05:44:04Z

/azp run

azure-pipelines · 2021-11-25T05:44:15Z

Azure Pipelines successfully started running 1 pipeline(s).

* Add response publisher. * Add APPL STATE DB recording. * Add response path into vrforch. Submission containing materials of a third party: Copyright Google LLC; Licensed under Apache 2.0 Co-authored-by: Runming Wu <runmingwu@google.com> Co-authored-by: Yilan Ji <yilanji@google.com> Co-authored-by: Akarsh Gupta <akarshgupta@google.com> Co-authored-by: Jay Hu <jiarenhu@google.com> Signed-off-by: Don Newton don@opennetworking.org

mint570 · 2021-11-26T02:24:44Z

@prsunny
All tests have been passed. Please approve and merge the pr. Thanks.

bocon13 · 2021-11-26T17:54:21Z

Thanks @prsunny!

691c37b [Route bulk] Fix bugs in case a SET operation follows a DEL operation in the same bulk (sonic-net/sonic-swss#2086) a4c80c3 patch for issue sonic-net/sonic-swss#1971 - enable Rx Drop handling for cisco-8000 (sonic-net/sonic-swss#2041) 71751d1 [macsec] Support setting IPG by gearbox_config.json (sonic-net/sonic-swss#2051) 5d5c169 [bulk mode] Fix bulk conflict when in case there are both remove and set operations (sonic-net/sonic-swss#2071) 8bbdbd2 Fix SRV6 NHOP CRM object type (sonic-net/sonic-swss#2072) ef5b35f [vstest] VS test failure fix after fabric port orch PR merge (sonic-net/sonic-swss#1811) 89ea538 Supply the missing ingress/egress port profile list in document (sonic-net/sonic-swss#2064) 8123437 [pfc_detect] fix RedisReply errors (sonic-net/sonic-swss#2040) b38f527 [swss][CRM][MPLS] MPLS CRM Nexthop - switch back to using SAI OBJECT rather than SWITCH OBJECT ae061e5 create debug_shell_enable config to enable debug shell (sonic-net/sonic-swss#2060) 45e446d [cbf] Fix max FC value (sonic-net/sonic-swss#2049) b1b5b29 Initial p4orch pytest code. (sonic-net/sonic-swss#2054) d352d5a Update default route status to state DB (sonic-net/sonic-swss#2009) 24a64d6 Orchagent: Integrate P4Orch (sonic-net/sonic-swss#2029) 15a3b6c Delete the IPv6 link-local Neighbor when ipv6 link-local mode is disabled (sonic-net/sonic-swss#1897) ed783e1 [orchagent] Add trap flow counter support (sonic-net/sonic-swss#1951) e9b05a3 [vnetorch] ECMP for vnet tunnel routes with endpoint health monitor (sonic-net/sonic-swss#1955) bcb7d61 P4Orch: inital add of source (sonic-net/sonic-swss#1997) f6f6f86 [mclaglink] fix acl out ports (sonic-net/sonic-swss#2026) fd887bf [Reclaim buffer] Reclaim unused buffer for dynamic buffer model (sonic-net/sonic-swss#1910) 9258978 [orchagent, cfgmgr] Add response publisher and state recording (sonic-net/sonic-swss#1992) 3d862a7 Fixing subport vs test script for subport under VNET (sonic-net/sonic-swss#2048) fb0a5fd Don't handle buffer pool watermark during warm reboot reconciling (sonic-net/sonic-swss#1987) 16d4bcd Routed subinterface enhancements (sonic-net/sonic-swss#1907) 9639db7 [vstest/subintf] Add vs test to validate sub interface ingress to a vnet (sonic-net/sonic-swss#1642) Signed-off-by: Stephen Sun stephens@nvidia.com

…-net#1992) * Add response publisher , Add APPL STATE DB recording. Co-authored-by: PINS Working Group <sonic-pins-subgroup@googlegroups.com>

bocon13 requested a review from prsunny as a code owner October 28, 2021 05:23

bocon13 mentioned this pull request Oct 28, 2021

PINS Upstream Tracking for MVP sonic-net/SONiC#841

Open

donNewtonAlpha force-pushed the pr2 branch from 60885bd to 0ec5b09 Compare October 30, 2021 02:22

mint570 reviewed Nov 2, 2021

View reviewed changes

orchagent/vrforch.cpp Outdated Show resolved Hide resolved

orchagent/vrforch.cpp Outdated Show resolved Hide resolved

orchagent/orch.cpp Outdated Show resolved Hide resolved

mint570 reviewed Nov 2, 2021

View reviewed changes

orchagent/orch.cpp Outdated Show resolved Hide resolved

prsunny reviewed Nov 3, 2021

View reviewed changes

prsunny reviewed Nov 5, 2021

View reviewed changes

cfgmgr/buffermgrd.cpp Outdated Show resolved Hide resolved

donNewtonAlpha force-pushed the pr2 branch from a8a491a to 66a942e Compare November 13, 2021 01:17

StephenWangGoogle reviewed Nov 17, 2021

View reviewed changes

orchagent/orch.cpp Show resolved Hide resolved

StephenWangGoogle reviewed Nov 17, 2021

View reviewed changes

orchagent/orch.cpp Outdated Show resolved Hide resolved

mint570 reviewed Nov 17, 2021

View reviewed changes

orchagent/orch.cpp Show resolved Hide resolved

orchagent/orch.cpp Outdated Show resolved Hide resolved

StephenWangGoogle previously approved these changes Nov 18, 2021

View reviewed changes

donNewtonAlpha dismissed StephenWangGoogle’s stale review via 789f9e7 November 19, 2021 21:59

prsunny previously approved these changes Nov 19, 2021

View reviewed changes

bocon13 commented Nov 23, 2021

View reviewed changes

cfgmgr/Makefile.am Outdated Show resolved Hide resolved

mint570 force-pushed the pr2 branch from 60a20c0 to 5369767 Compare November 25, 2021 00:09

mint570 force-pushed the pr2 branch 2 times, most recently from 9b8a1b0 to 071a06b Compare November 25, 2021 22:08

prsunny approved these changes Nov 26, 2021

View reviewed changes

prsunny merged commit 9258978 into sonic-net:master Nov 26, 2021

stephenxs mentioned this pull request Jan 6, 2022

[sonic-swss][202111] Submodule update (#9397) sonic-net/sonic-buildimage#9685

Merged

5 tasks

mint570 deleted the pr2 branch July 22, 2022 16:39

liuh-80 mentioned this pull request Aug 29, 2024

Fix port up/bfd sessions bringup notification delay issue. #3269

Merged

10 tasks

Conversation

bocon13 commented Oct 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bocon13 commented Oct 28, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lgtm-com bot commented Nov 2, 2021

Uh oh!

lgtm-com bot commented Nov 2, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mint570 commented Nov 5, 2021

Uh oh!

Uh oh!

donNewtonAlpha commented Nov 13, 2021

Uh oh!

Uh oh!

Uh oh!

donNewtonAlpha commented Nov 17, 2021 via email

Uh oh!

Uh oh!

Uh oh!

StephenWangGoogle commented Nov 18, 2021

Uh oh!

prsunny commented Nov 19, 2021

Uh oh!

donNewtonAlpha commented Nov 19, 2021

Uh oh!

prsunny commented Nov 22, 2021

Uh oh!

azure-pipelines bot commented Nov 22, 2021

Uh oh!

prsunny commented Nov 22, 2021

Uh oh!

bocon13 commented Nov 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prsunny commented Nov 23, 2021

Uh oh!

prsunny commented Nov 23, 2021

Uh oh!

azure-pipelines bot commented Nov 23, 2021

Uh oh!

Uh oh!

bocon13 commented Nov 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bocon13 commented Nov 24, 2021

Uh oh!

mssonicbld commented Nov 24, 2021

Uh oh!

azure-pipelines bot commented Nov 24, 2021

Uh oh!

bocon13 commented Nov 24, 2021

Uh oh!

mssonicbld commented Nov 24, 2021

Uh oh!

azure-pipelines bot commented Nov 24, 2021

Uh oh!

prsunny commented Nov 25, 2021

Uh oh!

azure-pipelines bot commented Nov 25, 2021

Uh oh!

mint570 commented Nov 26, 2021

Uh oh!

bocon13 commented Nov 26, 2021

Uh oh!

Reviewers

Assignees

bocon13 commented Oct 28, 2021 •

edited

Loading

bocon13 commented Nov 23, 2021 •

edited

Loading

bocon13 commented Nov 23, 2021 •

edited

Loading