Skip to content

Conversation

@tgwizard
Copy link

@tgwizard tgwizard commented May 30, 2020

This PR adds a new command, which is a mix of MGET and HGET. It allows you to query for fields from multiple hashes in one command:

MHGET key field [key field ...]

I find this useful in a few cases. The main use-case I have currently is that we store user experiment assignments in Redis, as one hash per experiment, one field per user, value is the assignment group. By having the hash be per experiment, we can easily drop the hash once the experiment is over (we actually HSCAN through the hash to delete fields in batches before dropping the key, to not pay the O(N) cost in a single query). The drawback of this structure is that we need to execute several HGET commands to get all experiment assignments for a specific user. (We have usually <20 active experiments, but 10s of millions of active users).

I thought I'd propose a specific command for this use-case. I apologize for not going through the flow recommended in the contributing guidelines, of posting to the mailing list first. I wanted to build something, code in C again after a long break. If this isn't deemed a fit in Redis I don't consider the hour I spent coding and learning the codebase wasted. If you think it's worth posting about this on the mailing list to solicit more feedback I'm more than happy to do so.

I think MHGET is slightly unfortunate as a command name. All other hash commands start with H. Perhaps HMHGET could work?

@gittyup2018
Copy link

gittyup2018 commented Jun 1, 2020

Thanks for coding this, I think this is useful and should be merged as it also solves my use case
as well.

To retrieve different key/hash combinations I currently use 3 operations to retrieve the results (sadd,sort,del) to temporary store values in a set, with this I only need to use 1 command and while HMGET is great it is only really useful on one key, and this new command allows access to every hash and every hash value.

@bantmen
Copy link

bantmen commented Jan 25, 2021

Pinging as this would be a significant optimization for Shopify's experiments infra -- is there appetite to merge the change from the maintainer side @oranagra?

@oranagra
Copy link
Member

Hi all.
Thanks for suggesting and codding this.
Generally i think we want to avoid adding such multi-key commands, unless there's a real good reason for them.

For example: SUNIONSTORE reads from one key and stores into another. such a thing would be impossible to do in a transaction, so it can either be done on the client side (wasting a round-trip), or with a Lua script (a bit more complicated).
Also, doing such a thing inside redis, improves it's efficiency.

However, for this request, i think it's trivial to issue multiple HGET commands either using pipeline (if atomicity isn't required), or with MULTI-EXEC transaction.
Either way, you'll get all the results and there's no real big disadvantage performance wise.
Please let me know why this isn't a good solution for you.

We can consider adding some convenience commands (such as GET+DEL) as one command to save users the minor trouble of using a transaction if they're very commonly used. but i think doing that for multi-key commands is were we draw the line (unless there's a real good reason).

ping @itamarhaber for additional feeback.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@tgwizard tgwizard closed this Mar 25, 2024
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.

5 participants