Uploaded image for project: 'Beam'
  1. Beam
  2. BEAM-12979

[Go SDK] Avoid shallow copying CacheToken protobufs in statecache.go

Details

    • Bug
    • Status: Resolved
    • P2
    • Resolution: Fixed
    • None
    • 2.34.0
    • sdk-go
    • None

    Description

      Copying protobuf messages should be avoided because it causes a ton of subtle bugs, especially when mutexes inside the object get copied. We should be using pointers instead.

      Some spots where copies of CacheTokens are made instead of pointers:

      https://github.com/apache/beam/blob/f051cd91d46e5dab0ca48f108b27d9d87e6e5e8f/sdks/go/pkg/beam/core/runtime/harness/statecache/statecache.go#L92

      https://github.com/apache/beam/blob/f051cd91d46e5dab0ca48f108b27d9d87e6e5e8f/sdks/go/pkg/beam/core/runtime/harness/statecache/statecache.go#L124

      https://github.com/apache/beam/blob/f051cd91d46e5dab0ca48f108b27d9d87e6e5e8f/sdks/go/pkg/beam/core/runtime/harness/statecache/statecache_test.go#L119

      This isn't an exhaustive list, but it covers some of the major instances. Should be easy enough to ctrl+f "fnpb.ProcessBundleRequest_CacheToken" and just make sure it's used as a pointer wherever possible.

      Attachments

        Issue Links

          Activity

            People

              jrmccluskey Jack McCluskey
              danoliveira Daniel Oliveira
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 1.5h
                  1.5h