apache / apache/beam

AppliedPTransform is used as a key in hashmaps but PTransform is not hashable/equality-comparable

Open
#18,496 0 comments 0 reactions 0 assignees View on GitHub
bug core P3 runners
Dominant language
Java
Stars
8.7k
Forks
4.7k
Avg merge
2d 5h
Merged PRs (30d)
204

Description

There's plenty of occurrences in runners-core of Map or BiMap where the key is an AppliedPTransform.

However, PTransform does not advertise that it is required to implement equals/hashCode, and some transforms can't do it properly anyway - for example, transforms that capture a ValueProvider which is also not hashable/eq-comparable. I'm surprised that things aren't already very broken because of this.

Fundamentally, I don't see why we should ever compare two PTransform's for equality.

I looked at the code and wondered "can AppliedPTransform simply be identity-hashable", but right now the answer is no because we can create an AppliedPTransform for the same transform applied to the same thing multiple times.

Fixing that appears to be not very easy, but definitely possible. Ideally TransformHierarchy.Node would just know its AppliedPTransform, however a Node can be constructed when there's yet no Pipeline. Suppose there's gotta be some way to propagate a Pipeline into Node.finishSpecifying() (which should be called exactly once on the Node, and this should be enforced), and have finishSpecifying() return the AppliedPTransform, and have the caller use that instead of potentially repeatedly calling .toAppliedPTransform() on the same Node.

[~kenn] is on vacation but perhaps [~tgroh] can help with this meanwhile?

CC: [~reuvenlax]

Imported from Jira [BEAM-2699](https://issues.apache.org/jira/browse/BEAM-2699). Original Jira may contain additional context.
Reported by: jkff.

Contributor guide

Open the contributing guide

Research direction

Start by tracing the runners-core Map and BiMap uses keyed by AppliedPTransform, then read PTransform and TransformHierarchy.Node, especially Node.finishSpecifying() and toAppliedPTransform(). Confirm where the same transform application can produce multiple AppliedPTransform instances and define done as eliminating equality/hashability requirements without breaking those uses.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, distributed-systems
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.