-
Notifications
You must be signed in to change notification settings - Fork 162
[Feature][api][runtime] Support auto resolve memory reference for passing data across actions. #950
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
base: main
Are you sure you want to change the base?
Changes from all commits
9bde274
1c9da03
600b901
e4ffa8e
e2aa09e
e124231
ff464d9
3303fb9
ba79e7d
28da154
6019ef9
213ca02
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,13 @@ | |
| import com.fasterxml.jackson.annotation.JsonIgnore; | ||
| import com.fasterxml.jackson.annotation.JsonInclude; | ||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
| import com.fasterxml.jackson.core.JsonParser; | ||
| import com.fasterxml.jackson.databind.DeserializationContext; | ||
| import com.fasterxml.jackson.databind.JsonDeserializer; | ||
| import com.fasterxml.jackson.databind.JsonNode; | ||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import com.fasterxml.jackson.databind.annotation.JsonDeserialize; | ||
| import org.apache.flink.agents.api.context.MemoryRef; | ||
|
|
||
| import javax.annotation.Nullable; | ||
|
|
||
|
|
@@ -42,6 +48,11 @@ public class Event { | |
| private final String type; | ||
| private final Map<String, Object> attributes; | ||
|
|
||
| // Keep the annotation on the field as well as the creator parameter so it also applies when | ||
| // Jackson constructs Event subclasses whose creators do not declare attachments. | ||
| @JsonDeserialize(contentUsing = AttachmentValueDeserializer.class) | ||
| private final Map<String, Object> attachments; | ||
|
|
||
| @Nullable private UUID upstreamEventId; | ||
| @Nullable private String upstreamActionName; | ||
|
|
||
|
|
@@ -53,7 +64,7 @@ public class Event { | |
|
|
||
| /** Unified event with user-defined type and attributes. */ | ||
| public Event(String type, Map<String, Object> attributes) { | ||
| this(UUID.randomUUID(), type, attributes); | ||
| this(UUID.randomUUID(), type, attributes, new HashMap<>()); | ||
| } | ||
|
|
||
| /** Unified event with user-defined type and empty attributes. */ | ||
|
|
@@ -78,6 +89,9 @@ public Event( | |
| @JsonProperty("id") UUID id, | ||
| @JsonProperty("type") String type, | ||
| @JsonProperty("attributes") Map<String, Object> attributes, | ||
| @JsonProperty("attachments") | ||
| @JsonDeserialize(contentUsing = AttachmentValueDeserializer.class) | ||
| Map<String, Object> attachments, | ||
| @JsonProperty("upstreamEventId") @Nullable UUID upstreamEventId, | ||
| @JsonProperty("upstreamActionName") @Nullable String upstreamActionName) { | ||
| if (type == null || type.isEmpty()) { | ||
|
|
@@ -86,14 +100,31 @@ public Event( | |
| // Explicit null matches an omitted id: both mint a per-occurrence UUID. | ||
| this.id = id != null ? id : UUID.randomUUID(); | ||
| this.type = type; | ||
| this.attributes = attributes != null ? attributes : new HashMap<>(); | ||
| this.attributes = attributes != null ? new HashMap<>(attributes) : new HashMap<>(); | ||
| this.attachments = attachments != null ? new HashMap<>(attachments) : new HashMap<>(); | ||
| this.upstreamEventId = upstreamEventId; | ||
| this.upstreamActionName = upstreamActionName; | ||
| } | ||
|
|
||
| /** Reconstructs an Event with an existing identity, attachments, and no upstream lineage. */ | ||
| public Event( | ||
| UUID id, String type, Map<String, Object> attributes, Map<String, Object> attachments) { | ||
| this(id, type, attributes, attachments, null, null); | ||
| } | ||
|
|
||
| /** Reconstructs an Event with an existing identity and optional framework-managed lineage. */ | ||
| public Event( | ||
| UUID id, | ||
| String type, | ||
| Map<String, Object> attributes, | ||
| @Nullable UUID upstreamEventId, | ||
| @Nullable String upstreamActionName) { | ||
| this(id, type, attributes, new HashMap<>(), upstreamEventId, upstreamActionName); | ||
| } | ||
|
|
||
| /** Reconstructs an Event with an existing identity and no upstream lineage. */ | ||
| public Event(UUID id, String type, Map<String, Object> attributes) { | ||
| this(id, type, attributes, null, null); | ||
| this(id, type, attributes, new HashMap<>(), null, null); | ||
| } | ||
|
|
||
| public UUID getId() { | ||
|
|
@@ -110,6 +141,10 @@ public Map<String, Object> getAttributes() { | |
| return attributes; | ||
| } | ||
|
|
||
| public Map<String, Object> getAttachments() { | ||
| return attachments; | ||
| } | ||
|
|
||
| /** Returns the ID of the Event consumed by the Action that emitted this Event. */ | ||
| @Nullable | ||
| @JsonInclude(JsonInclude.Include.NON_NULL) | ||
|
|
@@ -150,6 +185,14 @@ public void setAttr(String name, Object value) { | |
| attributes.put(name, value); | ||
| } | ||
|
|
||
| public Object getAttachment(String name) { | ||
| return attachments.get(name); | ||
| } | ||
|
|
||
| public void setAttachment(String name, Object value) { | ||
| attachments.put(name, value); | ||
| } | ||
|
|
||
| @JsonIgnore | ||
| public boolean hasSourceTimestamp() { | ||
| return sourceTimestamp != null; | ||
|
|
@@ -166,9 +209,9 @@ public void setSourceTimestamp(long timestamp) { | |
| } | ||
|
|
||
| /** | ||
| * Creates a base Event from another Event, copying its identity, data, and framework metadata. | ||
| * Subclasses override this to reconstruct typed event objects with proper field | ||
| * deserialization. | ||
| * Creates a base Event from another Event, copying its identity, data, attachments, and | ||
| * framework metadata. Subclasses override this to reconstruct typed event objects with proper | ||
| * field deserialization. | ||
| */ | ||
| public static Event fromEvent(Event event) { | ||
| return reconstructFrom( | ||
|
|
@@ -194,6 +237,8 @@ protected static <T extends Event> T reconstructFrom( | |
| + source.getId()); | ||
| } | ||
| Event reconstructedEvent = reconstructed; | ||
| reconstructedEvent.attachments.clear(); | ||
| reconstructedEvent.attachments.putAll(source.attachments); | ||
| reconstructedEvent.sourceTimestamp = source.sourceTimestamp; | ||
| reconstructedEvent.upstreamEventId = source.upstreamEventId; | ||
| reconstructedEvent.upstreamActionName = source.upstreamActionName; | ||
|
|
@@ -211,18 +256,34 @@ public static Event fromJson(String json) throws IOException { | |
| return MAPPER.readValue(json, Event.class); | ||
| } | ||
|
|
||
| /** Deserializes one attachment value, preserving explicitly tagged memory references. */ | ||
| static final class AttachmentValueDeserializer extends JsonDeserializer<Object> { | ||
|
|
||
| @Override | ||
| public Object deserialize(JsonParser parser, DeserializationContext context) | ||
| throws IOException { | ||
| JsonNode node = parser.getCodec().readTree(parser); | ||
| if (node.isObject() | ||
| && MemoryRef.TYPE_VALUE.equals(node.path(MemoryRef.TYPE_FIELD).asText())) { | ||
| return parser.getCodec().treeToValue(node, MemoryRef.class); | ||
| } | ||
| return parser.getCodec().treeToValue(node, Object.class); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
| if (o == null || getClass() != o.getClass()) return false; | ||
| Event other = (Event) o; | ||
| return Objects.equals(this.id, other.id) | ||
| && Objects.equals(this.getType(), other.getType()) | ||
| && Objects.equals(this.attributes, other.attributes); | ||
| && Objects.equals(this.attributes, other.attributes) | ||
| && Objects.equals(this.attachments, other.attachments); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(id, getType(), attributes); | ||
| return Objects.hash(id, getType(), attributes, attachments); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Including A fan-out with durable execution on is where that shows: Adding
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for point that. I think the case you described can occur when durable execution is enabled. However, I do not see a simple fix that fits within this PR. Maybe we need to redesign Do you have any suggestions on this? I would appreciate your thoughts.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, an occurrence id is the right direction. But it cannot be
So the id has to do two things at once. It has to be unique per occurrence inside one There may already be a pattern to borrow. If you go that way, an ordinal is the cheap option, but it is only replay-stable if dispatch order is deterministic across a restart, and I have not checked whether that holds once continuations and async calls interleave. A lineage path (parent occurrence, action name, index in that action's output list) sidesteps that question. One thing that may take some pressure off this PR: the collision does not need attachments. Two events with the same attributes sent to the same action already land on one Your PR makes it much easier to hit, because a fan-out that carries its payload in an attachment leaves the attributes identical by design. But the hole is already there. So this looks like a durable execution issue rather than an attachments one, and probably wants its own issue and its own tests rather than a fix squeezed in here. Also my repro above does not compile, sorry. There is no
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the detailed analysis. I created #1084 to track this separately. Since the collision already exists without attachments and likely requires a replay-stable occurrence identity, I agree that it should not be addressed in this PR. |
||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.