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

[portsyncd]: Add producer for state database updates #336

Merged
merged 1 commit into from
Oct 25, 2017

Conversation

stcheng
Copy link
Contributor

@stcheng stcheng commented Oct 4, 2017

  • Add m_stateTableProducer to write to the state database once the ports are ready
  • Remove suffix Consumer in some variables since they are not 'consumers'

Signed-off-by: Shu0T1an ChenG [email protected]

@stcheng stcheng requested a review from taoyl-ms October 4, 2017 18:36
@stcheng
Copy link
Contributor Author

stcheng commented Oct 4, 2017

@jipanyanga please check this pull request.

FieldValueTuple tuple("state", "TRUE");
vector<FieldValueTuple> vector;
vector.push_back(tuple);
m_stateTableProducer.set(key, vector);
Copy link
Contributor

Choose a reason for hiding this comment

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

The problem I see here is that it will update stateDB port table with ":" separator which is inconsistent with configDB.
It is a general problem, due to table separator is changed to "|" from ":" by sonic-py-swsssdk, all other C++ swss-common library which has fixed ":" table separator won't be able to work with configDB.

Copy link
Contributor

Choose a reason for hiding this comment

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

we should not use stateTableProducer, because we will not use stateTable Consumer to consume the data as it only supports one consumer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll use Table class for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you update the change with latest Table class and use "|" for port state table in stateDB?

@stcheng stcheng self-assigned this Oct 5, 2017
ProducerStateTable p(&db, APP_PORT_TABLE_NAME);
DBConnector appl_db(APPL_DB, DBConnector::DEFAULT_UNIXSOCKET, 0);
DBConnector state_db(STATE_DB, DBConnector::DEFAULT_UNIXSOCKET, 0);
ProducerStateTable p(&appl_db, APP_PORT_TABLE_NAME);
Copy link
Contributor

Choose a reason for hiding this comment

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

use Table instead of producer state table.

@lguohan
Copy link
Contributor

lguohan commented Oct 25, 2017

Can you check the #344 to add similiar output for this PR?

m_vlanMemberTableProducer(appl_db, APP_VLAN_MEMBER_TABLE_NAME),
m_portTable(appl_db, APP_PORT_TABLE_NAME),
m_vlanMemberTable(appl_db, APP_VLAN_MEMBER_TABLE_NAME),
m_stateTableProducer(state_db, "PORT_TABLE")
Copy link
Contributor

Choose a reason for hiding this comment

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

-> m_statePortTable(state_db, STATE_PORT_TABLE_NAME, CONFIGDB_TABLE_NAME_SEPARATOR)

Copy link
Contributor

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

use table instead of producer table class for state db write.

@stcheng stcheng force-pushed the portsyncd branch 2 times, most recently from 810a4c6 to 97a72a8 Compare October 25, 2017 18:50
- Add m_statePortTable to write to the state database once the ports are ready
- Remove suffix Consumer in some variables since they are not 'consumers'

Signed-off-by: Shu0T1an ChenG <[email protected]>
@stcheng
Copy link
Contributor Author

stcheng commented Oct 25, 2017

updated

Copy link
Contributor

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

can you show the output of the state db after this patch?

@@ -195,6 +196,10 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
if (!g_init && g_portSet.find(key) != g_portSet.end())
{
g_portSet.erase(key);
FieldValueTuple tuple("state", "TRUE");
Copy link
Contributor

Choose a reason for hiding this comment

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

true - ok

@stcheng
Copy link
Contributor Author

stcheng commented Oct 25, 2017

root@str-s6000-acs-7:/home/admin# redis-cli         
127.0.0.1:6379> select 6                            
OK                                                  
127.0.0.1:6379[6]> keys *                           
 1) "PORT_TABLE|Ethernet56"                         
 2) "PORT_TABLE|Ethernet96"                         
 3) "PORT_TABLE|Ethernet124"                        
 4) "PORT_TABLE|Ethernet24"                         
 5) "PORT_TABLE|Ethernet68"                         
 6) "PORT_TABLE|Ethernet0"                          
 7) "PORT_TABLE|Ethernet120"                        
 8) "PORT_TABLE|Ethernet112"                        
 9) "PORT_TABLE|Ethernet8"                          
10) "PORT_TABLE|Ethernet60"                         
11) "PORT_TABLE|Ethernet116"                        
12) "PORT_TABLE|Ethernet44"                         
13) "PORT_TABLE|Ethernet76"                         
14) "PORT_TABLE|Ethernet36"                         
15) "PORT_TABLE|Ethernet84"                         
16) "PORT_TABLE|Ethernet72"                         
17) "PORT_TABLE|Ethernet20"                         
18) "PORT_TABLE|Ethernet100"                        
19) "PORT_TABLE|Ethernet108"                        
20) "PORT_TABLE|Ethernet52"                         
21) "PORT_TABLE|Ethernet88"                         
22) "PORT_TABLE|Ethernet92"                         
23) "PORT_TABLE|Ethernet12"                         
24) "PORT_TABLE|Ethernet4"                          
25) "PORT_TABLE|Ethernet104"                        
26) "PORT_TABLE|Ethernet28"                         
27) "PORT_TABLE|Ethernet32"                         
28) "PORT_TABLE|Ethernet16"                         
29) "PORT_TABLE|Ethernet64"                         
30) "PORT_TABLE|Ethernet80"                         
31) "PORT_TABLE|Ethernet40"                         
32) "PORT_TABLE|Ethernet48"                         
127.0.0.1:6379[6]> hgetall "PORT_TABLE|Ethernet124" 
1) "state"                                          
2) "ok"                                           

@lguohan lguohan merged commit e7741ca into sonic-net:master Oct 25, 2017
@stcheng stcheng deleted the portsyncd branch October 25, 2017 21:29
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
…) to construct producer g_asicState (sonic-net#336)

* create RedisPipeline obj by default constructor , which enabled redispipeline with parameter 128
* create producer g_asicState using redisPipeline instance with redispipeline enabled
* with the changes in swss and swss-common, route performance improved by 200~300 routes/sec
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.

3 participants