-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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]: Pointer dereference error with Spanner partitioned reads on Dataflow #28959
Comments
I am running into the same error using
using spannerio.Read works |
This also appears entirely related to partitioned reads because I can get query to work if I set |
This does seem like a bug in the batch partitioned reads. The failing line SDK side: https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/io/spannerio/read_batch.go#L70 Appears to be sending in a bad BatchReadOnlyTransactionID from the partitionedRead, And the issue appears to be because those spanner types don't JSON serialize, they BinaryMarshals. So of course the type is bad on encode/decode. Beam doesn't automatically handle binary marshalling, and neither does the final fallback JSON encoding. https://pkg.go.dev/cloud.google.com/go/spanner#BatchReadOnlyTransactionID The fastest fix is to register encoder and decoder functions for the type. So what probably needs to be done is to register a coder for eg. Similar to the following should be added to the spannerio package, but with error handling added in, and pseudo code filled in.
See the doc for some light documentation https://pkg.go.dev/github.com/apache/beam/sdks/v2/go/pkg/beam#RegisterCoder . It still claims json is the default coder, but that's not been true for a bit. The rest of it remains valid however. If one of you two @pequalsnp @azach were to try it and validate it for your usecase(s), that would be a big help versus my simple guess. IOs are hard because one needs to be an expert in both beam and the datasource them to write them well. I'm very happy to review/merge any changes that work for y'all. |
I did write a coder, essentially like this (I took this from an SDF I re-wrote):
That gets rid of that error, but there were other issues. For example, client is nil in |
I'm 50/50 on if that's the right behavior. SDFs get split up into several execution components, so if setup is expensive, it could be called once on a node that never actually needs those parts setup. This could be worked around by moving the necessary client setup code into a sync.Once guarded function, which is called before it's needed. But if we do feel it is a bug with Beam Go SDFs: Setup would be called like here for normal ParDos: That would be fixed here in exec/sdf.go for Creating Restrictions. https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/core/runtime/exec/sdf.go#L52C35-L52C35 But also here for Splitting and sizing restrictions: Here for truncations: https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/core/runtime/exec/sdf.go#L268 It is already being called via the up call for ProcessSizedElementsAndRestrictions, but I'm mentioning it for completeness: |
What happened?
Running into the following panic when attempting a partitioned read on Dataflow:
Error seen on Dataflow:
This collection works when batching is disabled. I've also confirmed this is a root partitionable query (it's also just a simple read from the table).
Issue Priority
Priority: 2 (default / most bugs should be filed as P2)
Issue Components
The text was updated successfully, but these errors were encountered: