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

[FlexCounter] Fix shutdown syncd hanging problem #1006

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kuanyu99
Copy link

@kuanyu99 kuanyu99 commented Feb 21, 2022

What I did

Add a check before the FlexCounter thread execute the wait_for() funciton.

Why I did it

I encountered syncd shutdown overtime issue during running the test, sonic-mgmt/ansible/roles/test/tasks/restart_syncd.yml
I found it is due to it stuck by FlexCounter during removeAllCounters()

How I verified it

After modification, run the test for over 50 times

Additional information

The following is the syslog when I encountered the failure test.

Feb  1 08:00:13.270840 as5812-54x-3 NOTICE syncd#syncd: :- handleRestartQuery: received COLD switch shutdown event
Feb  1 08:00:18.025017 as5812-54x-3 INFO lldp#lldpd[40]: decode a received frame on Ethernet28
Feb  1 08:00:18.025684 as5812-54x-3 INFO lldp#/supervisord: lldpd 2022-02-01T00:00:18 [INFO/decode] decode a received frame on Ethernet28
Feb  1 08:00:18.085595 as5812-54x-3 INFO lldp#lldpd[40]: decode a received frame on Ethernet29
Feb  1 08:00:18.085595 as5812-54x-3 INFO lldp#/supervisord: lldpd 2022-02-01T00:00:18 [INFO/decode] decode a received frame on Ethernet29
Feb  1 08:00:18.108628 as5812-54x-3 INFO lldp#lldpd[40]: decode a received frame on Ethernet30
Feb  1 08:00:18.108927 as5812-54x-3 INFO lldp#/supervisord: lldpd 2022-02-01T00:00:18 [INFO/decode] decode a received frame on Ethernet30
Feb  1 08:00:18.123554 as5812-54x-3 INFO lldp#lldpd[40]: decode a received frame on Ethernet31
Feb  1 08:00:18.123802 as5812-54x-3 INFO lldp#/supervisord: lldpd 2022-02-01T00:00:18 [INFO/decode] decode a received frame on Ethernet31
Feb  1 08:00:24.115319 as5812-54x-3 INFO ansible-command: Invoked with creates=None executable=None _uses_shell=True strip_empty_ends=True _raw_params=pgrep syncd -a -x removes=None argv=None warn=True chdir=None stdin_add_newline=True stdin=None
Feb  1 08:00:24.166669 as5812-54x-3 NOTICE syncd#syncd: :- removeAllSwitches: Removing all switches

* If the flexCounter try to end the thread while the thread is collecting counter,
  it will miss the m_cvSleep.notify_all()
  After collecting counter, it will still execute m_cvSleep.wait_for() and be idle
  for about 10 seconds.

* Check if the m_runFlexCounterThread is disabled before executing the wait_for() command

m_cvSleep.wait_for(lk, std::chrono::milliseconds(correction));
// If m_runFlexCounterThread is already disabled, do not wait for signal
if (m_runFlexCounterThread)
Copy link
Collaborator

Choose a reason for hiding this comment

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

how this was hanging ? if m_runFlexCounterThread is already false, then wait_for will wait for correction time and in next loop iteration it will break since m_runFlexCounterThread is false on line 2360
so where was hanging problem? was that deadlock? is m_mtxSleep used somewhere else?

Copy link
Collaborator

Choose a reason for hiding this comment

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

also this does not solve problem completely, since ther can be race condition, and m_runFlexCounters can be TRUE on line 2384 but it can be set to FALSE at line 2386 by other thread, so swill wait_for will be executed

Copy link
Author

Choose a reason for hiding this comment

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

It won't hang forever but about 10 seconds for some kinds of counter thread. And the 10 seconds is also a threshold of the test. The root cause just like your second comment. The m_runFlexCounterThread turns from TRUE to FALSE before the thread run to line 2385.

This is not a solution to solve the race condition but just a simple trick to solve 99% hanging 10 seconds situation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

i would prefer to solve this completely to not get back to this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants