Improve ConsumerStateTable: del after consumed, and let consumer writes the final table #204

qiluo-msft · 2018-05-30T02:20:48Z

Original implementation will have 2 issues:

The consumer side will accumulate all the state into redis hashes, and each time a new field/value, the consumer will pop it, and return all the filed/value accumulated.
The producer side modify the redis hashes directly. To behavior the same way as Producer/ConsumerTable, let the consumer side modify the redis hashes.

kcudnik · 2018-05-30T07:25:43Z

whats the motivation behind this pr? #Resolved

qiluo-msft · 2018-05-30T16:44:35Z

@kcudnik I modified the first comment to answer your question. #Closed

jipanyang · 2018-05-30T18:21:06Z

common/consumer_state_table_pops.lua

+   for f, v in pairs(fieldvalues) do
+      redis.call('HSET', tablename..key, f, v)
+   end
+   redis.call('DEL', stateprefix..tablename..key)


With the extra HSET and DEL processing, it looks the number of redis operations has been doubled. Is the performance impact a concern here?

Is it possible to compress all the data in sadd argument and process them after SPOP? #Resolved

Since the requirement changes, performance is not the first concern here. I am open to any optimization suggestion.

Regarding compress, the SADD is used to keep keys, so we have chance to merge the operations on the same key. Compressing key/filed/value into SADD doesn't help.

In reply to: 191872386 [](ancestors = 191872386)

jipanyang · 2018-07-07T02:26:41Z

Please ignore this message. I found it was caused by outdated swss-common library on host test environment. The library inside vs docker is up to date but pytest uses library on host *

It looks vs test_FDBAddedAfterMemberCreated will fail with this change?

sudo pytest -v --dvsname=vs --notempview -k 'test_FDBAddedAfterMemberCreated'
============================================================================== test session starts ==============================================================================
platform linux2 -- Python 2.7.15rc1, pytest-3.3.0, py-1.5.3, pluggy-0.6.0 -- /usr/bin/python
cachedir: .cache
rootdir: /home/jipan/warm_reboot/sonic-buildimage/src/sonic-swss/tests, inifile:
collected 47 items                                                                                                                                                              

test_fdb.py::test_FDBAddedAfterMemberCreated FAILED                                                                                                                       [100%]

=================================================================================== FAILURES ====================================================================================
________________________________________________________________________ test_FDBAddedAfterMemberCreated ________________________________________________________________________

dvs = <conftest.DockerVirtualSwitch object at 0x7f30b822bb50>

    def test_FDBAddedAfterMemberCreated(dvs):
        appl_db = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0)
        asic_db = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)
        conf_db = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0)
    
        # create a FDB entry in Application DB
        create_entry_pst(
            appl_db,
            "FDB_TABLE", "Vlan2:52-54-00-25-06-E9",
            [
                ("port", "Ethernet0"),
                ("type", "dynamic"),
            ]
        )
    
        # check that the FDB entry wasn't inserted into ASIC DB
        assert how_many_entries_exist(asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY") == 0, "The fdb entry leaked to ASIC"
    
        # create vlan
        create_entry_tbl(
            conf_db,
            "VLAN", "Vlan2",
            [
                ("vlanid", "2"),
            ]
        )
    
        # create vlan member entry in application db
        create_entry_tbl(
            conf_db,
            "VLAN_MEMBER", "Vlan2|Ethernet0",
             [
                ("tagging_mode", "untagged"),
             ]
        )
    
        # check that the vlan information was propagated
        assert how_many_entries_exist(asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_VLAN") == 2, "The 2 vlan wasn't created"
        assert how_many_entries_exist(asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT") == 1, "The bridge port wasn't created"
        assert how_many_entries_exist(asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_VLAN_MEMBER") == 1, "The vlan member wasn't added"
    
        # Get mapping between interface name and its bridge port_id
        iface_2_bridge_port_id = get_map_iface_bridge_port_id(asic_db, dvs)
    
        # check that the FDB entry was inserted into ASIC DB
>       assert how_many_entries_exist(asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY") == 1, "The fdb entry wasn't inserted to ASIC"
E       AssertionError: The fdb entry wasn't inserted to ASIC
E       assert 0 == 1
E        +  where 0 = how_many_entries_exist(<swsscommon.swsscommon.DBConnector; proxy of <Swig Object of type 'swss::DBConnector *' at 0x7f30b8295ea0> >, 'ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY')

test_fdb.py:123: AssertionError
------------------------------------------------------------------------------ Captured log setup -------------------------------------------------------------------------------
auth.py                    234 DEBUG    Trying paths: ['/home/jipan/.docker/config.json', '/home/jipan/.dockercfg']
auth.py                    241 DEBUG    No config file found
connectionpool.py          396 DEBUG    http://localhost:None "GET /v1.30/containers/json?all=0&limit=-1&trunc_cmd=0&size=0 HTTP/1.1" 200 1501
connectionpool.py          396 DEBUG    http://localhost:None "GET /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/json HTTP/1.1" 200 None
connectionpool.py          396 DEBUG    http://localhost:None "GET /v1.30/containers/9652a812b62dc6da224da2eb10afb9c133da50c1cc84d88cc7a67c4492987521/json HTTP/1.1" 200 None
connectionpool.py          396 DEBUG    http://localhost:None "GET /v1.30/containers/json?all=0&limit=-1&trunc_cmd=0&size=0 HTTP/1.1" 200 1501
connectionpool.py          396 DEBUG    http://localhost:None "GET /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/json HTTP/1.1" 200 None
connectionpool.py          396 DEBUG    http://localhost:None "GET /v1.30/containers/9652a812b62dc6da224da2eb10afb9c133da50c1cc84d88cc7a67c4492987521/json HTTP/1.1" 200 None
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/restart?t=10 HTTP/1.1" 204 0
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/exec HTTP/1.1" 201 74
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/exec/e6b68214102ba58e961059ff73b8e97025ad20468b932ec2d758021dc5e4be36/start HTTP/1.1" 101 0
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/exec HTTP/1.1" 201 74
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/exec/60e69f5817d3254d18ffbc6dbfbd52c22eec478919092a17f99a16a2cc0253ef/start HTTP/1.1" 101 0
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/exec HTTP/1.1" 201 74
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/exec/139a4db9cdaf2f28148fc17b6751a8d567c8dbe27e124ecdfe4eb5aa5b136869/start HTTP/1.1" 101 0
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/exec HTTP/1.1" 201 74
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/exec/fb58dd2e6afd48d036454e64db4d0e57e34e7d07614eabdc8de24db744a6e9a8/start HTTP/1.1" 101 0
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/exec HTTP/1.1" 201 74
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/exec/4f09fb8e00b3bf05d76757baf8fc4728f2bdf3a84c5c68a57449d9ae910265c4/start HTTP/1.1" 101 0
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/exec HTTP/1.1" 201 74
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/exec/56aa247091281d554d3475b2b7f4175f686a63d7046ec430dbe3299fbbb7d83f/start HTTP/1.1" 101 0
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/exec HTTP/1.1" 201 74
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/exec/ef8e578e56e4c90882fb928c1defdb22c9057043f1a4b30c269987f0560ca10d/start HTTP/1.1" 101 0
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/exec HTTP/1.1" 201 74
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/exec/19ee2162af3d5bdd1586617fe391b5d7f3babf1d6b5235276c2bdfda782aceb0/start HTTP/1.1" 101 0
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/exec HTTP/1.1" 201 74
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/exec/98f9d62ea809f081d290c92b240e6bf3f79c5317045630cb714fdd57f3e7ea44/start HTTP/1.1" 101 0
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/exec HTTP/1.1" 201 74
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/exec/5a6b72f5e3b8dbe6f1122fb09f62c284e0399edf3d16ab2ba7cb2099894a634c/start HTTP/1.1" 101 0
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/exec HTTP/1.1" 201 74
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/exec/20e0320ae4551100fc47865049eb5e8922cc98ffa813cbb26b9ff22a0b0676a8/start HTTP/1.1" 101 0
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/exec HTTP/1.1" 201 74
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/exec/4e8d3bcdce068f7b60e66d49b7df04478573b0759416308374336dfb5422ac3c/start HTTP/1.1" 101 0
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/exec HTTP/1.1" 201 74
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/exec/e6c1dc710a8e55d3568a7fe8833955d0bb28544e99d45f01251b6c8baea0b920/start HTTP/1.1" 101 0
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/containers/85b80d419bbaa3ebefc2ce354db2da0724fe4e435c2200ef73b9c86664928b0e/exec HTTP/1.1" 201 74
connectionpool.py          396 DEBUG    http://localhost:None "POST /v1.30/exec/90f58175ff0c76033baca36e7d37782e562b9900c666dd8885bf2b3ca8d10e9c/start HTTP/1.1" 101 0
============================================================================== 46 tests deselected ==============================================================================
=================================================================== 1 failed, 46 deselected in 22.13 seconds ====================================================================

``` #Closed

jipanyang

Though it passed swss common unit test, the data left in DB table is wrong: Sequence number is taken as field while both the original field/value are set as value.

root@80fe1d6fbd69:/sonic/src/sonic-sairedis/tests# redis-cli -n 0
127.0.0.1:6379> hget UT_REDIS_THREAD_0:key 818 1
(error) ERR wrong number of arguments for 'hget' command
127.0.0.1:6379> hget "UT_REDIS_THREAD_0:key 818" 1
"field 0"
127.0.0.1:6379> hget "UT_REDIS_THREAD_0:key 818" 2
""
127.0.0.1:6379> hget "UT_REDIS_THREAD_0:key 818" 3
"field 1"
127.0.0.1:6379> hget "UT_REDIS_THREAD_0:key 818" 4
"value 1"

127.0.0.1:6379> hgetall "UT_REDIS_THREAD_0:key 818"

"1"
"field 0"
"2"
""
"3"
"field 1"
"4"
"value 1"
"5"
"field 2"
"6"
"value 2"
"7"
"field 3"
"8"
"value 3"
"9"
"field 4"
"10"
"value 4"
"11"
"field 5"
"12"
"value 5"
"13"
"field 6"
"14"
"value 6"
"15"
"field 7"
"16"
"value 7"
"17"
"field 8"
"18"
"value 8"
"19"
"field 9"
"20"
"value 9"
"21"
"field 10"
"22"
"value 10"
"23"
"field 11"
"24"
"value 11"
"25"
"field 12"
"26"
"value 12"
"27"
"field 13"
"28"
"value 13"
"29"
"field 14"
"30"
"value 14"
"31"
"field 15"
"32"
"value 15"
"33"
"field 16"
"34"
"value 16"
"35"
"field 17"
"36"
"value 17"
"37"
"field 18"
"38"
"value 18"
"39"
"field 19"
"40"
"value 19"
"41"
"field 20"
"42"
"value 20"
"43"
"field 21"
"44"
"value 21"
"45"
"field 22"
"46"
"value 22"
"47"
"field 23"
"48"
"value 23"
"49"
"field 24"
"50"
"value 24"
"51"
"field 25"
"52"
"value 25"
"53"
"field 26"
"54"
"value 26"
"55"
"field 27"
"56"
"value 27"
127.0.0.1:6379>

Signed-off-by: Qi Luo <[email protected]>

jipanyang · 2018-07-15T04:55:14Z

Correct data written to DB table after the fieldvalue set fix. thanks!

qiluo-msft requested review from pavel-shirshov, kcudnik, lguohan and stcheng May 30, 2018 02:20

jipanyang reviewed May 30, 2018

View reviewed changes

jipanyang reviewed Jul 11, 2018

View reviewed changes

qiluo-msft added 4 commits July 14, 2018 00:34

Add test assertion on state queue empty

f23ddd5

Signed-off-by: Qi Luo <[email protected]>

Improve ConsumerStateTable: del after consumed

5dcd3f0

Signed-off-by: Qi Luo <[email protected]>

Fix Producer/ConsumerStateTable: let consumer writes the final table

48b51a6

Signed-off-by: Qi Luo <[email protected]>

Fix fieldvalues loop

b073815

Signed-off-by: Qi Luo <[email protected]>

qiluo-msft force-pushed the qiluo/statequeuefix branch from 552152d to b073815 Compare July 14, 2018 01:37

Fix DEL, add tests

2d9759f

Signed-off-by: Qi Luo <[email protected]>

jipanyang approved these changes Jul 27, 2018

View reviewed changes

kcudnik approved these changes Jul 27, 2018

View reviewed changes

qiluo-msft merged commit 3edb017 into sonic-net:master Jul 27, 2018

qiluo-msft deleted the qiluo/statequeuefix branch July 27, 2018 22:40

qiluo-msft mentioned this pull request Aug 8, 2018

Improve PortsOrch::doPortTask() sonic-net/sonic-swss#566

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve ConsumerStateTable: del after consumed, and let consumer writes the final table #204

Improve ConsumerStateTable: del after consumed, and let consumer writes the final table #204

qiluo-msft commented May 30, 2018 •

edited

Loading

kcudnik commented May 30, 2018 •

edited by qiluo-msft

Loading

qiluo-msft commented May 30, 2018 •

edited

Loading

jipanyang May 30, 2018 •

edited by qiluo-msft

Loading

qiluo-msft May 30, 2018 •

edited

Loading

jipanyang commented Jul 7, 2018 •

edited by qiluo-msft

Loading

jipanyang left a comment

jipanyang commented Jul 15, 2018

Improve ConsumerStateTable: del after consumed, and let consumer writes the final table #204

Improve ConsumerStateTable: del after consumed, and let consumer writes the final table #204

Conversation

qiluo-msft commented May 30, 2018 • edited Loading

kcudnik commented May 30, 2018 • edited by qiluo-msft Loading

qiluo-msft commented May 30, 2018 • edited Loading

jipanyang May 30, 2018 • edited by qiluo-msft Loading

Choose a reason for hiding this comment

qiluo-msft May 30, 2018 • edited Loading

Choose a reason for hiding this comment

jipanyang commented Jul 7, 2018 • edited by qiluo-msft Loading

jipanyang left a comment

Choose a reason for hiding this comment

jipanyang commented Jul 15, 2018

qiluo-msft commented May 30, 2018 •

edited

Loading

kcudnik commented May 30, 2018 •

edited by qiluo-msft

Loading

qiluo-msft commented May 30, 2018 •

edited

Loading

jipanyang May 30, 2018 •

edited by qiluo-msft

Loading

qiluo-msft May 30, 2018 •

edited

Loading

jipanyang commented Jul 7, 2018 •

edited by qiluo-msft

Loading