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

Feature/redis module client side caching #662

Merged
merged 11 commits into from
Nov 9, 2023

Conversation

kopffarben
Copy link
Contributor

PR Details

Exeption Handeling and ClientSideCaching

Description

[RedisModuleClientSideCaching]
Update to StackExchange.Redis 2.7.4
https://stackexchange.github.io/StackExchange.Redis/ReleaseNotes#274
Enables the complete conversion to ClientSideCaching
This is how we get rid of the Custom IsChanged implementation.

All critical methods in the RedisCommandQueue and SubscriberExtension
implement the NodeContext and IVLRuntime.Current,
so that PersistentMessage can also be promoted to VL from Observables
that run concurrently.

Add HelpPatch for "HowTo External Write to Global Channel.vl"

TODO:
Implement SerializerEnum(MsgPack, JSon, None)
in RedisClient/RedisModule,
in Binding "Default, MsgPack, JSon, None" and
in Publish and Subscribe.
This will be done when VL.MessagePack is ready.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

update StackExchange.Redis to Version 2.7.4
this enable ClientSideCaching in transactional way

Change the way PubSub works ... now possible to receive all Changes in transactional way

Checked all Error Messages

Bring EnableClientSideCaching to RedisExtensions
Bring EnableClientSideCaching to RedisCommandQueue on Create and ReEnable on ConnectionRestored

TODO remove old Changes from RedisCommandQueue
Update to StackExchange.Redis 2.7.4
https://stackexchange.github.io/StackExchange.Redis/ReleaseNotes#274
Enables the complete conversion to ClientSideCaching
This is how we get rid of the Custom IsChanged implementation.

All critical methods in the RedisCommandQueue and SubscriberExtension
implement the NodeContext and IVLRuntime.Current,
so that PersistentMessage can also be promoted to VL from Observables
that run concurrently.

Add HelpPatch for "HowTo External Write to Global Channel.vl"

TODO:
Implement SerializerEnum(MsgPack, JSon, None)
in RedisClient/RedisModule,
in Binding "Default, MsgPack, JSon, None" and
in Publish and Subscribe.
This will be done when VL.MessagePack is ready.
@azeno
Copy link
Member

azeno commented Nov 8, 2023

Thanks! Will have a look at it today. In the meantime: Redis has two modes for caching, what was the reasoning for choosing the second one (broadcasting)? Can we peek into your thought process here a little bit?

Then we were also wondering how the caching modes play together with the transactional reading of the keys. If we only read those for which we got an invalidation message then we might run into inconsistent states, no?

@azeno
Copy link
Member

azeno commented Nov 8, 2023

Using the Reference Global Channel help patch, changing the Init mode of "MyInteger" to None or Local does not behave like expected:

First frame:
image

Fourth frame:
image

Shouldn't it stick to zero? Is this a misconception on my part?

Here seems to be a typo:
image
There should be a Z instead of S

@azeno azeno self-requested a review November 8, 2023 12:32

namespace VL.IO.Redis
{

public static class RedisExtensions
{
internal static ConnectionMultiplexer EnableClientSideCaching(this ConnectionMultiplexer ConnectionMultiplexer, string ClientName, out Spread<long> ClientID, out Spread<bool> IsEnabled)
Copy link
Member

Choose a reason for hiding this comment

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

Can we use named tuples here? For example out Spread<(long ClientId, bool IsEnabled)>

IsEnabled = IsEnabledBuilder.ToSpread();
return ConnectionMultiplexer;
}

public static Guid getID(this RedisCommandQueue queue) { return queue.id; }

public static ValueTuple<RedisCommandQueue, TInput> Enqueue<TInput, TOutput>
Copy link
Member

Choose a reason for hiding this comment

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

Again, named tuples would help the readability: public static (RedisCommandQueue commandQueue, TInput input) Enqueue...

cmd.Invoke(tran, input.Item2).ContinueWith(t => new KeyValuePair<Guid, object>(guid, (object)t.Result)),
keys.HasValue ? keys.Value.Invoke(input.Item2).Select(k => new RedisKey(k)) : Enumerable.Empty<RedisKey>()
)
(tran) => cmd.Invoke(tran, input.Item2).ContinueWith(t => new KeyValuePair<Guid, object>(guid, (object)t.Result))
Copy link
Member

Choose a reason for hiding this comment

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

Here it would then for example read input.commandQueue.Cmds.Enqueue

@azeno
Copy link
Member

azeno commented Nov 9, 2023

I'll merge this PR now and create separate issues for the things I mentioned here.

@azeno azeno merged commit 4d79919 into vvvv:main Nov 9, 2023
1 check passed
@azeno azeno mentioned this pull request Nov 9, 2023
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