Skip to content

Commit e0f394c

Browse files
Fix the issue of ignoring callback calls for removed keys. (sonic-net#789)
**What I did** Fix the issue of ignoring callback calls for removed keys. **Why I did it** ConfigDBConnector.listen method has a caching mechanism (added in sonic-net#587 PR) that preloads the DB state before starting. When the notification about the changed key is received the listen method gets key data from the DB (in all cases when the key was added, updated, or removed) and compares the data with the cache. It fires the callback only if data differ from the cache. Otherwise, the callback is ignored. If the `client.hgetall(key)` is called for the removed key it returns an empty dictionary (`{}`). This can be confused with the data of the key with no attributes. For example: `{"TABLE": {"KEY": {}}}`. So if preloaded data contains a key with no attributes and the listener method receives a notification about the removal of such key the callback is not called. The listener will simply remove the key from the cache without calling the callback. This leads to the situation when the removal of the key is not handled. The solution is to get data for the added or updated keys, and for the removed keys use `None` instead. This will ensure that the comparison with the cache will work as expected. **How I verified it** Compile the package and run the unit test. Unit tests are extended to cover the expected behavior.
1 parent 00db81f commit e0f394c

File tree

2 files changed

+35
-12
lines changed

2 files changed

+35
-12
lines changed

common/configdb.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ class ConfigDBConnector_Native : public SonicV2Connector_Native
120120
## Dynamic typed functions used in python
121121
@staticmethod
122122
def raw_to_typed(raw_data):
123-
if raw_data is None:
123+
if not raw_data or not raw_data.keys():
124124
return None
125125
typed_data = {}
126126
for raw_key in raw_data:

tests/test_redis_ut.py

+34-11
Original file line numberDiff line numberDiff line change
@@ -634,29 +634,40 @@ def thread_coming_entry():
634634
def test_ConfigDBInit():
635635
table_name_1 = 'TEST_TABLE_1'
636636
table_name_2 = 'TEST_TABLE_2'
637+
table_name_3 = 'TEST_TABLE_3'
637638
test_key = 'key1'
638639
test_data = {'field1': 'value1'}
639-
test_data_update = {'field1': 'value2'}
640+
641+
queue = multiprocessing.Queue()
640642

641643
manager = multiprocessing.Manager()
642644
ret_data = manager.dict()
643645

644-
def test_handler(table, key, data, ret):
645-
ret[table] = {key: data}
646+
def test_handler(table, key, data, ret, q=None):
647+
if data is None:
648+
ret[table] = {k: v for k, v in ret[table].items() if k != key}
649+
else:
650+
ret[table] = {key: data}
651+
652+
if q:
653+
q.put(ret[table])
646654

647-
def test_init_handler(data, ret):
655+
def test_init_handler(data, ret, queue):
648656
ret.update(data)
657+
queue.put(ret)
649658

650-
def thread_listen(ret):
659+
def thread_listen(ret, queue):
651660
config_db = ConfigDBConnector()
652661
config_db.connect(wait_for_init=False)
653662

654663
config_db.subscribe(table_name_1, lambda table, key, data: test_handler(table, key, data, ret),
655664
fire_init_data=False)
656665
config_db.subscribe(table_name_2, lambda table, key, data: test_handler(table, key, data, ret),
657666
fire_init_data=True)
667+
config_db.subscribe(table_name_3, lambda table, key, data: test_handler(table, key, data, ret, queue),
668+
fire_init_data=False)
658669

659-
config_db.listen(init_data_handler=lambda data: test_init_handler(data, ret))
670+
config_db.listen(init_data_handler=lambda data: test_init_handler(data, ret, queue))
660671

661672
config_db = ConfigDBConnector()
662673
config_db.connect(wait_for_init=False)
@@ -666,14 +677,26 @@ def thread_listen(ret):
666677
# Init table data
667678
config_db.set_entry(table_name_1, test_key, test_data)
668679
config_db.set_entry(table_name_2, test_key, test_data)
680+
config_db.set_entry(table_name_3, test_key, {})
669681

670-
thread = multiprocessing.Process(target=thread_listen, args=(ret_data,))
682+
thread = multiprocessing.Process(target=thread_listen, args=(ret_data, queue))
671683
thread.start()
672-
time.sleep(5)
673-
thread.terminate()
674684

675-
assert ret_data[table_name_1] == {test_key: test_data}
676-
assert ret_data[table_name_2] == {test_key: test_data}
685+
init_data = queue.get(5)
686+
687+
# Verify that all tables initialized correctly
688+
assert init_data[table_name_1] == {test_key: test_data}
689+
assert init_data[table_name_2] == {test_key: test_data}
690+
assert init_data[table_name_3] == {test_key: {}}
691+
692+
# Remove the entry (with no attributes) from the table.
693+
# Verify that the update is received and a callback is called
694+
config_db.set_entry(table_name_3, test_key, None)
695+
696+
table_3_data = queue.get(5)
697+
assert test_key not in table_3_data
698+
699+
thread.terminate()
677700

678701

679702
def test_DBConnectFailure():

0 commit comments

Comments
 (0)