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

[BUG] With Parallelism on same key with GetOrSetAsync and Redis as backplane #361

Open
ilioan opened this issue Jan 21, 2025 · 8 comments
Open
Assignees
Labels
bug Something isn't working

Comments

@ilioan
Copy link

ilioan commented Jan 21, 2025

Describe the bug

After updating to version 2.0.0 from 1.4.1

When I am using Tasks.WhenAll (Parellelism)
Inside tasks I have two calls on the same key with GetOrSetAsync with distributed backplane redis.
then I got an error of duplicate key. No cache entry at Redis backplane.

System.InvalidOperationException: Duplicate field-number detected; 1 on: ZiggyCreatures.Caching.Fusion.Internals.Distributed.FusionCacheDistributedEntry1[[System.Int64, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]] at ProtoBuf.Internal.Serializers.TypeSerializer1.Init(Int32[] fieldNumbers, IRuntimeProtoSerializerNode[] serializers, MethodInfo[] baseCtorCallbacks, Boolean isRootType, Boolean useConstructor, Boolean assertKnownType, CallbackSet callbacks, Type constructType, MethodInfo factory, SerializerFeatures features) in //src/protobuf-net/Internal/Serializers/TypeSerializer.cs:line 204
at ProtoBuf.Internal.Serializers.TypeSerializer.Create(Type forType, Int32[] fieldNumbers, IRuntimeProtoSerializerNode[] serializers, MethodInfo[] baseCtorCallbacks, Boolean isRootType, Boolean useConstructor, Boolean assertKnownType, CallbackSet callbacks, Type constructType, MethodInfo factory, Type rootType, SerializerFeatures features) in /
/src/protobuf-net/Internal/Serializers/TypeSerializer.cs:line 21
at ProtoBuf.Meta.MetaType.BuildSerializer() in //src/protobuf-net/Meta/MetaType.cs:line 625
at ProtoBuf.Meta.MetaType.get_Serializer() in /
/src/protobuf-net/Meta/MetaType.cs:line 465
at ProtoBuf.Meta.RuntimeTypeModel.g__GetServicesImpl|88_0(RuntimeTypeModel model, Type type, CompatibilityLevel ambient) in //src/protobuf-net/Meta/RuntimeTypeModel.cs:line 1033
at ProtoBuf.Meta.RuntimeTypeModel.GetServicesSlow(Type type, CompatibilityLevel ambient) in /
/src/protobuf-net/Meta/RuntimeTypeModel.cs:line 999
at ProtoBuf.Meta.RuntimeTypeModel.GetSerializerT in //src/protobuf-net/Meta/RuntimeTypeModel.cs:line 962
at ProtoBuf.Meta.TypeModel.TryGetSerializer[T](TypeModel model) in /
/src/protobuf-net.Core/Meta/TypeModel.cs:line 1486
at ProtoBuf.Meta.TypeModel.SerializeImpl[T](State& state, T value) in //src/protobuf-net.Core/Meta/TypeModel.cs:line 377
at ProtoBuf.Meta.TypeModel.Serialize[T](Stream dest, T value, Object userState) in /
/src/protobuf-net.Core/Meta/TypeModel.cs:line 328
at ZiggyCreatures.Caching.Fusion.Serialization.ProtoBufNet.FusionCacheProtoBufNetSerializer.Serialize[T](T obj) in //src/ZiggyCreatures.FusionCache.Serialization.ProtoBufNet/FusionCacheProtoBufNetSerializer.cs:line 107
at ZiggyCreatures.Caching.Fusion.Serialization.ProtoBufNet.FusionCacheProtoBufNetSerializer.SerializeAsync[T](T obj, CancellationToken token) in /
/src/ZiggyCreatures.FusionCache.Serialization.ProtoBufNet/FusionCacheProtoBufNetSerializer.cs:line 125
at ZiggyCreatures.Caching.Fusion.Internals.Distributed.DistributedCacheAccessor.SetEntryAsync[TValue](String operationId, String key, IFusionCacheEntry entry, FusionCacheEntryOptions options, Boolean isBackground, CancellationToken token) in /_/src/ZiggyCreatures.FusionCache/Internals/Distributed/DistributedCacheAccessor_Async.cs:line 85

To Reproduce

Here's a MRE (Minimal Reproducible Example) of the issue:

  • code sample block
  • or gist
  • or a repo
  • etc

Expected behavior

At previous version it was working perfect. At Redis cache the key was added normally.
Now seems that the key was added at memory but not on Redis backplane.

Versions

I've encountered this issue on:

  • FusionCache version
  • .NET version
  • OS version
  • others (eg: if applicable, the Redis/Memcached/etc version and if onprem/cloud/etc)

Screenshots

If applicable, add screenshots to help explain your problem.

Additional context

Add any other context about the problem here.

@jodydonetti
Copy link
Collaborator

Hi @ilioan , that's strange and seems like a protobuf-net specific thing.
I'll look into it and will let you know, thanks for reporting.

@jodydonetti
Copy link
Collaborator

ps: would you be able to create an MRE?

@ilioan
Copy link
Author

ilioan commented Jan 21, 2025

I ll try but I need few days to prepare it.

@jodydonetti
Copy link
Collaborator

Meanwhile I found this.

Will try to reproduce it locally.

@jodydonetti
Copy link
Collaborator

UPDATE: I think I've found the cause, it's probably a missing check in (what should've been) a classic double-checked lock, here. Will make some tests before/after and update you.

@ilioan
Copy link
Author

ilioan commented Jan 21, 2025

It was easier for me to change the Serializer to FusionCacheNewtonsoftJsonSerializer and test.
It is working as expected.
So the issue is connected with protobuf serialization

@jodydonetti jodydonetti self-assigned this Jan 21, 2025
@jodydonetti jodydonetti added the bug Something isn't working label Jan 21, 2025
@ilioan
Copy link
Author

ilioan commented Jan 22, 2025

` public async Task Test()
{

    var batch = new List<(long test1, string teststring1)>
    {
        (1, "1-one"),
        (2, "2-two"),
        (3, "3-three"),
        (1, "4-one"),
        (2, "5-two"),
        (3, "6-three"),
        (1, "7-one"),
        (1, "8-one"),
        (3, "9-three"),
        (2, "10-two")
    };



    object[][] values = await Task.WhenAll(
            batch.Select(
                async t => new object[]
                {
                    t.test1,
                    t.teststring1,
                    await TestFusion(t.test1,t.teststring1).ConfigureAwait(false),
                    await TestFusion(t.test1,t.teststring1).ConfigureAwait(false),
                    (long)(DateTime.UtcNow - new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc)).TotalMilliseconds
                })).ConfigureAwait(false);




    _logger.LogWarning($"Batch of {values.Length} rows inserted.");



}

private async Task<string> TestFusion(long argTest1, string argTeststring1)
{
    
        string cacheKey = $"TestKey-{argTest1}";

        try
        {
            var toReturnCached = await _cachingProvider.GetAsync(
                cacheKey,
                async () =>
                {
                    await Task.Delay(500);
                    return argTeststring1;
                },
                TimeSpan.FromHours(24)).ConfigureAwait(false);

            if (toReturnCached.HasValue)
            {
                return toReturnCached.Value;
            }

            _logger.LogError($"TestFusion: Set Key:{cacheKey} at {DateTime.Now}");
        }
        catch (Exception e)
        {
            _logger.LogError(e, "TestFusion: {cacheKey} - {Exception}", cacheKey, e);
        }

        return string.Empty;
    
}`

GetAsync calls GetOrSetAsync with Redis Backplane

With FusionCacheNewtonsoftJsonSerializer works.
with FusionCacheProtoBufNetSerializer raise the error as described.

System.InvalidOperationException: Duplicate field-number detected; 1 on: ZiggyCreatures.Caching.Fusion.Internals.Distributed.FusionCacheDistributedEntry1[[System.String, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]] at ProtoBuf.Internal.Serializers.TypeSerializer1.Init(Int32[] fieldNumbers, IRuntimeProtoSerializerNode[] serializers, MethodInfo[] baseCtorCallbacks, Boolean isRootType, Boolean useConstructor, Boolean assertKnownType, CallbackSet callbacks, Type constructType, MethodInfo factory, SerializerFeatures features) in //src/protobuf-net/Internal/Serializers/TypeSerializer.cs:line 204
at ProtoBuf.Internal.Serializers.TypeSerializer.Create(Type forType, Int32[] fieldNumbers, IRuntimeProtoSerializerNode[] serializers, MethodInfo[] baseCtorCallbacks, Boolean isRootType, Boolean useConstructor, Boolean assertKnownType, CallbackSet callbacks, Type constructType, MethodInfo factory, Type rootType, SerializerFeatures features) in /
/src/protobuf-net/Internal/Serializers/TypeSerializer.cs:line 21
at ProtoBuf.Meta.MetaType.BuildSerializer() in //src/protobuf-net/Meta/MetaType.cs:line 625
at ProtoBuf.Meta.MetaType.get_Serializer() in /
/src/protobuf-net/Meta/MetaType.cs:line 465
at ProtoBuf.Meta.RuntimeTypeModel.g__GetServicesImpl|88_0(RuntimeTypeModel model, Type type, CompatibilityLevel ambient) in //src/protobuf-net/Meta/RuntimeTypeModel.cs:line 1033
at ProtoBuf.Meta.RuntimeTypeModel.GetServicesSlow(Type type, CompatibilityLevel ambient) in /
/src/protobuf-net/Meta/RuntimeTypeModel.cs:line 999
at ProtoBuf.Meta.RuntimeTypeModel.GetSerializerT in //src/protobuf-net/Meta/RuntimeTypeModel.cs:line 962
at ProtoBuf.Meta.TypeModel.TryGetSerializer[T](TypeModel model) in /
/src/protobuf-net.Core/Meta/TypeModel.cs:line 1486
at ProtoBuf.Meta.TypeModel.SerializeImpl[T](State& state, T value) in //src/protobuf-net.Core/Meta/TypeModel.cs:line 377
at ProtoBuf.Meta.TypeModel.Serialize[T](Stream dest, T value, Object userState) in /
/src/protobuf-net.Core/Meta/TypeModel.cs:line 328
at ZiggyCreatures.Caching.Fusion.Serialization.ProtoBufNet.FusionCacheProtoBufNetSerializer.Serialize[T](T obj) in //src/ZiggyCreatures.FusionCache.Serialization.ProtoBufNet/FusionCacheProtoBufNetSerializer.cs:line 107
at ZiggyCreatures.Caching.Fusion.Serialization.ProtoBufNet.FusionCacheProtoBufNetSerializer.SerializeAsync[T](T obj, CancellationToken token) in /
/src/ZiggyCreatures.FusionCache.Serialization.ProtoBufNet/FusionCacheProtoBufNetSerializer.cs:line 125
at ZiggyCreatures.Caching.Fusion.Internals.Distributed.DistributedCacheAccessor.SetEntryAsync[TValue](String operationId, String key, IFusionCacheEntry entry, FusionCacheEntryOptions options, Boolean isBackground, CancellationToken token)

@ilioan ilioan closed this as completed Jan 22, 2025
@ilioan ilioan reopened this Jan 22, 2025
@jodydonetti
Copy link
Collaborator

Hi @ilioan , thanks!
I made a patch locally and will double check this MRE before/after the patch.

Will update soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants