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

add fix for STATIC_ROUTE_EXPIRY_TIME key not exist #12769

Merged
merged 1 commit into from
Nov 20, 2022
Merged

add fix for STATIC_ROUTE_EXPIRY_TIME key not exist #12769

merged 1 commit into from
Nov 20, 2022

Conversation

jcaiMR
Copy link
Contributor

@jcaiMR jcaiMR commented Nov 19, 2022

Why I did it

when key STATIC_ROUTE_EXPIRY_TIME is not exist in APPL_DB, self.db.get(self.db.APPL_DB, "STATIC_ROUTE_EXPIRY_TIME", "time") will rise runtime error, and python thread silent exist.

How I did it

Add key check before access it's value

How to verify it

Try it on DUT

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

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

Description for the changelog

Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.

Link to config_db schema for YANG module changes

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

Copy link
Contributor

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

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

Approved with minor suggestion.

@@ -16,6 +16,9 @@ def __init__(self):

def set_timer(self):
""" Check for custom route expiry time in STATIC_ROUTE:EXPIRY_TIME """
keys = self.db.keys(self.db.APPL_DB, "STATIC_ROUTE_EXPIRY_TIME")
if len(keys) == 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a log here to prove this thread is still alive? Maybe info/debug level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When key not exists. there will be log periodicly printed like :
WARNING bgp#bgpcfgd: :- operator(): DB '{APPL_DB}' is empty with pattern 'STATIC_ROUTE_EXPIRY_TIME'!

When key exist there will be log periodicly printed like:
INFO bgp#bgpcfgd: Static route expiry set to 160s

@qiluo-msft
Copy link
Collaborator

@liuh-80 to check whether this is swss-common bug. If yes, we need to fix in swss-common and revert this.

@liuh-80
Copy link
Contributor

liuh-80 commented Nov 24, 2022

Ack, I will check if this is a swsscommon issue.

@liuh-80
Copy link
Contributor

liuh-80 commented Nov 24, 2022

Can'r reproduce this issue with following code, will discussion offline:

from swsscommon import swsscommon
db = swsscommon.SonicV2Connector()
db.connect(db.APPL_DB)
db.keys(db.APPL_DB, "STATIC_ROUTE_EXPIRY_TIME")
[]
db.get(db.APPL_DB, "STATIC_ROUTE_EXPIRY_TIME", "time")

richardyu-ms pushed a commit to richardyu-ms/sonic-buildimage that referenced this pull request Nov 25, 2022
fix thread exist when key not exists

Public: sonic-net#12769
yxieca pushed a commit that referenced this pull request Nov 28, 2022
StormLiangMS pushed a commit to StormLiangMS/sonic-buildimage that referenced this pull request Dec 8, 2022
StormLiangMS pushed a commit that referenced this pull request Dec 10, 2022
yaqiangz pushed a commit to yaqiangz/sonic-buildimage that referenced this pull request Feb 14, 2023
Resolved merge conflicts in the following files manually

device/arista/x86_64-arista_7050_qx32/Arista-7050-QX32-Flex/buffers_defaults_t0.j2
device/arista/x86_64-arista_7050_qx32/Arista-7050-QX32-Flex/buffers_defaults_t1.j2
device/arista/x86_64-arista_7050_qx32/Arista-7050-QX32/buffers_defaults_t0.j2
device/arista/x86_64-arista_7050_qx32/Arista-7050-QX32/buffers_defaults_t1.j2
device/arista/x86_64-arista_7050_qx32s/Arista-7050QX-32S-S4Q31/buffers_defaults_t0.j2
device/arista/x86_64-arista_7050_qx32s/Arista-7050QX-32S-S4Q31/buffers_defaults_t1.j2
device/dell/x86_64-dell_s6000_s1220-r0/Force10-S6000/buffers_defaults_t0.j2
device/dell/x86_64-dell_s6000_s1220-r0/Force10-S6000/buffers_defaults_t1.j2
src/sonic-config-engine/tests/test_j2files.py

Related work items: sonic-net#11864, sonic-net#12585, sonic-net#12607, sonic-net#12619, sonic-net#12626, sonic-net#12707, sonic-net#12714, sonic-net#12729, sonic-net#12733, sonic-net#12736, sonic-net#12750, sonic-net#12751, sonic-net#12755, sonic-net#12757, sonic-net#12758, sonic-net#12760, sonic-net#12761, sonic-net#12769, sonic-net#12783
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