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

added abstraction for bulk operations through redis pipeline #82

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src/swsssdk/configdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,42 @@ def set_entry(self, table, key, data):
k = k + '@'
client.hdel(_hash, self.serialize_key(k))

def set_bulk(self, payload):
Copy link
Contributor

@qiluo-msft qiluo-msft Jun 25, 2020

Choose a reason for hiding this comment

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

set_bulk [](start = 8, length = 8)

What are the use cases for the new functions? Application can achieve pipeline by calling get_redis_client().pipeline(). And that solution will be more flexible since you can mix functions into the same pipeline.

Copy link
Contributor

Choose a reason for hiding this comment

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

Qi, Please refer to the below comment from. It was suggested not to use the pipeline in the application.

sonic-net/sonic-utilities#891 (comment)

The use case is also mentioned in the same PR. It is to create large number of VLANs. This also helps when querying and displaying the contents of a filled-up mac address table.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @rajendra-dendukuri sharing the background! As we planed to move away from sonic-py-swsssdk and converge everything to sonic-swss-common, I believe new feature here will be short-lived. We have implemented some pipeline support in sonic-swss-common *Tables classes. Could you check if that fulfill your requirement?

Sorry for the inconvenience to extend code here.


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

"""Write bulk entries to config db.
"""
client = self.redis_clients[self.db_name]
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to use the below API which is Multi-DB compliant.

self.get_redis_client(self.db_name). Modify other occurrences as well.

pipe = client.pipeline()
for (k,v) in payload:
pipe.hmset(k, v)
pipe.execute()

def del_bulk(self, payload):
"""Delete bulk entries from config db.
"""
client = self.redis_clients[self.db_name]
pipe = client.pipeline()
for (k) in payload:
pipe.delete(k)
pipe.execute()

def hdel_bulk(self, payload):
"""Delete bulk entries from config db.
"""
client = self.redis_clients[self.db_name]
pipe = client.pipeline()
for (k,v) in payload:
pipe.hdel(k,v)
pipe.execute()

def getall_bulk(self, payload):
"""hgetall bulk entries from config db.
"""
client = self.redis_clients[self.db_name]
pipe = client.pipeline()
for (k) in payload:
pipe.hgetall(k)
return pipe.execute()

def mod_entry(self, table, key, data):
"""Modify a table entry to config db.
Args:
Expand Down