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

json feature #30

Open
yovanoc opened this issue Jul 7, 2024 · 7 comments
Open

json feature #30

yovanoc opened this issue Jul 7, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@yovanoc
Copy link

yovanoc commented Jul 7, 2024

I think it would be a good idea to add an option to toggle a json mode, to use the json type in redis instead of string

@Julien-R44
Copy link
Owner

Can you explain why?

@yovanoc
Copy link
Author

yovanoc commented Oct 1, 2024

because im using ft.search and json.get tu search and get part of json object stored with bentocache, and I think this shouldn't be to difficult to add the option here directly if it help other people

@wodCZ
Copy link

wodCZ commented Oct 1, 2024

I'll add my 2 cents - de/serialization is known to be expensive, plus it unexpectedly transforms values.

A common use-case is to cache database models. Let's say your article model has createdAt attribute, hydrated by your ORM to Date object. You wrap the fetch to bentocache, and your GraphQL engine starts complaining, because the Date scalar expects Date instance, not string (which is the result of de/serialization).

This is especially confusing when using just L1 inmemory cache, which doesn't need serialization at all. Having an option to provide a no-op serializer, or better, to use best matching native methods at the storage driver level, would (massively, I believe) increase throughput/rps for L1-only scenarios, and reduce integration hiccups, such as the Date-string conversion.

@Julien-R44
Copy link
Owner

Julien-R44 commented Oct 2, 2024

I'll add my 2 cents - de/serialization is known to be expensive, plus it unexpectedly transforms values.

A common use-case is to cache database models. Let's say your article model has createdAt attribute, hydrated by your ORM to Date object. You wrap the fetch to bentocache, and your GraphQL engine starts complaining, because the Date scalar expects Date instance, not string (which is the result of de/serialization).

This is especially confusing when using just L1 inmemory cache, which doesn't need serialization at all. Having an option to provide a no-op serializer, or better, to use best matching native methods at the storage driver level, would (massively, I believe) increase throughput/rps for L1-only scenarios, and reduce integration hiccups, such as the Date-string conversion.

Sorry I think I did'nt really understood what you meant. Because all these problems will still exist if you use RedisJSON

To come back to the main issue: I think the ideal would just be to have a RedisJSON driver rather than modifying the original redis driver. Maybe a community package, or even a PR that add that driver could do the job. Would you be willing to contribute that @yovanoc ? Should be pretty easy to do if your take a look at other drivers : https://github.com/Julien-R44/bentocache/blob/main/packages/bentocache/src/drivers/redis.ts

@Julien-R44 Julien-R44 added the enhancement New feature or request label Oct 2, 2024
@yovanoc
Copy link
Author

yovanoc commented Oct 3, 2024

IORedis doesn't support JSON out of the box, but I created myself a wrapper around it, if I can add that too (only for this new redis json driver), I can try to open a PR !

@wodCZ
Copy link

wodCZ commented Oct 3, 2024

Because all these problems will still exist if you use RedisJSON

@Julien-R44 right, I'm sorry for the confusion and for "hijacking" the issue with a different topic.

@Julien-R44
Copy link
Owner

Julien-R44 commented Oct 3, 2024

Oh okay, no worries. Feel free to open another issue about this issue if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants