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

Display large number of FDB entries fast from state_db and appl_db #1273

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

Conversation

JiangboHe
Copy link
Contributor

This is a copy of pull request #968, we found and fixed it in 201911 branch. It need also be merged to master.

- What I did
An option is provided for showing large number of fdb tables faster.
The time cost to display change from 50 seconds to 2 seconds for 8000 fdb tables or 20 seconds to 1 seconds for 5000 fdb tables.

- How I did it
The key search and judge of fdb tables in asic table is too complex. It will cost 20 seconds for 5000 fdb tables or 50 seconds for 8000 fdb tables. It is not good for users or script running. The option is to show fdb tables from state_db.

- How to verify it
Learn thounds of fdb tables and show them with fdbshow

- Previous command output (if the output of a command-line utility has changed)

- New command output (if the output of a command-line utility has changed)

@JiangboHe
Copy link
Contributor Author

retest this please

@JiangboHe
Copy link
Contributor Author

retest this please

prsunny
prsunny previously approved these changes Dec 2, 2020
@prsunny
Copy link
Contributor

prsunny commented Dec 2, 2020

retest this please

@JiangboHe
Copy link
Contributor Author

Seem it is not the code of this pull request to cause the error result.

Test Result (失败)
test_drop_counters.TestDropCounters.test_deviceCapabilitiesTablePopulated
test_drop_counters.TestDropCounters.test_flexCounterGroupInitialized
test_drop_counters.TestDropCounters.test_createAndRemoveDropCounterBasic
test_drop_counters.TestDropCounters.test_createAndRemoveDropCounterReversed
test_drop_counters.TestDropCounters.test_createCounterWithInvalidCounterType
test_drop_counters.TestDropCounters.test_createCounterWithInvalidDropReason
test_drop_counters.TestDropCounters.test_addReasonToInitializedCounter
test_drop_counters.TestDropCounters.test_removeReasonFromInitializedCounter
test_drop_counters.TestDropCounters.test_removeAllDropReasons
test_drop_counters.TestDropCounters.test_addDropReasonMultipleTimes
test_drop_counters.TestDropCounters.test_addInvalidDropReason
test_drop_counters.TestDropCounters.test_removeDropReasonMultipleTimes
test_drop_counters.TestDropCounters.test_removeNonexistentDropReason
test_drop_counters.TestDropCounters.test_removeInvalidDropReason
test_drop_counters.TestDropCounters.test_createAndDeleteMultipleCounters

11:55:27 /usr/local/lib/python3.8/dist-packages/six.py:702:
11:55:27 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
11:55:27 conftest.py:1503: in dvs
11:55:27 dvs = DockerVirtualSwitch(name, imgname, keeptb, fakeplatform, log_path, max_cpu, forcedvs)
11:55:27 conftest.py:329: in init
11:55:27 server = VirtualServer(self.ctn_sw.name, self.ctn_sw_pid, i)
11:55:27 conftest.py:165: in init
11:55:27 ensure_system(f"ip link add {self.nsname[0:12]} type veth peer name {self.pifname}")

@prsunny
Copy link
Contributor

prsunny commented Dec 4, 2020

retest this please

1 similar comment
@lguohan
Copy link
Contributor

lguohan commented Dec 4, 2020

retest this please

@prsunny
Copy link
Contributor

prsunny commented Dec 9, 2020

@JiangboHe , could you please add unit test to cover this?

@JiangboHe
Copy link
Contributor Author

@prsunny OK, I will add some unit test later.

@liat-grozovik
Copy link
Collaborator

seems like the fix for 201911 was closed and the intention was first to take to master and then cherry pick it.
@prsunny can you please confirm?
@JiangboHe do you have plans to provide unit test so the PR can be merged?

@liat-grozovik liat-grozovik removed the Bug label Dec 29, 2020
@JiangboHe
Copy link
Contributor Author

@liat-grozovik I try to provide the UT next month.

@liat-grozovik
Copy link
Collaborator

@JiangboHe kindly reminder. we would like this fix to be in master and 202012 . will you be able to provide so it can be merged?

@liat-grozovik
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JiangboHe
Copy link
Contributor Author

@liat-grozovik Sorry to reply late. There was some building problem in my enviroment. I am trying to fix it and finish the test case. Thanks

@liat-grozovik
Copy link
Collaborator

@JiangboHe where do we stands with this PR?

@JiangboHe
Copy link
Contributor Author

@liat-grozovik There is some problem with my network in master branch building. I am trying to fix it.

@liat-grozovik
Copy link
Collaborator

liat-grozovik commented Mar 19, 2021 via email

@liat-grozovik
Copy link
Collaborator

@JiangboHe could you please fix conflicts and then we can re run tests to approve the fix?

@moshemos
Copy link

@prsunny can you please review the failure and update the status

@moshemos
Copy link

@JiangboHe can you update with the UT?

@dgsudharsan
Copy link
Collaborator

@JiangboHe Can you please provide an update?

@Yuval-Mellanox
Copy link

@zhangyanzhao Let's triage this issue.

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.

7 participants