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

ndb_redis: add pipeline suppport for REDIS Module #1079

Closed
wants to merge 4 commits into from

Conversation

claudiupb
Copy link

Hello,

I have added pipeline support to the ndb_redis_module. Two functions are added:
-redis_pipe_cmd which appends a command to be pipelined(syntax is identical with redis_cmd)
-redis_execute which sends one message with all existing pipelined commands and handles the replies

To send multiple commands in a single message, the user must call redis_pipe_cmd for each command, and then redis_execute to send all messages.

This enhancement is not compatible with the recently added REDIS cluster support, so if that is enabled the new functions don't do anything but log an error.

Thanks,
Claudiu Boriga

@miconda
Copy link
Member

miconda commented Apr 24, 2017

Pinging Carsten (@ngvoice) -- is it needed to wait for fixing the changes done by using shm memory or this PR can be merged?

@miconda
Copy link
Member

miconda commented Apr 24, 2017

@claudiupb - can you resolve the conflicts so this PR can be merged? The module was in a broken state at the time you did the PR, which needed to be fixed, sorry for the extra work.

@claudiupb
Copy link
Author

Ok, I will resolve the conflicts

@claudiupb
Copy link
Author

I resolved the conflicts in the PR.

@miconda
Copy link
Member

miconda commented Apr 24, 2017

@claudiupb - the travis-ci is reporting compilation errors with the patches, see the details from travis ci checks (next comment).

@claudiupb
Copy link
Author

claudiupb commented Apr 24, 2017

Hello, there was a problem because _redisc_srv_list type was modified. The problem if fixed now

@miconda
Copy link
Member

miconda commented Apr 24, 2017

Thanks, I merged it manually to get rid of the extra commit messages that didn't follow the required format -- commit ff8cfa6 .

@miconda miconda closed this Apr 24, 2017
@claudiupb claudiupb deleted the redis_pipeline branch May 10, 2017 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants