-
Notifications
You must be signed in to change notification settings - Fork 341
Add REDIS async support #390
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -242,3 +242,45 @@ string Table::stripSpecialSym(const string &key) | |
|
|
||
| return key; | ||
| } | ||
|
|
||
|
|
||
| /******************************************************************************* | ||
| / \ ___ _ _ _ __ ___ | ||
| / _ \ / __| | | | '_ \ / __| | ||
| / ___ \\__ \ |_| | | | | (__ | ||
| /_/ \_\___/\__, |_| |_|\___| | ||
| |___/ | ||
| *******************************************************************************/ | ||
| Table_async::Table_async(DBConnector_async & dbconn_r, const std::string & table_name_r) : | ||
| TableBase(table_name_r, SonicDBConfig::getSeparator(dbconn_r.getDbName())), | ||
| dbconn_rm(dbconn_r) | ||
| { | ||
| } | ||
|
|
||
| Table_async::~Table_async() | ||
| { | ||
| } | ||
|
|
||
| int Table_async::hdel(redisCallbackFn * cb_func_p, void * cb_data_p, const std::string & key_r, const std::string & field_r) | ||
| { | ||
| std::string key = getKeyName(key_r); | ||
| swss::RedisCommand cmd; | ||
| cmd.formatHDEL(key, field_r); | ||
| return dbconn_rm.formatted_command(cb_func_p, cb_data_p, cmd.c_str(), cmd.length()); | ||
| } | ||
|
|
||
| int Table_async::hget(redisCallbackFn * cb_func_p, void * cb_data_p, const std::string & key_r, const std::string & field_r) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Functions in this file fit better in the class
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm just following the same model that was used for |
||
| { | ||
| std::string key = getKeyName(key_r); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Use one blank character |
||
| swss::RedisCommand cmd; | ||
| cmd.formatHGET(key, field_r); | ||
| return dbconn_rm.formatted_command(cb_func_p, cb_data_p, cmd.c_str(), cmd.length()); | ||
| } | ||
|
|
||
| int Table_async::hset(redisCallbackFn * cb_func_p, void * cb_data_p, const std::string & key_r, const std::string & field_r, const std::string & value_r) | ||
| { | ||
| std::string key = getKeyName(key_r); | ||
| swss::RedisCommand cmd; | ||
| cmd.formatHSET(key, field_r, value_r); | ||
| return dbconn_rm.formatted_command(cb_func_p, cb_data_p, cmd.c_str(), cmd.length()); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you provide use cases? We also require unit test cases for new classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use cases are described in the Doxygen text above. Basically, this allows designing processes around an event loop such as GLib, libevent, qt, etc. as described in hiredis documentation: https://github.com/redis/hiredis/tree/master/adapters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks I read the Doxygen text.
I am asking about how do you plan to use the new classes in downstream repoes? Do you already have a PR following?
Require unit test cases in this PR. That also serves as use case.
In reply to: 532125398 [](ancestors = 532125398)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be used by hamd (pull request: sonic-net/sonic-buildimage#5553).
hamd is the Host Account Management Daemon. It is designed as an asynchronous, non-blocking, event-driven process. It uses the GLib event-driven main-loop framework (ref. https://www.freedesktop.org/software/gstreamer-sdk/data/docs/latest/glib/glib-The-Main-Event-Loop.html).
The hiredis library provides GLib bindings. hamd uses the hiredis asynchronous APIs to make non-blocking, asynchronous, requests to the REDIS server. When responses or events are received from the REDIS server, they get dispatched to callback functions by the GLib main-loop.
The following files show how hamd uses the new async APIs:
https://github.com/martin-belanger/sonic-buildimage/blob/master/src/ham/hamd/hamd_redis.h
https://github.com/martin-belanger/sonic-buildimage/blob/master/src/ham/hamd/hamd_redis.cpp
This shows how hamd's GLib Main-loop is created:
https://github.com/martin-belanger/sonic-buildimage/blob/master/src/ham/hamd/hamd_main.cpp