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

Removed archived evcs from self.circuits #433

Merged
merged 4 commits into from
Feb 16, 2024
Merged

Conversation

Alopalao
Copy link

@Alopalao Alopalao commented Feb 14, 2024

Closes #369

Summary

Removed archived evcs from self.circuits.

Local Tests

Tested link_down and link_up events.
Tried to delete, patch a deleted EVC.
Tried to create an schedule for a deleted EVC.

End-to-End Tests

+ python3 -m pytest tests/ --reruns 2 -r fEr
============================= test session starts ==============================
platform linux -- Python 3.9.2, pytest-7.2.0, pluggy-1.4.0
rootdir: /tests
plugins: timeout-2.1.0, rerunfailures-10.2, anyio-3.6.2
collected 257 items

tests/test_e2e_01_kytos_startup.py RRF.                                  [  0%]
tests/test_e2e_05_topology.py ....................                       [  8%]
tests/test_e2e_10_mef_eline.py ..........ss.....x.....x................  [ 24%]
tests/test_e2e_11_mef_eline.py ......                                    [ 26%]
tests/test_e2e_12_mef_eline.py .....Xx.                                  [ 29%]
tests/test_e2e_13_mef_eline.py ....Xs.s.....Xs.s.XXxX.xxxx..X........... [ 45%]
.                                                                        [ 45%]
tests/test_e2e_14_mef_eline.py x                                         [ 46%]
tests/test_e2e_15_mef_eline.py .....                                     [ 48%]
tests/test_e2e_16_mef_eline.py .                                         [ 48%]
tests/test_e2e_20_flow_manager.py .......R..............                 [ 56%]
tests/test_e2e_21_flow_manager.py ...                                    [ 57%]
tests/test_e2e_22_flow_manager.py ...............                        [ 63%]
tests/test_e2e_23_flow_manager.py ..............                         [ 69%]
tests/test_e2e_30_of_lldp.py ....                                        [ 70%]
tests/test_e2e_31_of_lldp.py ...                                         [ 71%]
tests/test_e2e_32_of_lldp.py ...                                         [ 73%]
tests/test_e2e_40_sdntrace.py ..............                             [ 78%]
tests/test_e2e_41_kytos_auth.py ........                                 [ 81%]
tests/test_e2e_42_sdntrace.py ..                                         [ 82%]
tests/test_e2e_50_maintenance.py ........................                [ 91%]
tests/test_e2e_60_of_multi_table.py .....                                [ 93%]
tests/test_e2e_70_kytos_stats.py ........                                [ 96%]
tests/test_e2e_80_pathfinder.py ss......                                 [100%]

The failures are popped because of an outdated kytos-docker without the removal of flow_stats

+ python3 -m pytest tests/test_e2e_01_kytos_startup.py --reruns 2 -r fEr
============================= test session starts ==============================
platform linux -- Python 3.9.2, pytest-7.2.0, pluggy-1.4.0
rootdir: /tests
plugins: timeout-2.1.0, rerunfailures-10.2, anyio-3.6.2
collected 2 items

tests/test_e2e_01_kytos_startup.py ..                                    [100%]

@Alopalao Alopalao requested a review from a team as a code owner February 14, 2024 22:02
models/evc.py Show resolved Hide resolved
models/evc.py Show resolved Hide resolved
tests/unit/test_main.py Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
Copy link
Member

@viniarck viniarck left a comment

Choose a reason for hiding this comment

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

Nicely done @Alopalao, it's almost there, just trivial details remaining. Also, look forward to the e2e test results.

main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
@viniarck viniarck self-requested a review February 15, 2024 18:27
Copy link
Member

@viniarck viniarck left a comment

Choose a reason for hiding this comment

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

I'll leave this pre-approved, check out the last thread that I opened though, there's a minor refactoring and lock that can get removed.

@Alopalao, I'll also recommend that you explore disabling/enabling EVCs and perform a few link flaps low frequency stuff should suffice. Also, when e2e tests are passing and the opened thread on this PR is addressed it can get merged.

@Alopalao
Copy link
Author

Performed the tests with link flaps and disabling/enabling EVCs. No errors and no warning others than the recently implemented:
2024-02-15 17:23:08,967 - WARNING [kytos.core.queue_monitor] (MainThread) threadpool_sb, counted: 5, min/avg/max size: 3804/3806.8/3818, first at: 2024-02-15 22:23:04.963305+00:00, last at: 2024-02-15 22:23:08.967486+00:00, delta secs: 5, min_hits: 5, min_size_threshold: 384

@viniarck
Copy link
Member

Performed the tests with link flaps and disabling/enabling EVCs. No errors and no warning others than the recently implemented: 2024-02-15 17:23:08,967 - WARNING [kytos.core.queue_monitor] (MainThread) threadpool_sb, counted: 5, min/avg/max size: 3804/3806.8/3818, first at: 2024-02-15 22:23:04.963305+00:00, last at: 2024-02-15 22:23:08.967486+00:00, delta secs: 5, min_hits: 5, min_size_threshold: 384

@Alopalao, 3800 more or less messages on avg over 5 secs queued on the threadpool_sb indeed it shows that it was not processing fast enough. Can you verify again if any of of_core's handlers like kytos/of_core.* where taking too long on APM than usual? Maybe you've hit hit a generalized slow down again like you've done before. Either way, it's a warning, but for sure the default queue monitor params should only trigger in overloaded situations, which indeed looks like it was here for your environment. Try also checking if you might need to bump vCPUs or a bit of RAM on your VM.

I also ran here 50 flaps per iteration waiting for 10 secs with 100 EVCs and no warnings on my environment at least:

python flap.py 
link_flap test iteration 0
         waiting for 10 secs before next iteration
link_flap test iteration 1
         waiting for 10 secs before next iteration
link_flap test iteration 2
         waiting for 10 secs before next iteration
link_flap test iteration 3
         waiting for 10 secs before next iteration
link_flap test iteration 4
         waiting for 10 secs before next iteration

If you still keep seeing this, ping me we can pair on it and analyze your environment, let's see if we might want increase a bit more the default params of queue monitors. From what I'm seeing here it detected correctly. Other than that, I'll go ahead and merge this PR, but let's keep this other discussion here going in the background. OK?

@viniarck viniarck merged commit 88e65f5 into master Feb 16, 2024
2 checks passed
@viniarck viniarck deleted the bug/archived_circuits branch February 16, 2024 13:58
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.

bug: archived EVC is allowing its metadata to be updated
2 participants