From 593418a6161e41f715c1a61ec9e45f2956c99b0e Mon Sep 17 00:00:00 2001 From: Keran Yang Date: Mon, 15 Jan 2024 10:47:39 -0500 Subject: [PATCH 01/11] copy reducer Signed-off-by: Keran Yang --- .../numaflow/reducestreamer/ActorRequest.java | 28 +++ .../reducestreamer/ActorResponse.java | 24 +++ .../numaflow/reducestreamer/Constants.java | 13 ++ .../numaflow/reducestreamer/Datum.java | 30 +++ .../numaflow/reducestreamer/GRPCConfig.java | 26 +++ .../numaflow/reducestreamer/HandlerDatum.java | 28 +++ .../reducestreamer/IntervalWindow.java | 22 +++ .../numaflow/reducestreamer/Message.java | 58 ++++++ .../numaflow/reducestreamer/MessageList.java | 39 ++++ .../numaflow/reducestreamer/Metadata.java | 14 ++ .../numaflow/reducestreamer/ReduceActor.java | 96 ++++++++++ .../reducestreamer/ReduceShutdownActor.java | 67 +++++++ .../reducestreamer/ReduceSupervisorActor.java | 181 ++++++++++++++++++ .../numaflow/reducestreamer/Reducer.java | 30 +++ .../reducestreamer/ReducerFactory.java | 10 + .../numaflow/reducestreamer/Server.java | 109 +++++++++++ .../numaflow/reducestreamer/Service.java | 127 ++++++++++++ .../metadata/IntervalWindowImpl.java | 26 +++ .../reducestreamer/metadata/MetadataImpl.java | 18 ++ 19 files changed, 946 insertions(+) create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/Constants.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/Datum.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/GRPCConfig.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/HandlerDatum.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/IntervalWindow.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/Message.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/MessageList.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/Metadata.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/ReduceActor.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/ReduceShutdownActor.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/Reducer.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/ReducerFactory.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/Server.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/Service.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/metadata/IntervalWindowImpl.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/metadata/MetadataImpl.java diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java new file mode 100644 index 00000000..7b71b781 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java @@ -0,0 +1,28 @@ +package io.numaproj.numaflow.reducestreamer; + +import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; +import lombok.AllArgsConstructor; +import lombok.Getter; + +/** + * ActorRequest is to store the request sent to ReduceActors. + */ +@Getter +@AllArgsConstructor +class ActorRequest { + ReduceOuterClass.ReduceRequest request; + + // TODO - do we need to include window information in the id? + // for aligned reducer, there is always single window. + // but at the same time, would like to be consistent with GO SDK implementation. + // we will revisit this one later. + public String getUniqueIdentifier() { + return String.join( + Constants.DELIMITER, + this.getRequest().getPayload().getKeysList().toArray(new String[0])); + } + + public String[] getKeySet() { + return this.getRequest().getPayload().getKeysList().toArray(new String[0]); + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java new file mode 100644 index 00000000..6cfd2234 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java @@ -0,0 +1,24 @@ +package io.numaproj.numaflow.reducestreamer; + +import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; +import lombok.AllArgsConstructor; +import lombok.Getter; + +/** + * ActorResponse is to store the response from ReduceActors. + */ +@Getter +@AllArgsConstructor +class ActorResponse { + ReduceOuterClass.ReduceResponse response; + + // TODO - do we need to include window information in the id? + // for aligned reducer, there is always single window. + // but at the same time, would like to be consistent with GO SDK implementation. + // we will revisit this one later. + public String getUniqueIdentifier() { + return String.join( + Constants.DELIMITER, + this.getResponse().getResult().getKeysList().toArray(new String[0])); + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Constants.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Constants.java new file mode 100644 index 00000000..a955e87e --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Constants.java @@ -0,0 +1,13 @@ +package io.numaproj.numaflow.reducestreamer; + +class Constants { + public static final int DEFAULT_MESSAGE_SIZE = 1024 * 1024 * 64; + + public static final String DEFAULT_SOCKET_PATH = "/var/run/numaflow/reduce.sock"; + + public static final String EOF = "EOF"; + + public static final String SUCCESS = "SUCCESS"; + + public static final String DELIMITER = ":"; +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Datum.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Datum.java new file mode 100644 index 00000000..75bf9c31 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Datum.java @@ -0,0 +1,30 @@ +package io.numaproj.numaflow.reducestreamer; + +import java.time.Instant; + +/** + * Datum contains methods to get the payload information. + */ + +public interface Datum { + /** + * method to get the payload value + * + * @return returns the payload value in byte array + */ + public byte[] getValue(); + + /** + * method to get the event time of the payload + * + * @return returns the event time of the payload + */ + public Instant getEventTime(); + + /** + * method to get the watermark information + * + * @return returns the watermark + */ + public Instant getWatermark(); +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/GRPCConfig.java b/src/main/java/io/numaproj/numaflow/reducestreamer/GRPCConfig.java new file mode 100644 index 00000000..5f5a1137 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/GRPCConfig.java @@ -0,0 +1,26 @@ +package io.numaproj.numaflow.reducestreamer; + +import io.numaproj.numaflow.info.ServerInfoAccessor; +import lombok.Builder; +import lombok.Getter; + +/** + * GRPCConfig is used to provide configurations for gRPC server. + */ +@Getter +@Builder(builderMethodName = "newBuilder") +public class GRPCConfig { + private String socketPath; + private int maxMessageSize; + private String infoFilePath; + + /** + * Static method to create default GRPCConfig. + */ + static GRPCConfig defaultGrpcConfig() { + return GRPCConfig.newBuilder() + .infoFilePath(ServerInfoAccessor.DEFAULT_SERVER_INFO_FILE_PATH) + .maxMessageSize(Constants.DEFAULT_MESSAGE_SIZE) + .socketPath(Constants.DEFAULT_SOCKET_PATH).build(); + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/HandlerDatum.java b/src/main/java/io/numaproj/numaflow/reducestreamer/HandlerDatum.java new file mode 100644 index 00000000..45333a03 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/HandlerDatum.java @@ -0,0 +1,28 @@ +package io.numaproj.numaflow.reducestreamer; + + +import lombok.AllArgsConstructor; + +import java.time.Instant; + +@AllArgsConstructor +class HandlerDatum implements Datum { + private byte[] value; + private Instant watermark; + private Instant eventTime; + + @Override + public Instant getWatermark() { + return this.watermark; + } + + @Override + public byte[] getValue() { + return this.value; + } + + @Override + public Instant getEventTime() { + return this.eventTime; + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/IntervalWindow.java b/src/main/java/io/numaproj/numaflow/reducestreamer/IntervalWindow.java new file mode 100644 index 00000000..2839cc31 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/IntervalWindow.java @@ -0,0 +1,22 @@ +package io.numaproj.numaflow.reducestreamer; + +import java.time.Instant; + +/** + * IntervalWindow contains methods to get the information for a given interval window. + */ +public interface IntervalWindow { + /** + * method to get the start time of the interval window + * + * @return start time of the window + */ + Instant getStartTime(); + + /** + * method to get the end time of the interval window + * + * @return end time of the window + */ + Instant getEndTime(); +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Message.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Message.java new file mode 100644 index 00000000..c3c3e48c --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Message.java @@ -0,0 +1,58 @@ +package io.numaproj.numaflow.reducestreamer; + +import lombok.Getter; + +/** + * Message is used to wrap the data returned by Reducer functions. + */ + +@Getter +public class Message { + public static final String DROP = "U+005C__DROP__"; + + private final String[] keys; + private final byte[] value; + private final String[] tags; + + + /** + * used to create Message with value, keys and tags(used for conditional forwarding) + * + * @param value message value + * @param keys message keys + * @param tags message tags which will be used for conditional forwarding + */ + public Message(byte[] value, String[] keys, String[] tags) { + this.keys = keys; + this.value = value; + this.tags = tags; + } + + /** + * used to create Message with value. + * + * @param value message value + */ + public Message(byte[] value) { + this(value, null, null); + } + + /** + * used to create Message with value and keys. + * + * @param value message value + * @param keys message keys + */ + public Message(byte[] value, String[] keys) { + this(value, keys, null); + } + + /** + * creates a Message which will be dropped + * + * @return returns the Message which will be dropped + */ + public static Message toDrop() { + return new Message(new byte[0], null, new String[]{DROP}); + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/MessageList.java b/src/main/java/io/numaproj/numaflow/reducestreamer/MessageList.java new file mode 100644 index 00000000..7e000dac --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/MessageList.java @@ -0,0 +1,39 @@ +package io.numaproj.numaflow.reducestreamer; + +import lombok.Builder; +import lombok.Getter; +import lombok.Singular; + +import java.util.ArrayList; +import java.util.Collection; + +/** + * MessageList is used to return the list of Messages returned from Reducer functions. + */ + +@Getter +@Builder(builderMethodName = "newBuilder") +public class MessageList { + + @Singular("addMessage") + private Iterable messages; + + /** + * Builder to build MessageList + */ + public static class MessageListBuilder { + /** + * @param messages to append all the messages to MessageList + * + * @return returns the builder + */ + public MessageListBuilder addMessages(Iterable messages) { + if (this.messages == null) { + this.messages = new ArrayList<>(); + return this; + } + this.messages.addAll((Collection) messages); + return this; + } + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Metadata.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Metadata.java new file mode 100644 index 00000000..f18be4b5 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Metadata.java @@ -0,0 +1,14 @@ +package io.numaproj.numaflow.reducestreamer; + +/** + * Metadata contains methods to get the metadata for the reduce operation. + */ +public interface Metadata { + /** + * method to get the interval window. + * + * @return IntervalWindow which has the window information + */ + IntervalWindow getIntervalWindow(); +} + diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceActor.java new file mode 100644 index 00000000..32522f40 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceActor.java @@ -0,0 +1,96 @@ +package io.numaproj.numaflow.reducestreamer; + +import akka.actor.AbstractActor; +import akka.actor.Props; +import akka.japi.pf.ReceiveBuilder; +import com.google.protobuf.ByteString; +import com.google.protobuf.Timestamp; +import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; +import lombok.AllArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +/** + * Reduce actor invokes the reducer and returns the result. + */ + +@Slf4j +@AllArgsConstructor +class ReduceActor extends AbstractActor { + private String[] keys; + private Metadata md; + private Reducer groupBy; + + public static Props props(String[] keys, Metadata md, Reducer groupBy) { + return Props.create(ReduceActor.class, keys, md, groupBy); + } + + @Override + public Receive createReceive() { + return ReceiveBuilder + .create() + .match(HandlerDatum.class, this::invokeHandler) + .match(String.class, this::getResult) + .build(); + } + + private void invokeHandler(HandlerDatum handlerDatum) { + this.groupBy.addMessage(keys, handlerDatum, md); + } + + private void getResult(String eof) { + MessageList resultMessages = this.groupBy.getOutput(keys, md); + // send the result back to sender(parent actor) + resultMessages.getMessages().forEach(message -> { + getSender().tell(buildActorResponse(message), getSelf()); + }); + // send a response back with EOF set to true, indicating the reducer has finished the data aggregation. + getSender().tell(buildEOFActorResponse(), getSelf()); + } + + private ActorResponse buildActorResponse(Message message) { + ReduceOuterClass.ReduceResponse.Builder responseBuilder = ReduceOuterClass.ReduceResponse.newBuilder(); + // set the window using the actor metadata. + responseBuilder.setWindow(ReduceOuterClass.Window.newBuilder() + .setStart(Timestamp.newBuilder() + .setSeconds(this.md.getIntervalWindow().getStartTime().getEpochSecond()) + .setNanos(this.md.getIntervalWindow().getStartTime().getNano())) + .setEnd(Timestamp.newBuilder() + .setSeconds(this.md.getIntervalWindow().getEndTime().getEpochSecond()) + .setNanos(this.md.getIntervalWindow().getEndTime().getNano())) + .setSlot("slot-0").build()); + responseBuilder.setEOF(false); + // set the result. + responseBuilder.setResult(ReduceOuterClass.ReduceResponse.Result + .newBuilder() + .setValue(ByteString.copyFrom(message.getValue())) + .addAllKeys(message.getKeys() + == null ? new ArrayList<>():Arrays.asList(message.getKeys())) + .addAllTags( + message.getTags() == null ? new ArrayList<>():List.of(message.getTags())) + .build()); + return new ActorResponse(responseBuilder.build()); + } + + private ActorResponse buildEOFActorResponse() { + ReduceOuterClass.ReduceResponse.Builder responseBuilder = ReduceOuterClass.ReduceResponse.newBuilder(); + responseBuilder.setWindow(ReduceOuterClass.Window.newBuilder() + .setStart(Timestamp.newBuilder() + .setSeconds(this.md.getIntervalWindow().getStartTime().getEpochSecond()) + .setNanos(this.md.getIntervalWindow().getStartTime().getNano())) + .setEnd(Timestamp.newBuilder() + .setSeconds(this.md.getIntervalWindow().getEndTime().getEpochSecond()) + .setNanos(this.md.getIntervalWindow().getEndTime().getNano())) + .setSlot("slot-0").build()); + responseBuilder.setEOF(true); + // set a dummy result with the keys. + responseBuilder.setResult(ReduceOuterClass.ReduceResponse.Result + .newBuilder() + .addAllKeys(List.of(this.keys)) + .build()); + return new ActorResponse(responseBuilder.build()); + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceShutdownActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceShutdownActor.java new file mode 100644 index 00000000..cb8bf1bb --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceShutdownActor.java @@ -0,0 +1,67 @@ +package io.numaproj.numaflow.reducestreamer; + +import akka.actor.AbstractActor; +import akka.actor.AllDeadLetters; +import akka.actor.Props; +import akka.japi.pf.ReceiveBuilder; +import lombok.AllArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +import java.util.Optional; +import java.util.concurrent.CompletableFuture; + +/** + * Shutdown actor, listens to exceptions and handles shutdown. + */ + + +@Slf4j +@AllArgsConstructor +class ReduceShutdownActor extends AbstractActor { + private final CompletableFuture failureFuture; + + public static Props props( + CompletableFuture failureFuture) { + return Props.create(ReduceShutdownActor.class, failureFuture); + } + + @Override + public void preRestart(Throwable reason, Optional message) { + failureFuture.completeExceptionally(reason); + } + + @Override + public Receive createReceive() { + return ReceiveBuilder + .create() + .match(Throwable.class, this::shutdown) + .match(String.class, this::completedSuccessfully) + .match(AllDeadLetters.class, this::handleDeadLetters) + .build(); + } + + /* + complete the future with exception so that the exception will be thrown + indicate that same to response observer. + */ + private void shutdown(Throwable throwable) { + log.debug("got a shut down exception"); + failureFuture.completeExceptionally(throwable); + } + + // if there are no exceptions, complete the future without exception. + private void completedSuccessfully(String eof) { + log.debug("completed successfully of shutdown executed"); + failureFuture.complete(null); + // if all the actors completed successfully, we can stop the shutdown actor. + getContext().getSystem().stop(getSelf()); + } + + // if we see dead letters, we need to stop the execution and exit + // to make sure no messages are lost + private void handleDeadLetters(AllDeadLetters deadLetter) { + log.debug("got a dead letter, stopping the execution"); + failureFuture.completeExceptionally(new Throwable("dead letters")); + getContext().getSystem().stop(getSelf()); + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java new file mode 100644 index 00000000..038d8d29 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java @@ -0,0 +1,181 @@ +package io.numaproj.numaflow.reducestreamer; + +import akka.actor.AbstractActor; +import akka.actor.ActorRef; +import akka.actor.ChildRestartStats; +import akka.actor.Props; +import akka.actor.SupervisorStrategy; +import akka.japi.pf.DeciderBuilder; +import akka.japi.pf.ReceiveBuilder; +import com.google.common.base.Preconditions; +import io.grpc.stub.StreamObserver; +import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; +import lombok.extern.slf4j.Slf4j; +import scala.PartialFunction; +import scala.collection.Iterable; + +import java.time.Instant; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; + +/** + * ReduceSupervisorActor actor distributes the messages to actors and handles failure. + */ +@Slf4j +class ReduceSupervisorActor extends AbstractActor { + private final ReducerFactory reducerFactory; + private final Metadata md; + private final ActorRef shutdownActor; + private final StreamObserver responseObserver; + private final Map actorsMap = new HashMap<>(); + + public ReduceSupervisorActor( + ReducerFactory reducerFactory, + Metadata md, + ActorRef shutdownActor, + StreamObserver responseObserver) { + this.reducerFactory = reducerFactory; + this.md = md; + this.shutdownActor = shutdownActor; + this.responseObserver = responseObserver; + } + + public static Props props( + ReducerFactory reducerFactory, + Metadata md, + ActorRef shutdownActor, + StreamObserver responseObserver) { + return Props.create( + ReduceSupervisorActor.class, + reducerFactory, + md, + shutdownActor, + responseObserver); + } + + // if there is an uncaught exception stop in the supervisor actor, send a signal to shut down + @Override + public void preRestart(Throwable reason, Optional message) { + log.debug("supervisor pre restart was executed"); + shutdownActor.tell(reason, ActorRef.noSender()); + Service.reduceActorSystem.stop(getSelf()); + } + + @Override + public SupervisorStrategy supervisorStrategy() { + return new ReduceSupervisorStrategy(); + } + + + @Override + public void postStop() { + log.debug("post stop of supervisor executed - {}", getSelf().toString()); + shutdownActor.tell(Constants.SUCCESS, ActorRef.noSender()); + } + + @Override + public Receive createReceive() { + return ReceiveBuilder + .create() + .match(ActorRequest.class, this::invokeActors) + .match(String.class, this::sendEOF) + .match(ActorResponse.class, this::responseListener) + .build(); + } + + /* + based on the keys of the input message invoke the right actor + if there is no actor for an incoming set of keys, create a new actor + track all the child actors using actors map + */ + private void invokeActors(ActorRequest actorRequest) { + String[] keys = actorRequest.getKeySet(); + String uniqueId = actorRequest.getUniqueIdentifier(); + if (!actorsMap.containsKey(uniqueId)) { + Reducer reduceHandler = reducerFactory.createReducer(); + ActorRef actorRef = getContext() + .actorOf(ReduceActor.props(keys, md, reduceHandler)); + actorsMap.put(uniqueId, actorRef); + } + + HandlerDatum handlerDatum = constructHandlerDatum(actorRequest.getRequest().getPayload()); + actorsMap.get(uniqueId).tell(handlerDatum, getSelf()); + } + + private void sendEOF(String EOF) { + for (Map.Entry entry : actorsMap.entrySet()) { + entry.getValue().tell(EOF, getSelf()); + } + } + + // listen to child actors for the result. + private void responseListener(ActorResponse actorResponse) { + /* + send the result back to the client + remove the child entry from the map after getting result. + if there are no entries in the map, that means processing is + done we can close the stream. + */ + responseObserver.onNext(actorResponse.getResponse()); + if (actorResponse.getResponse().getEOF()) { + actorsMap.remove(actorResponse.getUniqueIdentifier()); + if (actorsMap.isEmpty()) { + responseObserver.onCompleted(); + getContext().getSystem().stop(getSelf()); + } + } + } + + private HandlerDatum constructHandlerDatum(ReduceOuterClass.ReduceRequest.Payload payload) { + return new HandlerDatum( + payload.getValue().toByteArray(), + Instant.ofEpochSecond( + payload.getWatermark().getSeconds(), + payload.getWatermark().getNanos()), + Instant.ofEpochSecond( + payload.getEventTime().getSeconds(), + payload.getEventTime().getNanos()) + ); + } + + /* + We need supervisor to handle failures, by default if there are any failures + actors will be restarted, but we want to escalate the exception and terminate + the system. + */ + private final class ReduceSupervisorStrategy extends SupervisorStrategy { + + @Override + public PartialFunction decider() { + return DeciderBuilder.match(Exception.class, e -> SupervisorStrategy.stop()).build(); + } + + @Override + public void handleChildTerminated( + akka.actor.ActorContext context, + ActorRef child, + Iterable children) { + + } + + @Override + public void processFailure( + akka.actor.ActorContext context, + boolean restart, + ActorRef child, + Throwable cause, + ChildRestartStats stats, + Iterable children) { + + Preconditions.checkArgument( + !restart, + "on failures, we will never restart our actors, we escalate"); + /* + tell the shutdown actor about the exception. + */ + log.debug("process failure of supervisor strategy executed - {}", getSelf().toString()); + shutdownActor.tell(cause, context.parent()); + } + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Reducer.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Reducer.java new file mode 100644 index 00000000..e444416a --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Reducer.java @@ -0,0 +1,30 @@ +package io.numaproj.numaflow.reducestreamer; + + +/** + * Reducer exposes methods for performing reduce operation. + */ + + +public abstract class Reducer { + /** + * addMessage will be invoked for each input message. + * It can be used to read the input data from datum and + * update the result for given keys. + * + * @param keys message keys + * @param datum current message to be processed + * @param md metadata which contains window information + */ + public abstract void addMessage(String[] keys, Datum datum, Metadata md); + + /** + * getOutput will be invoked at the end of input. + * + * @param keys message keys + * @param md metadata which contains window information + * + * @return MessageList output value, aggregated result + */ + public abstract MessageList getOutput(String[] keys, Metadata md); +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReducerFactory.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReducerFactory.java new file mode 100644 index 00000000..6f1060ed --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReducerFactory.java @@ -0,0 +1,10 @@ +package io.numaproj.numaflow.reducestreamer; + + +/** + * ReducerFactory is the factory for Reducer. + */ + +public abstract class ReducerFactory { + public abstract ReducerT createReducer(); +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Server.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Server.java new file mode 100644 index 00000000..61ff542d --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Server.java @@ -0,0 +1,109 @@ +package io.numaproj.numaflow.reducestreamer; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.annotations.VisibleForTesting; +import io.grpc.ServerBuilder; +import io.numaproj.numaflow.info.ServerInfoAccessor; +import io.numaproj.numaflow.info.ServerInfoAccessorImpl; +import io.numaproj.numaflow.shared.GrpcServerUtils; +import lombok.extern.slf4j.Slf4j; + +import java.util.concurrent.TimeUnit; + +/** + * Server is the gRPC server for executing reduce operation. + */ +@Slf4j +public class Server { + + private final GRPCConfig grpcConfig; + private final Service service; + private final ServerInfoAccessor serverInfoAccessor = new ServerInfoAccessorImpl(new ObjectMapper()); + private io.grpc.Server server; + + /** + * constructor to create gRPC server. + * + * @param reducerFactory to process the message + */ + public Server(ReducerFactory reducerFactory) { + this(reducerFactory, GRPCConfig.defaultGrpcConfig()); + } + + /** + * constructor to create gRPC server with gRPC config. + * + * @param grpcConfig to configure the max message size for grpc + * @param reducerFactory to process the message + */ + public Server(ReducerFactory reducerFactory, GRPCConfig grpcConfig) { + this.service = new Service(reducerFactory); + this.grpcConfig = grpcConfig; + } + + /** + * Start serving requests. + * + * @throws Exception if server fails to start + */ + public void start() throws Exception { + GrpcServerUtils.writeServerInfo( + serverInfoAccessor, + grpcConfig.getSocketPath(), + grpcConfig.getInfoFilePath()); + + if (this.server == null) { + // create server builder + ServerBuilder serverBuilder = GrpcServerUtils.createServerBuilder( + grpcConfig.getSocketPath(), grpcConfig.getMaxMessageSize()); + + // build server + this.server = serverBuilder + .addService(this.service) + .build(); + } + + // start server + server.start(); + + log.info( + "Server started, listening on socket path: " + + grpcConfig.getSocketPath()); + + // register shutdown hook + Runtime.getRuntime().addShutdownHook(new Thread(() -> { + // Use stderr here since the logger may have been reset by its JVM shutdown hook. + System.err.println("*** shutting down gRPC server since JVM is shutting down"); + try { + Server.this.stop(); + } catch (InterruptedException e) { + Thread.interrupted(); + e.printStackTrace(System.err); + } + })); + } + + /** + * Stop serving requests and shutdown resources. Await termination on the main thread since the + * grpc library uses daemon threads. + * + * @throws InterruptedException if shutdown is interrupted + */ + public void stop() throws InterruptedException { + if (server != null) { + server.shutdown().awaitTermination(30, TimeUnit.SECONDS); + } + } + + /** + * Set server builder for testing. + * + * @param serverBuilder + */ + @VisibleForTesting + void setServerBuilder(ServerBuilder serverBuilder) { + this.server = serverBuilder + .addService(this.service) + .build(); + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java new file mode 100644 index 00000000..fc9cbf67 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java @@ -0,0 +1,127 @@ +package io.numaproj.numaflow.reducestreamer; + +import akka.actor.ActorRef; +import akka.actor.ActorSystem; +import akka.actor.AllDeadLetters; +import com.google.protobuf.Empty; +import io.grpc.Status; +import io.grpc.stub.StreamObserver; +import io.numaproj.numaflow.reduce.v1.ReduceGrpc; +import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; +import io.numaproj.numaflow.reducestreamer.metadata.IntervalWindowImpl; +import io.numaproj.numaflow.reducestreamer.metadata.MetadataImpl; +import io.numaproj.numaflow.shared.GrpcServerUtils; +import lombok.extern.slf4j.Slf4j; + +import java.time.Instant; +import java.util.concurrent.CompletableFuture; + +import static io.numaproj.numaflow.reduce.v1.ReduceGrpc.getReduceFnMethod; + +@Slf4j +class Service extends ReduceGrpc.ReduceImplBase { + + public static final ActorSystem reduceActorSystem = ActorSystem.create("reduce"); + + private ReducerFactory reducerFactory; + + public Service(ReducerFactory reducerFactory) { + this.reducerFactory = reducerFactory; + } + + static void handleFailure( + CompletableFuture failureFuture, + StreamObserver responseObserver) { + new Thread(() -> { + try { + failureFuture.get(); + } catch (Exception e) { + e.printStackTrace(); + var status = Status.UNKNOWN.withDescription(e.getMessage()).withCause(e); + responseObserver.onError(status.asException()); + } + }).start(); + } + + /** + * Streams input data to reduceFn and returns the result. + */ + @Override + public StreamObserver reduceFn(final StreamObserver responseObserver) { + + if (this.reducerFactory == null) { + return io.grpc.stub.ServerCalls.asyncUnimplementedStreamingCall( + getReduceFnMethod(), + responseObserver); + } + + // get window start and end time from gPRC metadata + String winSt = GrpcServerUtils.WINDOW_START_TIME.get(); + String winEt = GrpcServerUtils.WINDOW_END_TIME.get(); + + // convert the start and end time to Instant + Instant startTime = Instant.ofEpochMilli(Long.parseLong(winSt)); + Instant endTime = Instant.ofEpochMilli(Long.parseLong(winEt)); + + // create metadata + IntervalWindow iw = new IntervalWindowImpl(startTime, endTime); + Metadata md = new MetadataImpl(iw); + + CompletableFuture failureFuture = new CompletableFuture<>(); + + // create a shutdown actor that listens to exceptions. + ActorRef shutdownActorRef = reduceActorSystem. + actorOf(ReduceShutdownActor.props(failureFuture)); + + // subscribe for dead letters + reduceActorSystem.getEventStream().subscribe(shutdownActorRef, AllDeadLetters.class); + + handleFailure(failureFuture, responseObserver); + /* + create a supervisor actor which assign the tasks to child actors. + we create a child actor for every unique set of keys in a window + */ + ActorRef supervisorActor = reduceActorSystem + .actorOf(ReduceSupervisorActor.props( + reducerFactory, + md, + shutdownActorRef, + responseObserver)); + + + return new StreamObserver<>() { + @Override + public void onNext(ReduceOuterClass.ReduceRequest datum) { + // send the message to parent actor, which takes care of distribution. + if (!supervisorActor.isTerminated()) { + supervisorActor.tell(new ActorRequest(datum), ActorRef.noSender()); + } else { + responseObserver.onError(new Throwable("Supervisor actor was terminated")); + } + } + + @Override + public void onError(Throwable throwable) { + log.error("Error from the client - {}", throwable.getMessage()); + responseObserver.onError(throwable); + } + + @Override + public void onCompleted() { + // indicate the end of input to the supervisor + supervisorActor.tell(Constants.EOF, ActorRef.noSender()); + } + }; + } + + /** + * IsReady is the heartbeat endpoint for gRPC. + */ + @Override + public void isReady( + Empty request, + StreamObserver responseObserver) { + responseObserver.onNext(ReduceOuterClass.ReadyResponse.newBuilder().setReady(true).build()); + responseObserver.onCompleted(); + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/metadata/IntervalWindowImpl.java b/src/main/java/io/numaproj/numaflow/reducestreamer/metadata/IntervalWindowImpl.java new file mode 100644 index 00000000..0815d295 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/metadata/IntervalWindowImpl.java @@ -0,0 +1,26 @@ +package io.numaproj.numaflow.reducestreamer.metadata; + +import io.numaproj.numaflow.reducestreamer.IntervalWindow; +import lombok.AllArgsConstructor; + +import java.time.Instant; + +/** + * IntervalWindowImpl implements IntervalWindow interface which will be passed as metadata to reduce + * handlers + */ +@AllArgsConstructor +public class IntervalWindowImpl implements IntervalWindow { + private final Instant startTime; + private final Instant endTime; + + @Override + public Instant getStartTime() { + return this.startTime; + } + + @Override + public Instant getEndTime() { + return this.endTime; + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/metadata/MetadataImpl.java b/src/main/java/io/numaproj/numaflow/reducestreamer/metadata/MetadataImpl.java new file mode 100644 index 00000000..19b9bf71 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/metadata/MetadataImpl.java @@ -0,0 +1,18 @@ +package io.numaproj.numaflow.reducestreamer.metadata; + +import io.numaproj.numaflow.reducestreamer.IntervalWindow; +import io.numaproj.numaflow.reducestreamer.Metadata; +import lombok.AllArgsConstructor; + +/** + * MetadataImpl implements Metadata interface which will be passed to reduce handlers + */ +@AllArgsConstructor +public class MetadataImpl implements Metadata { + private final IntervalWindow intervalWindow; + + @Override + public IntervalWindow getIntervalWindow() { + return intervalWindow; + } +} From 3164f91ec7effcd22adf355151aadfb62410fc12 Mon Sep 17 00:00:00 2001 From: Keran Yang Date: Mon, 15 Jan 2024 14:37:20 -0500 Subject: [PATCH 02/11] an initial try Signed-off-by: Keran Yang --- ...torResponse.java => ActorEOFResponse.java} | 3 +- .../numaflow/reducestreamer/ActorRequest.java | 2 +- .../numaflow/reducestreamer/Constants.java | 2 +- .../numaflow/reducestreamer/ReduceActor.java | 96 ------------------- .../reducestreamer/ReduceStreamerActor.java | 77 +++++++++++++++ .../reducestreamer/ReduceSupervisorActor.java | 38 ++++---- .../numaflow/reducestreamer/Reducer.java | 30 ------ .../reducestreamer/ReducerFactory.java | 10 -- .../numaflow/reducestreamer/Server.java | 18 ++-- .../numaflow/reducestreamer/Service.java | 20 ++-- .../reducestreamer/{ => model}/Datum.java | 8 +- .../{ => model}/HandlerDatum.java | 4 +- .../{ => model}/IntervalWindow.java | 2 +- .../IntervalWindowImpl.java | 3 +- .../reducestreamer/{ => model}/Message.java | 2 +- .../{ => model}/MessageList.java | 2 +- .../reducestreamer/{ => model}/Metadata.java | 2 +- .../{metadata => model}/MetadataImpl.java | 4 +- .../user/OutputStreamObserver.java | 15 +++ .../user/OutputStreamObserverImpl.java | 52 ++++++++++ .../reducestreamer/user/ReduceStreamer.java | 15 +++ .../user/ReduceStreamerFactory.java | 12 +++ 22 files changed, 232 insertions(+), 185 deletions(-) rename src/main/java/io/numaproj/numaflow/reducestreamer/{ActorResponse.java => ActorEOFResponse.java} (93%) delete mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/ReduceActor.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java delete mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/Reducer.java delete mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/ReducerFactory.java rename src/main/java/io/numaproj/numaflow/reducestreamer/{ => model}/Datum.java (76%) rename src/main/java/io/numaproj/numaflow/reducestreamer/{ => model}/HandlerDatum.java (81%) rename src/main/java/io/numaproj/numaflow/reducestreamer/{ => model}/IntervalWindow.java (89%) rename src/main/java/io/numaproj/numaflow/reducestreamer/{metadata => model}/IntervalWindowImpl.java (81%) rename src/main/java/io/numaproj/numaflow/reducestreamer/{ => model}/Message.java (96%) rename src/main/java/io/numaproj/numaflow/reducestreamer/{ => model}/MessageList.java (94%) rename src/main/java/io/numaproj/numaflow/reducestreamer/{ => model}/Metadata.java (84%) rename src/main/java/io/numaproj/numaflow/reducestreamer/{metadata => model}/MetadataImpl.java (67%) create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserver.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserverImpl.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamer.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamerFactory.java diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorEOFResponse.java similarity index 93% rename from src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/ActorEOFResponse.java index 6cfd2234..48ca0622 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorEOFResponse.java @@ -6,10 +6,11 @@ /** * ActorResponse is to store the response from ReduceActors. + * TODO - not required. Remove */ @Getter @AllArgsConstructor -class ActorResponse { +class ActorEOFResponse { ReduceOuterClass.ReduceResponse response; // TODO - do we need to include window information in the id? diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java index 7b71b781..f8b31136 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java @@ -5,7 +5,7 @@ import lombok.Getter; /** - * ActorRequest is to store the request sent to ReduceActors. + * ActorRequest is to store the request sent to ReduceStreamActors. */ @Getter @AllArgsConstructor diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Constants.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Constants.java index a955e87e..16746dab 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/Constants.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Constants.java @@ -3,7 +3,7 @@ class Constants { public static final int DEFAULT_MESSAGE_SIZE = 1024 * 1024 * 64; - public static final String DEFAULT_SOCKET_PATH = "/var/run/numaflow/reduce.sock"; + public static final String DEFAULT_SOCKET_PATH = "/var/run/numaflow/reducestream.sock"; public static final String EOF = "EOF"; diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceActor.java deleted file mode 100644 index 32522f40..00000000 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceActor.java +++ /dev/null @@ -1,96 +0,0 @@ -package io.numaproj.numaflow.reducestreamer; - -import akka.actor.AbstractActor; -import akka.actor.Props; -import akka.japi.pf.ReceiveBuilder; -import com.google.protobuf.ByteString; -import com.google.protobuf.Timestamp; -import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; -import lombok.AllArgsConstructor; -import lombok.extern.slf4j.Slf4j; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - -/** - * Reduce actor invokes the reducer and returns the result. - */ - -@Slf4j -@AllArgsConstructor -class ReduceActor extends AbstractActor { - private String[] keys; - private Metadata md; - private Reducer groupBy; - - public static Props props(String[] keys, Metadata md, Reducer groupBy) { - return Props.create(ReduceActor.class, keys, md, groupBy); - } - - @Override - public Receive createReceive() { - return ReceiveBuilder - .create() - .match(HandlerDatum.class, this::invokeHandler) - .match(String.class, this::getResult) - .build(); - } - - private void invokeHandler(HandlerDatum handlerDatum) { - this.groupBy.addMessage(keys, handlerDatum, md); - } - - private void getResult(String eof) { - MessageList resultMessages = this.groupBy.getOutput(keys, md); - // send the result back to sender(parent actor) - resultMessages.getMessages().forEach(message -> { - getSender().tell(buildActorResponse(message), getSelf()); - }); - // send a response back with EOF set to true, indicating the reducer has finished the data aggregation. - getSender().tell(buildEOFActorResponse(), getSelf()); - } - - private ActorResponse buildActorResponse(Message message) { - ReduceOuterClass.ReduceResponse.Builder responseBuilder = ReduceOuterClass.ReduceResponse.newBuilder(); - // set the window using the actor metadata. - responseBuilder.setWindow(ReduceOuterClass.Window.newBuilder() - .setStart(Timestamp.newBuilder() - .setSeconds(this.md.getIntervalWindow().getStartTime().getEpochSecond()) - .setNanos(this.md.getIntervalWindow().getStartTime().getNano())) - .setEnd(Timestamp.newBuilder() - .setSeconds(this.md.getIntervalWindow().getEndTime().getEpochSecond()) - .setNanos(this.md.getIntervalWindow().getEndTime().getNano())) - .setSlot("slot-0").build()); - responseBuilder.setEOF(false); - // set the result. - responseBuilder.setResult(ReduceOuterClass.ReduceResponse.Result - .newBuilder() - .setValue(ByteString.copyFrom(message.getValue())) - .addAllKeys(message.getKeys() - == null ? new ArrayList<>():Arrays.asList(message.getKeys())) - .addAllTags( - message.getTags() == null ? new ArrayList<>():List.of(message.getTags())) - .build()); - return new ActorResponse(responseBuilder.build()); - } - - private ActorResponse buildEOFActorResponse() { - ReduceOuterClass.ReduceResponse.Builder responseBuilder = ReduceOuterClass.ReduceResponse.newBuilder(); - responseBuilder.setWindow(ReduceOuterClass.Window.newBuilder() - .setStart(Timestamp.newBuilder() - .setSeconds(this.md.getIntervalWindow().getStartTime().getEpochSecond()) - .setNanos(this.md.getIntervalWindow().getStartTime().getNano())) - .setEnd(Timestamp.newBuilder() - .setSeconds(this.md.getIntervalWindow().getEndTime().getEpochSecond()) - .setNanos(this.md.getIntervalWindow().getEndTime().getNano())) - .setSlot("slot-0").build()); - responseBuilder.setEOF(true); - // set a dummy result with the keys. - responseBuilder.setResult(ReduceOuterClass.ReduceResponse.Result - .newBuilder() - .addAllKeys(List.of(this.keys)) - .build()); - return new ActorResponse(responseBuilder.build()); - } -} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java new file mode 100644 index 00000000..fbd6ee8d --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java @@ -0,0 +1,77 @@ +package io.numaproj.numaflow.reducestreamer; + +import akka.actor.AbstractActor; +import akka.actor.Props; +import akka.japi.pf.ReceiveBuilder; +import com.google.protobuf.Timestamp; +import io.grpc.stub.StreamObserver; +import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; +import io.numaproj.numaflow.reducestreamer.model.HandlerDatum; +import io.numaproj.numaflow.reducestreamer.model.Metadata; +import io.numaproj.numaflow.reducestreamer.user.OutputStreamObserver; +import io.numaproj.numaflow.reducestreamer.user.OutputStreamObserverImpl; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; +import lombok.AllArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +import java.util.List; + +/** + * Reduce stream actor invokes the reducer and returns the result. + */ + +@Slf4j +@AllArgsConstructor +public class ReduceStreamerActor extends AbstractActor { + private String[] keys; + private Metadata md; + private ReduceStreamer groupBy; + private OutputStreamObserver outputStream; + + public static Props props( + String[] keys, Metadata md, ReduceStreamer groupBy, + StreamObserver responseStreamObserver) { + return Props.create( + ReduceStreamerActor.class, + keys, + md, + groupBy, + new OutputStreamObserverImpl(md, responseStreamObserver)); + } + + @Override + public Receive createReceive() { + return ReceiveBuilder + .create() + .match(HandlerDatum.class, this::invokeHandler) + .match(String.class, this::sendEOF) + .build(); + } + + private void invokeHandler(HandlerDatum handlerDatum) { + this.groupBy.processMessage(keys, handlerDatum, outputStream, md); + } + + private void sendEOF(String EOF) { + getSender().tell(buildEOFResponse(), getSelf()); + } + + private ActorEOFResponse buildEOFResponse() { + ReduceOuterClass.ReduceResponse.Builder responseBuilder = ReduceOuterClass.ReduceResponse.newBuilder(); + responseBuilder.setWindow(ReduceOuterClass.Window.newBuilder() + .setStart(Timestamp.newBuilder() + .setSeconds(this.md.getIntervalWindow().getStartTime().getEpochSecond()) + .setNanos(this.md.getIntervalWindow().getStartTime().getNano())) + .setEnd(Timestamp.newBuilder() + .setSeconds(this.md.getIntervalWindow().getEndTime().getEpochSecond()) + .setNanos(this.md.getIntervalWindow().getEndTime().getNano())) + .setSlot("slot-0").build()); + responseBuilder.setEOF(true); + // set a dummy result with the keys. + responseBuilder.setResult(ReduceOuterClass.ReduceResponse.Result + .newBuilder() + .addAllKeys(List.of(this.keys)) + .build()); + return new ActorEOFResponse(responseBuilder.build()); + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java index 038d8d29..89dfea6a 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java @@ -10,6 +10,10 @@ import com.google.common.base.Preconditions; import io.grpc.stub.StreamObserver; import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; +import io.numaproj.numaflow.reducestreamer.model.HandlerDatum; +import io.numaproj.numaflow.reducestreamer.model.Metadata; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; import lombok.extern.slf4j.Slf4j; import scala.PartialFunction; import scala.collection.Iterable; @@ -24,31 +28,31 @@ */ @Slf4j class ReduceSupervisorActor extends AbstractActor { - private final ReducerFactory reducerFactory; + private final ReduceStreamerFactory reduceStreamerFactory; private final Metadata md; private final ActorRef shutdownActor; private final StreamObserver responseObserver; private final Map actorsMap = new HashMap<>(); public ReduceSupervisorActor( - ReducerFactory reducerFactory, + ReduceStreamerFactory reduceStreamerFactory, Metadata md, ActorRef shutdownActor, StreamObserver responseObserver) { - this.reducerFactory = reducerFactory; + this.reduceStreamerFactory = reduceStreamerFactory; this.md = md; this.shutdownActor = shutdownActor; this.responseObserver = responseObserver; } public static Props props( - ReducerFactory reducerFactory, + ReduceStreamerFactory reduceStreamerFactory, Metadata md, ActorRef shutdownActor, StreamObserver responseObserver) { return Props.create( ReduceSupervisorActor.class, - reducerFactory, + reduceStreamerFactory, md, shutdownActor, responseObserver); @@ -80,7 +84,7 @@ public Receive createReceive() { .create() .match(ActorRequest.class, this::invokeActors) .match(String.class, this::sendEOF) - .match(ActorResponse.class, this::responseListener) + .match(ActorEOFResponse.class, this::eofResponseListener) .build(); } @@ -93,9 +97,13 @@ private void invokeActors(ActorRequest actorRequest) { String[] keys = actorRequest.getKeySet(); String uniqueId = actorRequest.getUniqueIdentifier(); if (!actorsMap.containsKey(uniqueId)) { - Reducer reduceHandler = reducerFactory.createReducer(); + ReduceStreamer reduceStreamerHandler = reduceStreamerFactory.createReduceStreamer(); ActorRef actorRef = getContext() - .actorOf(ReduceActor.props(keys, md, reduceHandler)); + .actorOf(ReduceStreamerActor.props( + keys, + md, + reduceStreamerHandler, + responseObserver)); actorsMap.put(uniqueId, actorRef); } @@ -110,20 +118,18 @@ private void sendEOF(String EOF) { } // listen to child actors for the result. - private void responseListener(ActorResponse actorResponse) { + private void eofResponseListener(ActorEOFResponse actorEOFResponse) { /* send the result back to the client remove the child entry from the map after getting result. if there are no entries in the map, that means processing is done we can close the stream. */ - responseObserver.onNext(actorResponse.getResponse()); - if (actorResponse.getResponse().getEOF()) { - actorsMap.remove(actorResponse.getUniqueIdentifier()); - if (actorsMap.isEmpty()) { - responseObserver.onCompleted(); - getContext().getSystem().stop(getSelf()); - } + responseObserver.onNext(actorEOFResponse.getResponse()); + actorsMap.remove(actorEOFResponse.getUniqueIdentifier()); + if (actorsMap.isEmpty()) { + responseObserver.onCompleted(); + getContext().getSystem().stop(getSelf()); } } diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Reducer.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Reducer.java deleted file mode 100644 index e444416a..00000000 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/Reducer.java +++ /dev/null @@ -1,30 +0,0 @@ -package io.numaproj.numaflow.reducestreamer; - - -/** - * Reducer exposes methods for performing reduce operation. - */ - - -public abstract class Reducer { - /** - * addMessage will be invoked for each input message. - * It can be used to read the input data from datum and - * update the result for given keys. - * - * @param keys message keys - * @param datum current message to be processed - * @param md metadata which contains window information - */ - public abstract void addMessage(String[] keys, Datum datum, Metadata md); - - /** - * getOutput will be invoked at the end of input. - * - * @param keys message keys - * @param md metadata which contains window information - * - * @return MessageList output value, aggregated result - */ - public abstract MessageList getOutput(String[] keys, Metadata md); -} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReducerFactory.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReducerFactory.java deleted file mode 100644 index 6f1060ed..00000000 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReducerFactory.java +++ /dev/null @@ -1,10 +0,0 @@ -package io.numaproj.numaflow.reducestreamer; - - -/** - * ReducerFactory is the factory for Reducer. - */ - -public abstract class ReducerFactory { - public abstract ReducerT createReducer(); -} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Server.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Server.java index 61ff542d..50c2248b 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/Server.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Server.java @@ -5,13 +5,15 @@ import io.grpc.ServerBuilder; import io.numaproj.numaflow.info.ServerInfoAccessor; import io.numaproj.numaflow.info.ServerInfoAccessorImpl; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; import io.numaproj.numaflow.shared.GrpcServerUtils; import lombok.extern.slf4j.Slf4j; import java.util.concurrent.TimeUnit; /** - * Server is the gRPC server for executing reduce operation. + * Server is the gRPC server for executing reduce stream operation. */ @Slf4j public class Server { @@ -24,20 +26,22 @@ public class Server { /** * constructor to create gRPC server. * - * @param reducerFactory to process the message + * @param reduceStreamerFactory to process the message */ - public Server(ReducerFactory reducerFactory) { - this(reducerFactory, GRPCConfig.defaultGrpcConfig()); + public Server(ReduceStreamerFactory reduceStreamerFactory) { + this(reduceStreamerFactory, GRPCConfig.defaultGrpcConfig()); } /** * constructor to create gRPC server with gRPC config. * * @param grpcConfig to configure the max message size for grpc - * @param reducerFactory to process the message + * @param reduceStreamerFactory to process the message */ - public Server(ReducerFactory reducerFactory, GRPCConfig grpcConfig) { - this.service = new Service(reducerFactory); + public Server( + ReduceStreamerFactory reduceStreamerFactory, + GRPCConfig grpcConfig) { + this.service = new Service(reduceStreamerFactory); this.grpcConfig = grpcConfig; } diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java index fc9cbf67..16f92d75 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java @@ -8,8 +8,12 @@ import io.grpc.stub.StreamObserver; import io.numaproj.numaflow.reduce.v1.ReduceGrpc; import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; -import io.numaproj.numaflow.reducestreamer.metadata.IntervalWindowImpl; -import io.numaproj.numaflow.reducestreamer.metadata.MetadataImpl; +import io.numaproj.numaflow.reducestreamer.model.IntervalWindow; +import io.numaproj.numaflow.reducestreamer.model.IntervalWindowImpl; +import io.numaproj.numaflow.reducestreamer.model.Metadata; +import io.numaproj.numaflow.reducestreamer.model.MetadataImpl; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; import io.numaproj.numaflow.shared.GrpcServerUtils; import lombok.extern.slf4j.Slf4j; @@ -21,12 +25,12 @@ @Slf4j class Service extends ReduceGrpc.ReduceImplBase { - public static final ActorSystem reduceActorSystem = ActorSystem.create("reduce"); + public static final ActorSystem reduceActorSystem = ActorSystem.create("reducestream"); - private ReducerFactory reducerFactory; + private ReduceStreamerFactory reduceStreamerFactory; - public Service(ReducerFactory reducerFactory) { - this.reducerFactory = reducerFactory; + public Service(ReduceStreamerFactory reduceStreamerFactory) { + this.reduceStreamerFactory = reduceStreamerFactory; } static void handleFailure( @@ -49,7 +53,7 @@ static void handleFailure( @Override public StreamObserver reduceFn(final StreamObserver responseObserver) { - if (this.reducerFactory == null) { + if (this.reduceStreamerFactory == null) { return io.grpc.stub.ServerCalls.asyncUnimplementedStreamingCall( getReduceFnMethod(), responseObserver); @@ -83,7 +87,7 @@ public StreamObserver reduceFn(final StreamObser */ ActorRef supervisorActor = reduceActorSystem .actorOf(ReduceSupervisorActor.props( - reducerFactory, + reduceStreamerFactory, md, shutdownActorRef, responseObserver)); diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Datum.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/Datum.java similarity index 76% rename from src/main/java/io/numaproj/numaflow/reducestreamer/Datum.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/model/Datum.java index 75bf9c31..491d7619 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/Datum.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/Datum.java @@ -1,4 +1,4 @@ -package io.numaproj.numaflow.reducestreamer; +package io.numaproj.numaflow.reducestreamer.model; import java.time.Instant; @@ -12,19 +12,19 @@ public interface Datum { * * @return returns the payload value in byte array */ - public byte[] getValue(); + byte[] getValue(); /** * method to get the event time of the payload * * @return returns the event time of the payload */ - public Instant getEventTime(); + Instant getEventTime(); /** * method to get the watermark information * * @return returns the watermark */ - public Instant getWatermark(); + Instant getWatermark(); } diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/HandlerDatum.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/HandlerDatum.java similarity index 81% rename from src/main/java/io/numaproj/numaflow/reducestreamer/HandlerDatum.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/model/HandlerDatum.java index 45333a03..55c281c2 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/HandlerDatum.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/HandlerDatum.java @@ -1,4 +1,4 @@ -package io.numaproj.numaflow.reducestreamer; +package io.numaproj.numaflow.reducestreamer.model; import lombok.AllArgsConstructor; @@ -6,7 +6,7 @@ import java.time.Instant; @AllArgsConstructor -class HandlerDatum implements Datum { +public class HandlerDatum implements Datum { private byte[] value; private Instant watermark; private Instant eventTime; diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/IntervalWindow.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/IntervalWindow.java similarity index 89% rename from src/main/java/io/numaproj/numaflow/reducestreamer/IntervalWindow.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/model/IntervalWindow.java index 2839cc31..4068d8a6 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/IntervalWindow.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/IntervalWindow.java @@ -1,4 +1,4 @@ -package io.numaproj.numaflow.reducestreamer; +package io.numaproj.numaflow.reducestreamer.model; import java.time.Instant; diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/metadata/IntervalWindowImpl.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/IntervalWindowImpl.java similarity index 81% rename from src/main/java/io/numaproj/numaflow/reducestreamer/metadata/IntervalWindowImpl.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/model/IntervalWindowImpl.java index 0815d295..e9afcb9c 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/metadata/IntervalWindowImpl.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/IntervalWindowImpl.java @@ -1,6 +1,5 @@ -package io.numaproj.numaflow.reducestreamer.metadata; +package io.numaproj.numaflow.reducestreamer.model; -import io.numaproj.numaflow.reducestreamer.IntervalWindow; import lombok.AllArgsConstructor; import java.time.Instant; diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Message.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/Message.java similarity index 96% rename from src/main/java/io/numaproj/numaflow/reducestreamer/Message.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/model/Message.java index c3c3e48c..b9669a69 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/Message.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/Message.java @@ -1,4 +1,4 @@ -package io.numaproj.numaflow.reducestreamer; +package io.numaproj.numaflow.reducestreamer.model; import lombok.Getter; diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/MessageList.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/MessageList.java similarity index 94% rename from src/main/java/io/numaproj/numaflow/reducestreamer/MessageList.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/model/MessageList.java index 7e000dac..5d418745 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/MessageList.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/MessageList.java @@ -1,4 +1,4 @@ -package io.numaproj.numaflow.reducestreamer; +package io.numaproj.numaflow.reducestreamer.model; import lombok.Builder; import lombok.Getter; diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Metadata.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/Metadata.java similarity index 84% rename from src/main/java/io/numaproj/numaflow/reducestreamer/Metadata.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/model/Metadata.java index f18be4b5..e59a61db 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/Metadata.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/Metadata.java @@ -1,4 +1,4 @@ -package io.numaproj.numaflow.reducestreamer; +package io.numaproj.numaflow.reducestreamer.model; /** * Metadata contains methods to get the metadata for the reduce operation. diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/metadata/MetadataImpl.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/MetadataImpl.java similarity index 67% rename from src/main/java/io/numaproj/numaflow/reducestreamer/metadata/MetadataImpl.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/model/MetadataImpl.java index 19b9bf71..2b57725f 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/metadata/MetadataImpl.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/MetadataImpl.java @@ -1,7 +1,5 @@ -package io.numaproj.numaflow.reducestreamer.metadata; +package io.numaproj.numaflow.reducestreamer.model; -import io.numaproj.numaflow.reducestreamer.IntervalWindow; -import io.numaproj.numaflow.reducestreamer.Metadata; import lombok.AllArgsConstructor; /** diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserver.java b/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserver.java new file mode 100644 index 00000000..24c3135a --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserver.java @@ -0,0 +1,15 @@ +package io.numaproj.numaflow.reducestreamer.user; + +import io.numaproj.numaflow.reducestreamer.model.Message; + +/** + * OutputObserver receives messages from the MapStreamer. + */ +public interface OutputStreamObserver { + /** + * method will be used for sending messages to the output. + * + * @param message the message to be sent + */ + void send(Message message); +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserverImpl.java b/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserverImpl.java new file mode 100644 index 00000000..f795ff31 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserverImpl.java @@ -0,0 +1,52 @@ +package io.numaproj.numaflow.reducestreamer.user; + +import com.google.protobuf.ByteString; +import com.google.protobuf.Timestamp; +import io.grpc.stub.StreamObserver; +import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; +import io.numaproj.numaflow.reduce.v1.ReduceOuterClass.ReduceResponse; +import io.numaproj.numaflow.reducestreamer.model.Message; +import io.numaproj.numaflow.reducestreamer.model.Metadata; +import lombok.AllArgsConstructor; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +@AllArgsConstructor +public +class OutputStreamObserverImpl implements OutputStreamObserver { + Metadata md; + StreamObserver responseObserver; + + @Override + public void send(Message message) { + ReduceResponse response = buildResponse(message); + responseObserver.onNext(response); + } + + private ReduceResponse buildResponse(Message message) { + ReduceOuterClass.ReduceResponse.Builder responseBuilder = ReduceOuterClass.ReduceResponse.newBuilder(); + // set the window using the actor metadata. + responseBuilder.setWindow(ReduceOuterClass.Window.newBuilder() + .setStart(Timestamp.newBuilder() + .setSeconds(this.md.getIntervalWindow().getStartTime().getEpochSecond()) + .setNanos(this.md.getIntervalWindow().getStartTime().getNano())) + .setEnd(Timestamp.newBuilder() + .setSeconds(this.md.getIntervalWindow().getEndTime().getEpochSecond()) + .setNanos(this.md.getIntervalWindow().getEndTime().getNano())) + .setSlot("slot-0").build()); + responseBuilder.setEOF(false); + // set the result. + responseBuilder.setResult(ReduceOuterClass.ReduceResponse.Result + .newBuilder() + .setValue(ByteString.copyFrom(message.getValue())) + .addAllKeys(message.getKeys() + == null ? new ArrayList<>():Arrays.asList(message.getKeys())) + .addAllTags( + message.getTags() == null ? new ArrayList<>():List.of(message.getTags())) + .build()); + + return responseBuilder.build(); + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamer.java b/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamer.java new file mode 100644 index 00000000..7be0351a --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamer.java @@ -0,0 +1,15 @@ +package io.numaproj.numaflow.reducestreamer.user; + +import io.numaproj.numaflow.reducestreamer.model.Datum; +import io.numaproj.numaflow.reducestreamer.model.Metadata; + +/** + * TODO - add descriptions + */ +public abstract class ReduceStreamer { + public abstract void processMessage( + String[] keys, + Datum datum, + OutputStreamObserver outputStream, + Metadata md); +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamerFactory.java b/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamerFactory.java new file mode 100644 index 00000000..d433142b --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamerFactory.java @@ -0,0 +1,12 @@ +package io.numaproj.numaflow.reducestreamer.user; + + +/** + * ReducerFactory is the factory for Reducer. + *

+ * TODO - do we need this? + */ + +public abstract class ReduceStreamerFactory { + public abstract ReduceStreamerT createReduceStreamer(); +} From 13a33b6367178ec53470459d8d50681b920af264 Mon Sep 17 00:00:00 2001 From: Keran Yang Date: Mon, 15 Jan 2024 15:10:54 -0500 Subject: [PATCH 03/11] add server unit tests Signed-off-by: Keran Yang --- .../reducestreamer/GRPCConfigTest.java | 39 +++ .../ReduceOutputStreamObserver.java | 38 +++ .../reducestreamer/ServerErrTest.java | 171 +++++++++++++ .../numaflow/reducestreamer/ServerTest.java | 224 ++++++++++++++++++ 4 files changed, 472 insertions(+) create mode 100644 src/test/java/io/numaproj/numaflow/reducestreamer/GRPCConfigTest.java create mode 100644 src/test/java/io/numaproj/numaflow/reducestreamer/ReduceOutputStreamObserver.java create mode 100644 src/test/java/io/numaproj/numaflow/reducestreamer/ServerErrTest.java create mode 100644 src/test/java/io/numaproj/numaflow/reducestreamer/ServerTest.java diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/GRPCConfigTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/GRPCConfigTest.java new file mode 100644 index 00000000..a84a1539 --- /dev/null +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/GRPCConfigTest.java @@ -0,0 +1,39 @@ +package io.numaproj.numaflow.reducestreamer; + +import io.numaproj.numaflow.info.ServerInfoAccessor; +import org.junit.Assert; +import org.junit.Test; + +public class GRPCConfigTest { + + @Test + public void testDefaultGrpcConfig() { + GRPCConfig grpcConfig = GRPCConfig.defaultGrpcConfig(); + Assert.assertNotNull(grpcConfig); + Assert.assertEquals( + ServerInfoAccessor.DEFAULT_SERVER_INFO_FILE_PATH, + grpcConfig.getInfoFilePath()); + Assert.assertEquals( + io.numaproj.numaflow.reducestreamer.Constants.DEFAULT_MESSAGE_SIZE, + grpcConfig.getMaxMessageSize()); + Assert.assertEquals( + io.numaproj.numaflow.reducestreamer.Constants.DEFAULT_SOCKET_PATH, + grpcConfig.getSocketPath()); + } + + @Test + public void testNewBuilder() { + String socketPath = "test-socket-path"; + int maxMessageSize = 2000; + String infoFilePath = "test-info-file-path"; + GRPCConfig grpcConfig = GRPCConfig.newBuilder() + .socketPath(socketPath) + .maxMessageSize(maxMessageSize) + .infoFilePath(infoFilePath) + .build(); + Assert.assertNotNull(grpcConfig); + Assert.assertEquals(socketPath, grpcConfig.getSocketPath()); + Assert.assertEquals(maxMessageSize, grpcConfig.getMaxMessageSize()); + Assert.assertEquals(infoFilePath, grpcConfig.getInfoFilePath()); + } +} diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceOutputStreamObserver.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceOutputStreamObserver.java new file mode 100644 index 00000000..07b74bd4 --- /dev/null +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceOutputStreamObserver.java @@ -0,0 +1,38 @@ +package io.numaproj.numaflow.reducestreamer; + +import io.grpc.stub.StreamObserver; +import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; +import lombok.extern.slf4j.Slf4j; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; + +/** + * This is a dummy implementation of reduce output stream observer for testing purpose. + */ +@Slf4j +public class ReduceOutputStreamObserver implements StreamObserver { + public AtomicReference completed = new AtomicReference<>(false); + public AtomicReference> resultDatum = new AtomicReference<>( + new ArrayList<>()); + public Throwable t; + + @Override + public void onNext(ReduceOuterClass.ReduceResponse response) { + List receivedResponses = resultDatum.get(); + receivedResponses.add(response); + resultDatum.set(receivedResponses); + } + + @Override + public void onError(Throwable throwable) { + t = throwable; + } + + @Override + public void onCompleted() { + log.info("on completed executed"); + this.completed.set(true); + } +} diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ServerErrTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ServerErrTest.java new file mode 100644 index 00000000..e1d0f1b6 --- /dev/null +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ServerErrTest.java @@ -0,0 +1,171 @@ +package io.numaproj.numaflow.reducestreamer; + +import com.google.protobuf.ByteString; +import io.grpc.Context; +import io.grpc.Contexts; +import io.grpc.ManagedChannel; +import io.grpc.Metadata; +import io.grpc.ServerCall; +import io.grpc.ServerCallHandler; +import io.grpc.ServerInterceptor; +import io.grpc.inprocess.InProcessChannelBuilder; +import io.grpc.inprocess.InProcessServerBuilder; +import io.grpc.stub.MetadataUtils; +import io.grpc.stub.StreamObserver; +import io.grpc.testing.GrpcCleanupRule; +import io.numaproj.numaflow.reduce.v1.ReduceGrpc; +import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; +import io.numaproj.numaflow.reducestreamer.model.Datum; +import io.numaproj.numaflow.reducestreamer.user.OutputStreamObserver; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; +import io.numaproj.numaflow.shared.GrpcServerUtils; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +import java.util.concurrent.atomic.AtomicReference; + +import static io.numaproj.numaflow.shared.GrpcServerUtils.WIN_END_KEY; +import static io.numaproj.numaflow.shared.GrpcServerUtils.WIN_START_KEY; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +public class ServerErrTest { + public static final Metadata.Key DATUM_METADATA_WIN_START = Metadata.Key.of( + WIN_START_KEY, + Metadata.ASCII_STRING_MARSHALLER); + public static final Metadata.Key DATUM_METADATA_WIN_END = Metadata.Key.of( + WIN_END_KEY, + Metadata.ASCII_STRING_MARSHALLER); + @Rule + public final GrpcCleanupRule grpcCleanup = new GrpcCleanupRule(); + private Server server; + private ManagedChannel inProcessChannel; + + @Before + public void setUp() throws Exception { + ServerInterceptor interceptor = new ServerInterceptor() { + @Override + public ServerCall.Listener interceptCall( + ServerCall call, + Metadata headers, + ServerCallHandler next) { + final var context = + Context.current().withValues( + GrpcServerUtils.WINDOW_START_TIME, + headers.get(DATUM_METADATA_WIN_START), + GrpcServerUtils.WINDOW_END_TIME, + headers.get(DATUM_METADATA_WIN_END)); + return Contexts.interceptCall(context, call, headers, next); + } + }; + + String serverName = InProcessServerBuilder.generateName(); + + GRPCConfig grpcServerConfig = GRPCConfig.newBuilder() + .maxMessageSize(io.numaproj.numaflow.reducestreamer.Constants.DEFAULT_MESSAGE_SIZE) + .socketPath(io.numaproj.numaflow.reducestreamer.Constants.DEFAULT_SOCKET_PATH) + .infoFilePath("/tmp/numaflow-test-server-info)") + .build(); + + server = new Server( + new ReduceStreamerErrTestFactory(), + grpcServerConfig); + + server.setServerBuilder(InProcessServerBuilder.forName(serverName) + .intercept(interceptor) + .directExecutor()); + + server.start(); + + inProcessChannel = grpcCleanup.register(InProcessChannelBuilder + .forName(serverName) + .directExecutor() + .build()); + } + + @After + public void tearDown() throws Exception { + server.stop(); + } + + @Test + public void given_reducerThrows_when_serverRuns_then_outputStreamContainsThrowable() { + Metadata metadata = new Metadata(); + metadata.put(Metadata.Key.of(WIN_START_KEY, Metadata.ASCII_STRING_MARSHALLER), "60000"); + metadata.put(Metadata.Key.of(WIN_END_KEY, Metadata.ASCII_STRING_MARSHALLER), "120000"); + + // create an output stream observer + ReduceOutputStreamObserver outputStreamObserver = new ReduceOutputStreamObserver(); + // we need to maintain a reference to any exceptions thrown inside the thread, otherwise even if the assertion failed in the thread, + // the test can still succeed. + AtomicReference exceptionInThread = new AtomicReference<>(); + + Thread t = new Thread(() -> { + while (outputStreamObserver.t == null) { + try { + Thread.sleep(100); + } catch (InterruptedException e) { + exceptionInThread.set(e); + } + } + try { + assertEquals( + "UNKNOWN: java.lang.RuntimeException: unknown exception", + outputStreamObserver.t.getMessage()); + } catch (Throwable e) { + exceptionInThread.set(e); + } + }); + t.start(); + + StreamObserver inputStreamObserver = ReduceGrpc + .newStub(inProcessChannel) + .withInterceptors(MetadataUtils.newAttachHeadersInterceptor(metadata)) + .reduceFn(outputStreamObserver); + + for (int i = 1; i <= 10; i++) { + ReduceOuterClass.ReduceRequest reduceRequest = ReduceOuterClass.ReduceRequest + .newBuilder() + .setPayload(ReduceOuterClass.ReduceRequest.Payload + .newBuilder() + .addKeys("reduce-key") + .setValue(ByteString.copyFromUtf8(String.valueOf(i))) + .build()) + .build(); + inputStreamObserver.onNext(reduceRequest); + } + + inputStreamObserver.onCompleted(); + + try { + t.join(); + } catch (InterruptedException e) { + fail("Thread got interrupted before test assertion."); + } + // Fail the test if any exception caught in the thread + if (exceptionInThread.get() != null) { + fail("Assertion failed in the thread: " + exceptionInThread.get().getMessage()); + } + } + + public static class ReduceStreamerErrTestFactory extends ReduceStreamerFactory { + @Override + public TestReduceHandler createReduceStreamer() { + return new ServerErrTest.ReduceStreamerErrTestFactory.TestReduceHandler(); + } + + public static class TestReduceHandler extends ReduceStreamer { + @Override + public void processMessage( + String[] keys, + Datum datum, + OutputStreamObserver outputStream, + io.numaproj.numaflow.reducestreamer.model.Metadata md) { + throw new RuntimeException("unknown exception"); + } + } + } +} diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ServerTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ServerTest.java new file mode 100644 index 00000000..8872050f --- /dev/null +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ServerTest.java @@ -0,0 +1,224 @@ +package io.numaproj.numaflow.reducestreamer; + +import com.google.protobuf.ByteString; +import io.grpc.Context; +import io.grpc.Contexts; +import io.grpc.ManagedChannel; +import io.grpc.Metadata; +import io.grpc.ServerCall; +import io.grpc.ServerCallHandler; +import io.grpc.ServerInterceptor; +import io.grpc.inprocess.InProcessChannelBuilder; +import io.grpc.inprocess.InProcessServerBuilder; +import io.grpc.stub.MetadataUtils; +import io.grpc.stub.StreamObserver; +import io.grpc.testing.GrpcCleanupRule; +import io.numaproj.numaflow.reduce.v1.ReduceGrpc; +import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; +import io.numaproj.numaflow.reducestreamer.model.Datum; +import io.numaproj.numaflow.reducestreamer.model.Message; +import io.numaproj.numaflow.reducestreamer.user.OutputStreamObserver; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; +import io.numaproj.numaflow.shared.GrpcServerUtils; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +import java.util.Arrays; +import java.util.List; + +import static io.numaproj.numaflow.shared.GrpcServerUtils.WIN_END_KEY; +import static io.numaproj.numaflow.shared.GrpcServerUtils.WIN_START_KEY; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class ServerTest { + public static final Metadata.Key DATUM_METADATA_WIN_START = Metadata.Key.of( + WIN_START_KEY, + Metadata.ASCII_STRING_MARSHALLER); + public static final Metadata.Key DATUM_METADATA_WIN_END = Metadata.Key.of( + WIN_END_KEY, + Metadata.ASCII_STRING_MARSHALLER); + private final static String REDUCE_PROCESSED_KEY_SUFFIX = "-processed"; + @Rule + public final GrpcCleanupRule grpcCleanup = new GrpcCleanupRule(); + private io.numaproj.numaflow.reducestreamer.Server server; + private ManagedChannel inProcessChannel; + + @Before + public void setUp() throws Exception { + + ServerInterceptor interceptor = new ServerInterceptor() { + @Override + public ServerCall.Listener interceptCall( + ServerCall call, + Metadata headers, + ServerCallHandler next) { + final var context = + Context.current().withValues( + GrpcServerUtils.WINDOW_START_TIME, + headers.get(DATUM_METADATA_WIN_START), + GrpcServerUtils.WINDOW_END_TIME, + headers.get(DATUM_METADATA_WIN_END)); + return Contexts.interceptCall(context, call, headers, next); + } + }; + + String serverName = InProcessServerBuilder.generateName(); + + io.numaproj.numaflow.reducestreamer.GRPCConfig grpcServerConfig = GRPCConfig.newBuilder() + .maxMessageSize(io.numaproj.numaflow.reducestreamer.Constants.DEFAULT_MESSAGE_SIZE) + .socketPath(io.numaproj.numaflow.reducestreamer.Constants.DEFAULT_SOCKET_PATH) + .infoFilePath("/tmp/numaflow-test-server-info)") + .build(); + + server = new Server( + new ReduceStreamerTestFactory(), + grpcServerConfig); + + server.setServerBuilder(InProcessServerBuilder.forName(serverName) + .intercept(interceptor) + .directExecutor()); + + server.start(); + + inProcessChannel = grpcCleanup.register(InProcessChannelBuilder + .forName(serverName) + .directExecutor() + .build()); + } + + @After + public void tearDown() throws Exception { + server.stop(); + } + + @Test + public void given_inputReduceRequestsShareSameKey_when_serverStarts_then_allRequestsGetAggregatedToOneResponse() { + String reduceKey = "reduce-key"; + + Metadata metadata = new Metadata(); + metadata.put(Metadata.Key.of(WIN_START_KEY, Metadata.ASCII_STRING_MARSHALLER), "60000"); + metadata.put(Metadata.Key.of(WIN_END_KEY, Metadata.ASCII_STRING_MARSHALLER), "120000"); + + // create an output stream observer + io.numaproj.numaflow.reducer.ReduceOutputStreamObserver outputStreamObserver = new io.numaproj.numaflow.reducer.ReduceOutputStreamObserver(); + + StreamObserver inputStreamObserver = ReduceGrpc + .newStub(inProcessChannel) + .withInterceptors(MetadataUtils.newAttachHeadersInterceptor(metadata)) + .reduceFn(outputStreamObserver); + + for (int i = 1; i <= 10; i++) { + ReduceOuterClass.ReduceRequest request = ReduceOuterClass.ReduceRequest.newBuilder() + .setPayload(ReduceOuterClass.ReduceRequest.Payload + .newBuilder() + .setValue(ByteString.copyFromUtf8(String.valueOf(i))) + .addAllKeys(Arrays.asList(reduceKey)) + .build()) + .build(); + inputStreamObserver.onNext(request); + } + + inputStreamObserver.onCompleted(); + + String[] expectedKeys = new String[]{reduceKey + REDUCE_PROCESSED_KEY_SUFFIX}; + // sum of first 10 numbers 1 to 10 -> 55 + ByteString expectedValue = ByteString.copyFromUtf8(String.valueOf(55)); + while (!outputStreamObserver.completed.get()) ; + + // Expect 2 responses, one containing the aggregated data and the other indicating EOF. + assertEquals(2, outputStreamObserver.resultDatum.get().size()); + assertEquals( + expectedKeys, + outputStreamObserver.resultDatum + .get() + .get(0) + .getResult() + .getKeysList() + .toArray(new String[0])); + assertEquals( + expectedValue, + outputStreamObserver.resultDatum + .get() + .get(0) + .getResult() + .getValue()); + assertTrue(outputStreamObserver.resultDatum.get().get(1).getEOF()); + } + + @Test + public void given_inputReduceRequestsHaveDifferentKeySets_when_serverStarts_then_requestsGetAggregatedSeparately() { + String reduceKey = "reduce-key"; + int keyCount = 3; + + Metadata metadata = new Metadata(); + metadata.put(Metadata.Key.of(WIN_START_KEY, Metadata.ASCII_STRING_MARSHALLER), "60000"); + metadata.put(Metadata.Key.of(WIN_END_KEY, Metadata.ASCII_STRING_MARSHALLER), "120000"); + + // create an output stream observer + io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver outputStreamObserver = new ReduceOutputStreamObserver(); + + StreamObserver inputStreamObserver = ReduceGrpc + .newStub(inProcessChannel) + .withInterceptors(MetadataUtils.newAttachHeadersInterceptor(metadata)) + .reduceFn(outputStreamObserver); + + // send messages with keyCount different keys + for (int j = 0; j < keyCount; j++) { + for (int i = 1; i <= 10; i++) { + ReduceOuterClass.ReduceRequest request = ReduceOuterClass.ReduceRequest + .newBuilder() + .setPayload(ReduceOuterClass.ReduceRequest.Payload.newBuilder() + .addAllKeys(Arrays.asList(reduceKey + j)) + .setValue(ByteString.copyFromUtf8(String.valueOf(i))) + .build()) + .build(); + inputStreamObserver.onNext(request); + } + } + + inputStreamObserver.onCompleted(); + + // sum of first 10 numbers 1 to 10 -> 55 + ByteString expectedValue = ByteString.copyFromUtf8(String.valueOf(55)); + + while (!outputStreamObserver.completed.get()) ; + List result = outputStreamObserver.resultDatum.get(); + // the outputStreamObserver should have observed 2*keyCount responses, because for each key set, one response for the aggregated result, the other for EOF. + assertEquals(keyCount * 2, result.size()); + result.forEach(response -> { + assertTrue(response.getResult().getValue().equals(expectedValue) || response.getEOF()); + }); + } + + public static class ReduceStreamerTestFactory extends ReduceStreamerFactory { + @Override + public ServerTest.ReduceStreamerTestFactory.TestReduceHandler createReduceStreamer() { + return new ServerTest.ReduceStreamerTestFactory.TestReduceHandler(); + } + + public static class TestReduceHandler extends ReduceStreamer { + private int sum = 0; + + @Override + public void processMessage( + String[] keys, + Datum datum, + OutputStreamObserver outputStream, + io.numaproj.numaflow.reducestreamer.model.Metadata md) { + sum += Integer.parseInt(new String(datum.getValue())); + if (sum > 50) { + String[] updatedKeys = Arrays + .stream(keys) + .map(c -> c + "-processed") + .toArray(String[]::new); + Message message = new Message(String.valueOf(sum).getBytes(), updatedKeys); + outputStream.send(message); + } + } + } + } +} From 2174c7398bc35460f593f64ea400f3864461e2d1 Mon Sep 17 00:00:00 2001 From: Keran Yang Date: Mon, 15 Jan 2024 17:05:57 -0500 Subject: [PATCH 04/11] add an example Signed-off-by: Keran Yang --- examples/pom.xml | 17 +++++++++++ .../reducestreamer/sum/SumFactory.java | 19 ++++++++++++ .../reducestreamer/sum/SumFunction.java | 29 +++++++++++++++++++ 3 files changed, 65 insertions(+) create mode 100644 examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFactory.java create mode 100644 examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFunction.java diff --git a/examples/pom.xml b/examples/pom.xml index 8696e5d6..63df86f2 100644 --- a/examples/pom.xml +++ b/examples/pom.xml @@ -134,6 +134,23 @@ + + reduce-stream-sum + package + + dockerBuild + + + + + io.numaproj.numaflow.examples.reducestreamer.sum.SumFactory + + + + numaflow-java-examples/reduce-stream-sum + + + diff --git a/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFactory.java b/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFactory.java new file mode 100644 index 00000000..15907f50 --- /dev/null +++ b/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFactory.java @@ -0,0 +1,19 @@ +package io.numaproj.numaflow.examples.reducestreamer.sum; + +import io.numaproj.numaflow.reducestreamer.Server; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class SumFactory extends ReduceStreamerFactory { + + public static void main(String[] args) throws Exception { + log.info("sum udf was invoked"); + new Server(new SumFactory()).start(); + } + + @Override + public SumFunction createReduceStreamer() { + return new SumFunction(); + } +} diff --git a/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFunction.java b/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFunction.java new file mode 100644 index 00000000..9a3c7939 --- /dev/null +++ b/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFunction.java @@ -0,0 +1,29 @@ +package io.numaproj.numaflow.examples.reducestreamer.sum; + +import io.numaproj.numaflow.reducestreamer.model.Message; +import io.numaproj.numaflow.reducestreamer.user.OutputStreamObserver; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class SumFunction extends ReduceStreamer { + + private int sum = 0; + + @Override + public void processMessage( + String[] keys, + io.numaproj.numaflow.reducestreamer.model.Datum datum, + OutputStreamObserver outputStream, + io.numaproj.numaflow.reducestreamer.model.Metadata md) { + try { + sum += Integer.parseInt(new String(datum.getValue())); + } catch (NumberFormatException e) { + log.info("error while parsing integer - {}", e.getMessage()); + } + if (sum >= 100) { + outputStream.send(new Message(String.valueOf(sum).getBytes())); + sum = 0; + } + } +} From c2937a9016e8dea87acfc3ce8a2bcc5741d4773b Mon Sep 17 00:00:00 2001 From: Keran Yang Date: Wed, 17 Jan 2024 11:14:53 -0500 Subject: [PATCH 05/11] add handleEndOfStream method Signed-off-by: Keran Yang --- .../reducestreamer/sum/SumFunction.java | 18 +- .../reducestreamer/ActorEOFResponse.java | 6 +- .../numaflow/reducestreamer/ActorRequest.java | 4 +- .../reducestreamer/ReduceShutdownActor.java | 2 - .../reducestreamer/ReduceStreamerActor.java | 7 +- .../reducestreamer/ReduceSupervisorActor.java | 1 + .../user/OutputStreamObserver.java | 2 +- .../reducestreamer/user/ReduceStreamer.java | 26 ++- .../user/ReduceStreamerFactory.java | 4 +- .../ReduceSupervisorActorTest.java | 163 ++++++++++++++++++ .../reducestreamer/ServerErrTest.java | 16 +- .../numaflow/reducestreamer/ServerTest.java | 69 ++++++-- .../reducestreamer/ShutDownActorTest.java | 133 ++++++++++++++ 13 files changed, 417 insertions(+), 34 deletions(-) create mode 100644 src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java create mode 100644 src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java diff --git a/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFunction.java b/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFunction.java index 9a3c7939..4bd2ce57 100644 --- a/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFunction.java +++ b/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFunction.java @@ -1,10 +1,16 @@ package io.numaproj.numaflow.examples.reducestreamer.sum; import io.numaproj.numaflow.reducestreamer.model.Message; +import io.numaproj.numaflow.reducestreamer.model.Metadata; import io.numaproj.numaflow.reducestreamer.user.OutputStreamObserver; import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; import lombok.extern.slf4j.Slf4j; +/** + * SumFunction is a User Defined Reduce Stream Function example which sums up the values for the given keys + * and outputs the sum when the sum is greater than 100. + * When the input stream closes, the function outputs the sum no matter what value it holds. + */ @Slf4j public class SumFunction extends ReduceStreamer { @@ -14,7 +20,7 @@ public class SumFunction extends ReduceStreamer { public void processMessage( String[] keys, io.numaproj.numaflow.reducestreamer.model.Datum datum, - OutputStreamObserver outputStream, + OutputStreamObserver outputStreamObserver, io.numaproj.numaflow.reducestreamer.model.Metadata md) { try { sum += Integer.parseInt(new String(datum.getValue())); @@ -22,8 +28,16 @@ public void processMessage( log.info("error while parsing integer - {}", e.getMessage()); } if (sum >= 100) { - outputStream.send(new Message(String.valueOf(sum).getBytes())); + outputStreamObserver.send(new Message(String.valueOf(sum).getBytes())); sum = 0; } } + + @Override + public void handleEndOfStream( + String[] keys, + OutputStreamObserver outputStreamObserver, + Metadata md) { + outputStreamObserver.send(new Message(String.valueOf(sum).getBytes())); + } } diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorEOFResponse.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorEOFResponse.java index 48ca0622..c5436f2b 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorEOFResponse.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorEOFResponse.java @@ -5,8 +5,10 @@ import lombok.Getter; /** - * ActorResponse is to store the response from ReduceActors. - * TODO - not required. Remove + * ActorEOFResponse is to store the EOF signal from a ReduceStreamerActor. + * ReduceStreamerActor sends it back to the supervisor actor to indicate that + * the streamer actor itself has finished processing the data and is ready to be + * released. */ @Getter @AllArgsConstructor diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java index f8b31136..2d94a500 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java @@ -5,7 +5,9 @@ import lombok.Getter; /** - * ActorRequest is to store the request sent to ReduceStreamActors. + * ActorRequest is a wrapper of the gRpc input request. + * It is constructed by the service when service receives an input request and then sent to + * the supervisor actor. */ @Getter @AllArgsConstructor diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceShutdownActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceShutdownActor.java index cb8bf1bb..af25d5b7 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceShutdownActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceShutdownActor.java @@ -13,8 +13,6 @@ /** * Shutdown actor, listens to exceptions and handles shutdown. */ - - @Slf4j @AllArgsConstructor class ReduceShutdownActor extends AbstractActor { diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java index fbd6ee8d..6d9770d6 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java @@ -17,9 +17,11 @@ import java.util.List; /** - * Reduce stream actor invokes the reducer and returns the result. + * Reduce stream actor invokes user defined functions to handle reduce request. + * When receiving an input request, it invokes the processMessage to handle the datum. + * When receiving an EOF signal from the supervisor, it invokes the handleEndOfStream to execute + * the user-defined end of stream processing logics. */ - @Slf4j @AllArgsConstructor public class ReduceStreamerActor extends AbstractActor { @@ -53,6 +55,7 @@ private void invokeHandler(HandlerDatum handlerDatum) { } private void sendEOF(String EOF) { + this.groupBy.handleEndOfStream(keys, outputStream, md); getSender().tell(buildEOFResponse(), getSelf()); } diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java index 89dfea6a..bacd3aec 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java @@ -98,6 +98,7 @@ private void invokeActors(ActorRequest actorRequest) { String uniqueId = actorRequest.getUniqueIdentifier(); if (!actorsMap.containsKey(uniqueId)) { ReduceStreamer reduceStreamerHandler = reduceStreamerFactory.createReduceStreamer(); + // FIXME - the responseObserver is NOT thread-safe but multiple actors are sharing it. ActorRef actorRef = getContext() .actorOf(ReduceStreamerActor.props( keys, diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserver.java b/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserver.java index 24c3135a..433ae8c6 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserver.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserver.java @@ -3,7 +3,7 @@ import io.numaproj.numaflow.reducestreamer.model.Message; /** - * OutputObserver receives messages from the MapStreamer. + * OutputObserver receives messages from the ReduceStreamer. */ public interface OutputStreamObserver { /** diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamer.java b/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamer.java index 7be0351a..dde4ff34 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamer.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamer.java @@ -4,12 +4,36 @@ import io.numaproj.numaflow.reducestreamer.model.Metadata; /** - * TODO - add descriptions + * ReduceStreamer exposes methods for performing reduce streaming operations. */ public abstract class ReduceStreamer { + /** + * processMessage is invoked for each reduce input message. + * It reads the input data from the datum and performs reduce operations for the given keys. + * An output stream is provided for sending back the result to the reduce output stream. + * + * @param keys message keys + * @param datum current message to be processed + * @param outputStream observer of the reduce result, which is used to send back reduce responses + * @param md metadata associated with the window + */ public abstract void processMessage( String[] keys, Datum datum, OutputStreamObserver outputStream, Metadata md); + + /** + * handleEndOfStream handles the closure of the reduce input stream. + * This method is invoked when the input reduce stream is closed. + * It provides the capability of constructing a final response based on the messages processed so far. + * + * @param keys message keys + * @param outputStreamObserver observer of the reduce result, which is used to send back reduce responses + * @param md metadata associated with the window + */ + public abstract void handleEndOfStream( + String[] keys, + OutputStreamObserver outputStreamObserver, + Metadata md); } diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamerFactory.java b/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamerFactory.java index d433142b..116d67b3 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamerFactory.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamerFactory.java @@ -2,9 +2,7 @@ /** - * ReducerFactory is the factory for Reducer. - *

- * TODO - do we need this? + * ReduceStreamerFactory is the factory for ReduceStreamer. */ public abstract class ReduceStreamerFactory { diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java new file mode 100644 index 00000000..20ed4fa0 --- /dev/null +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java @@ -0,0 +1,163 @@ +package io.numaproj.numaflow.reducestreamer; + +import akka.actor.ActorRef; +import akka.actor.ActorSystem; +import com.google.protobuf.ByteString; +import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; +import io.numaproj.numaflow.reducestreamer.model.Datum; +import io.numaproj.numaflow.reducestreamer.model.IntervalWindowImpl; +import io.numaproj.numaflow.reducestreamer.model.Message; +import io.numaproj.numaflow.reducestreamer.model.Metadata; +import io.numaproj.numaflow.reducestreamer.model.MetadataImpl; +import io.numaproj.numaflow.reducestreamer.user.OutputStreamObserver; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; +import org.junit.Test; + +import java.time.Instant; +import java.util.Arrays; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class ReduceSupervisorActorTest { + @Test + public void given_inputRequestsShareSameKeys_when_supervisorActorBroadcasts_then_onlyOneReducerActorGetsCreatedAndAggregatesAllRequests() throws RuntimeException { + final ActorSystem actorSystem = ActorSystem.create("test-system-1"); + CompletableFuture completableFuture = new CompletableFuture(); + + ActorRef shutdownActor = actorSystem + .actorOf(io.numaproj.numaflow.reducestreamer.ReduceShutdownActor + .props(completableFuture)); + + Metadata md = new MetadataImpl( + new IntervalWindowImpl(Instant.now(), Instant.now())); + + io.numaproj.numaflow.reducer.ReduceOutputStreamObserver outputStreamObserver = new io.numaproj.numaflow.reducer.ReduceOutputStreamObserver(); + + ActorRef supervisor = actorSystem + .actorOf(io.numaproj.numaflow.reducestreamer.ReduceSupervisorActor + .props( + new TestReduceStreamerFactory(), + md, + shutdownActor, + outputStreamObserver)); + + for (int i = 1; i <= 10; i++) { + io.numaproj.numaflow.reducestreamer.ActorRequest reduceRequest = new io.numaproj.numaflow.reducestreamer.ActorRequest( + ReduceOuterClass.ReduceRequest + .newBuilder() + .setPayload(ReduceOuterClass.ReduceRequest.Payload + .newBuilder() + // all reduce requests share same set of keys. + .addAllKeys(Arrays.asList("key-1", "key-2")) + .setValue(ByteString.copyFromUtf8(String.valueOf(i))) + .build()) + .build()); + supervisor.tell(reduceRequest, ActorRef.noSender()); + } + supervisor.tell(io.numaproj.numaflow.reducestreamer.Constants.EOF, ActorRef.noSender()); + + try { + completableFuture.get(); + // the observer should receive 2 messages, one is the aggregated result, the other is the EOF response. + assertEquals(2, outputStreamObserver.resultDatum.get().size()); + assertEquals("10", outputStreamObserver.resultDatum + .get() + .get(0) + .getResult() + .getValue() + .toStringUtf8()); + assertEquals(true, outputStreamObserver.resultDatum + .get() + .get(1) + .getEOF()); + } catch (InterruptedException | ExecutionException e) { + fail("Expected the future to complete without exception"); + } + } + + @Test + public void given_inputRequestsHaveDifferentKeySets_when_supervisorActorBroadcasts_then_multipleReducerActorsHandleKeySetsSeparately() throws RuntimeException { + final ActorSystem actorSystem = ActorSystem.create("test-system-2"); + CompletableFuture completableFuture = new CompletableFuture(); + + ActorRef shutdownActor = actorSystem + .actorOf(io.numaproj.numaflow.reducestreamer.ReduceShutdownActor + .props(completableFuture)); + + Metadata md = new MetadataImpl( + new IntervalWindowImpl(Instant.now(), Instant.now())); + + io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver outputStreamObserver = new ReduceOutputStreamObserver(); + ActorRef supervisor = actorSystem + .actorOf(io.numaproj.numaflow.reducestreamer.ReduceSupervisorActor + .props( + new TestReduceStreamerFactory(), + md, + shutdownActor, + outputStreamObserver) + ); + + for (int i = 1; i <= 10; i++) { + io.numaproj.numaflow.reducestreamer.ActorRequest reduceRequest = new io.numaproj.numaflow.reducestreamer.ActorRequest( + ReduceOuterClass.ReduceRequest + .newBuilder() + .setPayload(ReduceOuterClass.ReduceRequest.Payload + .newBuilder() + // each request contain a unique set of keys. + .addAllKeys(Arrays.asList("shared-key", "unique-key-" + i)) + .setValue(ByteString.copyFromUtf8(String.valueOf(i))) + .build()) + .build()); + supervisor.tell(reduceRequest, ActorRef.noSender()); + } + + supervisor.tell(io.numaproj.numaflow.reducestreamer.Constants.EOF, ActorRef.noSender()); + try { + completableFuture.get(); + // each reduce request generates two reduce responses, one containing the data and the other one indicating EOF. + assertEquals(20, outputStreamObserver.resultDatum.get().size()); + for (int i = 0; i < 20; i++) { + ReduceOuterClass.ReduceResponse response = outputStreamObserver.resultDatum + .get() + .get(i); + assertTrue(response.getResult().getValue().toStringUtf8().equals("1") + || response.getEOF()); + } + } catch (InterruptedException | ExecutionException e) { + fail("Expected the future to complete without exception"); + } + } + + public static class TestReduceStreamerFactory extends ReduceStreamerFactory { + @Override + public TestReduceStreamerHandler createReduceStreamer() { + return new TestReduceStreamerHandler(); + } + + public static class TestReduceStreamerHandler extends ReduceStreamer { + int count = 0; + + @Override + public void processMessage( + String[] keys, + Datum datum, + OutputStreamObserver outputStream, + Metadata md) { + count += 1; + } + + @Override + public void handleEndOfStream( + String[] keys, + OutputStreamObserver outputStreamObserver, + Metadata md) { + outputStreamObserver.send(new Message(String.valueOf(count).getBytes())); + } + } + } +} diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ServerErrTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ServerErrTest.java index e1d0f1b6..dd14c531 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/ServerErrTest.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ServerErrTest.java @@ -151,13 +151,13 @@ public void given_reducerThrows_when_serverRuns_then_outputStreamContainsThrowab } } - public static class ReduceStreamerErrTestFactory extends ReduceStreamerFactory { + public static class ReduceStreamerErrTestFactory extends ReduceStreamerFactory { @Override - public TestReduceHandler createReduceStreamer() { - return new ServerErrTest.ReduceStreamerErrTestFactory.TestReduceHandler(); + public TestReduceStreamHandler createReduceStreamer() { + return new ServerErrTest.ReduceStreamerErrTestFactory.TestReduceStreamHandler(); } - public static class TestReduceHandler extends ReduceStreamer { + public static class TestReduceStreamHandler extends ReduceStreamer { @Override public void processMessage( String[] keys, @@ -166,6 +166,14 @@ public void processMessage( io.numaproj.numaflow.reducestreamer.model.Metadata md) { throw new RuntimeException("unknown exception"); } + + @Override + public void handleEndOfStream( + String[] keys, + OutputStreamObserver outputStreamObserver, + io.numaproj.numaflow.reducestreamer.model.Metadata md) { + + } } } } diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ServerTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ServerTest.java index 8872050f..988166dc 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/ServerTest.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ServerTest.java @@ -111,7 +111,7 @@ public void given_inputReduceRequestsShareSameKey_when_serverStarts_then_allRequ .withInterceptors(MetadataUtils.newAttachHeadersInterceptor(metadata)) .reduceFn(outputStreamObserver); - for (int i = 1; i <= 10; i++) { + for (int i = 1; i <= 11; i++) { ReduceOuterClass.ReduceRequest request = ReduceOuterClass.ReduceRequest.newBuilder() .setPayload(ReduceOuterClass.ReduceRequest.Payload .newBuilder() @@ -126,11 +126,13 @@ public void given_inputReduceRequestsShareSameKey_when_serverStarts_then_allRequ String[] expectedKeys = new String[]{reduceKey + REDUCE_PROCESSED_KEY_SUFFIX}; // sum of first 10 numbers 1 to 10 -> 55 - ByteString expectedValue = ByteString.copyFromUtf8(String.valueOf(55)); + ByteString expectedFirstResponse = ByteString.copyFromUtf8(String.valueOf(55)); + // after the sum reaches 55, the test reducer reset the sum, hence when EOF is sent from input stream, the sum is 11 and gets sent to output stream. + ByteString expectedSecondResponse = ByteString.copyFromUtf8(String.valueOf(11)); while (!outputStreamObserver.completed.get()) ; // Expect 2 responses, one containing the aggregated data and the other indicating EOF. - assertEquals(2, outputStreamObserver.resultDatum.get().size()); + assertEquals(3, outputStreamObserver.resultDatum.get().size()); assertEquals( expectedKeys, outputStreamObserver.resultDatum @@ -140,13 +142,28 @@ public void given_inputReduceRequestsShareSameKey_when_serverStarts_then_allRequ .getKeysList() .toArray(new String[0])); assertEquals( - expectedValue, + expectedFirstResponse, outputStreamObserver.resultDatum .get() .get(0) .getResult() .getValue()); - assertTrue(outputStreamObserver.resultDatum.get().get(1).getEOF()); + assertEquals( + expectedKeys, + outputStreamObserver.resultDatum + .get() + .get(1) + .getResult() + .getKeysList() + .toArray(new String[0])); + assertEquals( + expectedSecondResponse, + outputStreamObserver.resultDatum + .get() + .get(1) + .getResult() + .getValue()); + assertTrue(outputStreamObserver.resultDatum.get().get(2).getEOF()); } @Test @@ -168,7 +185,7 @@ public void given_inputReduceRequestsHaveDifferentKeySets_when_serverStarts_then // send messages with keyCount different keys for (int j = 0; j < keyCount; j++) { - for (int i = 1; i <= 10; i++) { + for (int i = 1; i <= 11; i++) { ReduceOuterClass.ReduceRequest request = ReduceOuterClass.ReduceRequest .newBuilder() .setPayload(ReduceOuterClass.ReduceRequest.Payload.newBuilder() @@ -183,31 +200,36 @@ public void given_inputReduceRequestsHaveDifferentKeySets_when_serverStarts_then inputStreamObserver.onCompleted(); // sum of first 10 numbers 1 to 10 -> 55 - ByteString expectedValue = ByteString.copyFromUtf8(String.valueOf(55)); + ByteString expectedFirstResponse = ByteString.copyFromUtf8(String.valueOf(55)); + // after the sum reaches 55, the test reducer reset the sum, hence when EOF is sent from input stream, the sum is 11 and gets sent to output stream. + ByteString expectedSecondResponse = ByteString.copyFromUtf8(String.valueOf(11)); while (!outputStreamObserver.completed.get()) ; List result = outputStreamObserver.resultDatum.get(); - // the outputStreamObserver should have observed 2*keyCount responses, because for each key set, one response for the aggregated result, the other for EOF. - assertEquals(keyCount * 2, result.size()); + // the outputStreamObserver should have observed 3*keyCount responses, 2 with real output sum data, one as EOF. + assertEquals(keyCount * 3, result.size()); result.forEach(response -> { - assertTrue(response.getResult().getValue().equals(expectedValue) || response.getEOF()); + assertTrue(response.getResult().getValue().equals(expectedFirstResponse) || + response.getResult().getValue().equals(expectedSecondResponse) + || response.getEOF()); + }); } - public static class ReduceStreamerTestFactory extends ReduceStreamerFactory { + public static class ReduceStreamerTestFactory extends ReduceStreamerFactory { @Override - public ServerTest.ReduceStreamerTestFactory.TestReduceHandler createReduceStreamer() { - return new ServerTest.ReduceStreamerTestFactory.TestReduceHandler(); + public ServerTest.ReduceStreamerTestFactory.TestReduceStreamHandler createReduceStreamer() { + return new ServerTest.ReduceStreamerTestFactory.TestReduceStreamHandler(); } - public static class TestReduceHandler extends ReduceStreamer { + public static class TestReduceStreamHandler extends ReduceStreamer { private int sum = 0; @Override public void processMessage( String[] keys, Datum datum, - OutputStreamObserver outputStream, + OutputStreamObserver outputStreamObserver, io.numaproj.numaflow.reducestreamer.model.Metadata md) { sum += Integer.parseInt(new String(datum.getValue())); if (sum > 50) { @@ -216,9 +238,24 @@ public void processMessage( .map(c -> c + "-processed") .toArray(String[]::new); Message message = new Message(String.valueOf(sum).getBytes(), updatedKeys); - outputStream.send(message); + outputStreamObserver.send(message); + // reset sum + sum = 0; } } + + @Override + public void handleEndOfStream( + String[] keys, + OutputStreamObserver outputStreamObserver, + io.numaproj.numaflow.reducestreamer.model.Metadata md) { + String[] updatedKeys = Arrays + .stream(keys) + .map(c -> c + "-processed") + .toArray(String[]::new); + Message message = new Message(String.valueOf(sum).getBytes(), updatedKeys); + outputStreamObserver.send(message); + } } } } diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java new file mode 100644 index 00000000..6056601c --- /dev/null +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java @@ -0,0 +1,133 @@ +package io.numaproj.numaflow.reducestreamer; + +import akka.actor.ActorRef; +import akka.actor.ActorSystem; +import akka.actor.AllDeadLetters; +import akka.actor.DeadLetter; +import com.google.protobuf.ByteString; +import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; +import io.numaproj.numaflow.reducestreamer.model.Datum; +import io.numaproj.numaflow.reducestreamer.model.IntervalWindowImpl; +import io.numaproj.numaflow.reducestreamer.model.Message; +import io.numaproj.numaflow.reducestreamer.model.Metadata; +import io.numaproj.numaflow.reducestreamer.model.MetadataImpl; +import io.numaproj.numaflow.reducestreamer.user.OutputStreamObserver; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; +import org.junit.Test; + +import java.time.Instant; +import java.util.concurrent.CompletableFuture; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + + +public class ShutDownActorTest { + @Test + public void testFailure() { + final ActorSystem actorSystem = ActorSystem.create("test-system-1"); + CompletableFuture completableFuture = new CompletableFuture<>(); + + String reduceKey = "reduce-key"; + ReduceOuterClass.ReduceRequest.Payload.Builder payloadBuilder = ReduceOuterClass.ReduceRequest.Payload + .newBuilder() + .addKeys(reduceKey); + + ActorRef shutdownActor = actorSystem + .actorOf(io.numaproj.numaflow.reducestreamer.ReduceShutdownActor + .props(completableFuture)); + + Metadata md = new MetadataImpl( + new IntervalWindowImpl(Instant.now(), Instant.now())); + + ActorRef supervisor = actorSystem + .actorOf(io.numaproj.numaflow.reducestreamer.ReduceSupervisorActor + .props( + new TestExceptionFactory(), + md, + shutdownActor, + new io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver())); + + io.numaproj.numaflow.reducestreamer.ActorRequest reduceRequest = new io.numaproj.numaflow.reducestreamer.ActorRequest( + ReduceOuterClass.ReduceRequest.newBuilder() + .setPayload(payloadBuilder + .addKeys("reduce-test") + .setValue(ByteString.copyFromUtf8(String.valueOf(1))) + .build()) + .build()); + supervisor.tell(reduceRequest, ActorRef.noSender()); + + try { + completableFuture.get(); + fail("Expected the future to complete with exception"); + } catch (Exception e) { + assertEquals(e.getMessage(), "java.lang.RuntimeException: UDF Failure"); + } + } + + @Test + public void testDeadLetterHandling() { + final ActorSystem actorSystem = ActorSystem.create("test-system-2"); + CompletableFuture completableFuture = new CompletableFuture<>(); + + ActorRef shutdownActor = actorSystem + .actorOf(io.numaproj.numaflow.reducestreamer.ReduceShutdownActor + .props(completableFuture)); + + actorSystem.eventStream().subscribe(shutdownActor, AllDeadLetters.class); + + Metadata md = new MetadataImpl( + new IntervalWindowImpl(Instant.now(), Instant.now())); + + ActorRef supervisor = actorSystem + .actorOf(io.numaproj.numaflow.reducestreamer.ReduceSupervisorActor + .props( + new TestExceptionFactory(), + md, + shutdownActor, + new ReduceOutputStreamObserver())); + + DeadLetter deadLetter = new DeadLetter("dead-letter", shutdownActor, supervisor); + supervisor.tell(deadLetter, ActorRef.noSender()); + + try { + completableFuture.get(); + fail("Expected the future to complete with exception"); + } catch (Exception e) { + assertEquals(e.getMessage(), "java.lang.Throwable: dead letters"); + } + } + + + public static class TestExceptionFactory extends ReduceStreamerFactory { + + @Override + public TestException createReduceStreamer() { + return new TestException(); + } + + public static class TestException extends ReduceStreamer { + + int count = 0; + + @Override + public void processMessage( + String[] keys, + Datum datum, + OutputStreamObserver outputStream, + Metadata md) { + count += 1; + throw new RuntimeException("UDF Failure"); + } + + @Override + public void handleEndOfStream( + String[] keys, + OutputStreamObserver outputStreamObserver, + Metadata md) { + outputStreamObserver.send(new Message(String.valueOf(count).getBytes())); + } + } + } +} From 461610a95d88882c5ecc1b42e56e222d2134c4bf Mon Sep 17 00:00:00 2001 From: Keran Yang Date: Wed, 17 Jan 2024 20:15:47 -0500 Subject: [PATCH 06/11] make it thread-safe Signed-off-by: Keran Yang --- .../numaflow/reducestreamer/ActorRequest.java | 2 +- ...torEOFResponse.java => ActorResponse.java} | 14 +-- .../reducestreamer/ActorResponseType.java | 8 ++ .../reducestreamer/ReduceStreamerActor.java | 13 +-- .../reducestreamer/ReduceSupervisorActor.java | 47 ++++++---- .../reducestreamer/ResponseStreamActor.java | 93 +++++++++++++++++++ .../numaflow/reducestreamer/Service.java | 8 +- .../user/OutputStreamObserverImpl.java | 42 +-------- .../reducestreamer/user/ReduceStreamer.java | 2 +- .../ReduceOutputStreamObserver.java | 2 +- .../ReduceSupervisorActorTest.java | 25 +++-- .../reducestreamer/ShutDownActorTest.java | 14 ++- 12 files changed, 185 insertions(+), 85 deletions(-) rename src/main/java/io/numaproj/numaflow/reducestreamer/{ActorEOFResponse.java => ActorResponse.java} (56%) create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponseType.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java index 2d94a500..06c10a8e 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java @@ -7,7 +7,7 @@ /** * ActorRequest is a wrapper of the gRpc input request. * It is constructed by the service when service receives an input request and then sent to - * the supervisor actor. + * the supervisor actor, to be distributed to reduce stream actors. */ @Getter @AllArgsConstructor diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorEOFResponse.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java similarity index 56% rename from src/main/java/io/numaproj/numaflow/reducestreamer/ActorEOFResponse.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java index c5436f2b..c1bf4e2d 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorEOFResponse.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java @@ -5,21 +5,23 @@ import lombok.Getter; /** - * ActorEOFResponse is to store the EOF signal from a ReduceStreamerActor. - * ReduceStreamerActor sends it back to the supervisor actor to indicate that - * the streamer actor itself has finished processing the data and is ready to be - * released. + * ActorResponse is for child actors to report back to the supervisor actor about the status of the data processing. + * It serves two purposes: + * 1. Send to the supervisor an EOF response, which is to be sent to the gRPC output stream. + * 2. Send to the supervisor a signal, indicating that the actor has finished all its processing work, + * and it's ready to be cleaned up by the supervisor actor. */ @Getter @AllArgsConstructor -class ActorEOFResponse { +class ActorResponse { ReduceOuterClass.ReduceResponse response; + ActorResponseType type; // TODO - do we need to include window information in the id? // for aligned reducer, there is always single window. // but at the same time, would like to be consistent with GO SDK implementation. // we will revisit this one later. - public String getUniqueIdentifier() { + public String getActorUniqueIdentifier() { return String.join( Constants.DELIMITER, this.getResponse().getResult().getKeysList().toArray(new String[0])); diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponseType.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponseType.java new file mode 100644 index 00000000..01b91481 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponseType.java @@ -0,0 +1,8 @@ +package io.numaproj.numaflow.reducestreamer; + +public enum ActorResponseType { + // EOF_RESPONSE indicates that the actor response contains an EOF reduce response without real data. + EOF_RESPONSE, + // READY_FOR_CLEAN_UP_SIGNAL indicates that the actor has finished sending responses and now ready to be cleaned up. + READY_FOR_CLEAN_UP_SIGNAL, +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java index 6d9770d6..12d4938d 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java @@ -1,10 +1,10 @@ package io.numaproj.numaflow.reducestreamer; import akka.actor.AbstractActor; +import akka.actor.ActorRef; import akka.actor.Props; import akka.japi.pf.ReceiveBuilder; import com.google.protobuf.Timestamp; -import io.grpc.stub.StreamObserver; import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; import io.numaproj.numaflow.reducestreamer.model.HandlerDatum; import io.numaproj.numaflow.reducestreamer.model.Metadata; @@ -31,14 +31,13 @@ public class ReduceStreamerActor extends AbstractActor { private OutputStreamObserver outputStream; public static Props props( - String[] keys, Metadata md, ReduceStreamer groupBy, - StreamObserver responseStreamObserver) { + String[] keys, Metadata md, ReduceStreamer groupBy, ActorRef responseStreamActor) { return Props.create( ReduceStreamerActor.class, keys, md, groupBy, - new OutputStreamObserverImpl(md, responseStreamObserver)); + new OutputStreamObserverImpl(responseStreamActor)); } @Override @@ -55,11 +54,13 @@ private void invokeHandler(HandlerDatum handlerDatum) { } private void sendEOF(String EOF) { + // constructing final responses based on the messages processed so far and sending them out. this.groupBy.handleEndOfStream(keys, outputStream, md); + // constructing an EOF response and sending it back to the supervisor actor. getSender().tell(buildEOFResponse(), getSelf()); } - private ActorEOFResponse buildEOFResponse() { + private ActorResponse buildEOFResponse() { ReduceOuterClass.ReduceResponse.Builder responseBuilder = ReduceOuterClass.ReduceResponse.newBuilder(); responseBuilder.setWindow(ReduceOuterClass.Window.newBuilder() .setStart(Timestamp.newBuilder() @@ -75,6 +76,6 @@ private ActorEOFResponse buildEOFResponse() { .newBuilder() .addAllKeys(List.of(this.keys)) .build()); - return new ActorEOFResponse(responseBuilder.build()); + return new ActorResponse(responseBuilder.build(), ActorResponseType.EOF_RESPONSE); } } diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java index bacd3aec..af407089 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java @@ -31,6 +31,7 @@ class ReduceSupervisorActor extends AbstractActor { private final ReduceStreamerFactory reduceStreamerFactory; private final Metadata md; private final ActorRef shutdownActor; + private final ActorRef responseStreamActor; private final StreamObserver responseObserver; private final Map actorsMap = new HashMap<>(); @@ -38,10 +39,12 @@ public ReduceSupervisorActor( ReduceStreamerFactory reduceStreamerFactory, Metadata md, ActorRef shutdownActor, + ActorRef responseStreamActor, StreamObserver responseObserver) { this.reduceStreamerFactory = reduceStreamerFactory; this.md = md; this.shutdownActor = shutdownActor; + this.responseStreamActor = responseStreamActor; this.responseObserver = responseObserver; } @@ -49,12 +52,14 @@ public static Props props( ReduceStreamerFactory reduceStreamerFactory, Metadata md, ActorRef shutdownActor, + ActorRef responseStreamActor, StreamObserver responseObserver) { return Props.create( ReduceSupervisorActor.class, reduceStreamerFactory, md, shutdownActor, + responseStreamActor, responseObserver); } @@ -84,13 +89,13 @@ public Receive createReceive() { .create() .match(ActorRequest.class, this::invokeActors) .match(String.class, this::sendEOF) - .match(ActorEOFResponse.class, this::eofResponseListener) + .match(ActorResponse.class, this::handleActorResponse) .build(); } /* - based on the keys of the input message invoke the right actor - if there is no actor for an incoming set of keys, create a new actor + based on the keys of the input message invoke the right reduce streamer actor + if there is no actor for an incoming set of keys, create a new reduce streamer actor track all the child actors using actors map */ private void invokeActors(ActorRequest actorRequest) { @@ -98,13 +103,12 @@ private void invokeActors(ActorRequest actorRequest) { String uniqueId = actorRequest.getUniqueIdentifier(); if (!actorsMap.containsKey(uniqueId)) { ReduceStreamer reduceStreamerHandler = reduceStreamerFactory.createReduceStreamer(); - // FIXME - the responseObserver is NOT thread-safe but multiple actors are sharing it. ActorRef actorRef = getContext() .actorOf(ReduceStreamerActor.props( keys, - md, + this.md, reduceStreamerHandler, - responseObserver)); + this.responseStreamActor)); actorsMap.put(uniqueId, actorRef); } @@ -118,19 +122,24 @@ private void sendEOF(String EOF) { } } - // listen to child actors for the result. - private void eofResponseListener(ActorEOFResponse actorEOFResponse) { - /* - send the result back to the client - remove the child entry from the map after getting result. - if there are no entries in the map, that means processing is - done we can close the stream. - */ - responseObserver.onNext(actorEOFResponse.getResponse()); - actorsMap.remove(actorEOFResponse.getUniqueIdentifier()); - if (actorsMap.isEmpty()) { - responseObserver.onCompleted(); - getContext().getSystem().stop(getSelf()); + private void handleActorResponse(ActorResponse actorResponse) { + if (actorResponse.getType() == ActorResponseType.EOF_RESPONSE) { + // forward the response to the response stream actor to send back to gRPC output stream. + this.responseStreamActor.tell(actorResponse, getSelf()); + } else if (actorResponse.getType() == ActorResponseType.READY_FOR_CLEAN_UP_SIGNAL) { + // the corresponding actor is ready to be cleaned up. + // remove the child entry from the map. + // if there are no entries in the map, that means processing is + // done we can close the entire stream. + actorsMap.remove(actorResponse.getActorUniqueIdentifier()); + if (actorsMap.isEmpty()) { + responseObserver.onCompleted(); + getContext().getSystem().stop(getSelf()); + } + } else { + throw new RuntimeException( + "Supervisor actor received an actor response with unsupported type: " + + actorResponse.getType()); } } diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java new file mode 100644 index 00000000..16190979 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java @@ -0,0 +1,93 @@ +package io.numaproj.numaflow.reducestreamer; + +import akka.actor.AbstractActor; +import akka.actor.Props; +import com.google.protobuf.ByteString; +import com.google.protobuf.Timestamp; +import io.grpc.stub.StreamObserver; +import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; +import io.numaproj.numaflow.reducestreamer.model.Message; +import io.numaproj.numaflow.reducestreamer.model.Metadata; +import lombok.AllArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +/** + * ResponseStreamActor is dedicated to ensure synchronized calls to the responseObserver onNext(). + * ALL the responses are sent to ResponseStreamActor before getting sent to output gRPC stream. + *

+ * More details about gRPC StreamObserver concurrency: https://grpc.github.io/grpc-java/javadoc/io/grpc/stub/StreamObserver.html + */ +@Slf4j +@AllArgsConstructor +public class ResponseStreamActor extends AbstractActor { + StreamObserver responseObserver; + Metadata md; + + public static Props props( + StreamObserver responseObserver, + Metadata md) { + return Props.create(ResponseStreamActor.class, responseObserver, md); + } + + @Override + public Receive createReceive() { + return receiveBuilder() + .match(Message.class, this::sendMessage) + .match(ActorResponse.class, this::sendEOF) + .build(); + } + + private void sendMessage(Message message) { + // Synchronized access to the output stream + synchronized (responseObserver) { + responseObserver.onNext(this.buildResponse(message)); + } + } + + private void sendEOF(ActorResponse actorResponse) { + if (actorResponse.getType() != ActorResponseType.EOF_RESPONSE) { + throw new RuntimeException( + "Unexpected behavior - Response Stream actor received a non-eof response. Response type is: " + + actorResponse.getType()); + } + // Synchronized access to the output stream + synchronized (responseObserver) { + responseObserver.onNext(actorResponse.getResponse()); + } + // After the EOF response gets sent to gRPC output stream, + // tell the supervisor that the actor is ready to be cleaned up. + getSender().tell( + new ActorResponse( + actorResponse.getResponse(), + ActorResponseType.READY_FOR_CLEAN_UP_SIGNAL), + getSelf()); + } + + private ReduceOuterClass.ReduceResponse buildResponse(Message message) { + ReduceOuterClass.ReduceResponse.Builder responseBuilder = ReduceOuterClass.ReduceResponse.newBuilder(); + // set the window using the actor metadata. + responseBuilder.setWindow(ReduceOuterClass.Window.newBuilder() + .setStart(Timestamp.newBuilder() + .setSeconds(this.md.getIntervalWindow().getStartTime().getEpochSecond()) + .setNanos(this.md.getIntervalWindow().getStartTime().getNano())) + .setEnd(Timestamp.newBuilder() + .setSeconds(this.md.getIntervalWindow().getEndTime().getEpochSecond()) + .setNanos(this.md.getIntervalWindow().getEndTime().getNano())) + .setSlot("slot-0").build()); + responseBuilder.setEOF(false); + // set the result. + responseBuilder.setResult(ReduceOuterClass.ReduceResponse.Result + .newBuilder() + .setValue(ByteString.copyFrom(message.getValue())) + .addAllKeys(message.getKeys() + == null ? new ArrayList<>():Arrays.asList(message.getKeys())) + .addAllTags( + message.getTags() == null ? new ArrayList<>():List.of(message.getTags())) + .build()); + return responseBuilder.build(); + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java index 16f92d75..f84dbdec 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java @@ -52,7 +52,6 @@ static void handleFailure( */ @Override public StreamObserver reduceFn(final StreamObserver responseObserver) { - if (this.reduceStreamerFactory == null) { return io.grpc.stub.ServerCalls.asyncUnimplementedStreamingCall( getReduceFnMethod(), @@ -81,15 +80,20 @@ public StreamObserver reduceFn(final StreamObser reduceActorSystem.getEventStream().subscribe(shutdownActorRef, AllDeadLetters.class); handleFailure(failureFuture, responseObserver); + + // create a response stream actor that ensures synchronized delivery of reduce responses. + ActorRef responseStreamActor = reduceActorSystem. + actorOf(ResponseStreamActor.props(responseObserver, md)); /* create a supervisor actor which assign the tasks to child actors. - we create a child actor for every unique set of keys in a window + we create a child actor for every unique set of keys in a window. */ ActorRef supervisorActor = reduceActorSystem .actorOf(ReduceSupervisorActor.props( reduceStreamerFactory, md, shutdownActorRef, + responseStreamActor, responseObserver)); diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserverImpl.java b/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserverImpl.java index f795ff31..e1618506 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserverImpl.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserverImpl.java @@ -1,52 +1,16 @@ package io.numaproj.numaflow.reducestreamer.user; -import com.google.protobuf.ByteString; -import com.google.protobuf.Timestamp; -import io.grpc.stub.StreamObserver; -import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; -import io.numaproj.numaflow.reduce.v1.ReduceOuterClass.ReduceResponse; +import akka.actor.ActorRef; import io.numaproj.numaflow.reducestreamer.model.Message; -import io.numaproj.numaflow.reducestreamer.model.Metadata; import lombok.AllArgsConstructor; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - @AllArgsConstructor public class OutputStreamObserverImpl implements OutputStreamObserver { - Metadata md; - StreamObserver responseObserver; + private final ActorRef responseStreamActor; @Override public void send(Message message) { - ReduceResponse response = buildResponse(message); - responseObserver.onNext(response); - } - - private ReduceResponse buildResponse(Message message) { - ReduceOuterClass.ReduceResponse.Builder responseBuilder = ReduceOuterClass.ReduceResponse.newBuilder(); - // set the window using the actor metadata. - responseBuilder.setWindow(ReduceOuterClass.Window.newBuilder() - .setStart(Timestamp.newBuilder() - .setSeconds(this.md.getIntervalWindow().getStartTime().getEpochSecond()) - .setNanos(this.md.getIntervalWindow().getStartTime().getNano())) - .setEnd(Timestamp.newBuilder() - .setSeconds(this.md.getIntervalWindow().getEndTime().getEpochSecond()) - .setNanos(this.md.getIntervalWindow().getEndTime().getNano())) - .setSlot("slot-0").build()); - responseBuilder.setEOF(false); - // set the result. - responseBuilder.setResult(ReduceOuterClass.ReduceResponse.Result - .newBuilder() - .setValue(ByteString.copyFrom(message.getValue())) - .addAllKeys(message.getKeys() - == null ? new ArrayList<>():Arrays.asList(message.getKeys())) - .addAllTags( - message.getTags() == null ? new ArrayList<>():List.of(message.getTags())) - .build()); - - return responseBuilder.build(); + this.responseStreamActor.tell(message, ActorRef.noSender()); } } diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamer.java b/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamer.java index dde4ff34..793140df 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamer.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamer.java @@ -26,7 +26,7 @@ public abstract void processMessage( /** * handleEndOfStream handles the closure of the reduce input stream. * This method is invoked when the input reduce stream is closed. - * It provides the capability of constructing a final response based on the messages processed so far. + * It provides the capability of constructing final responses based on the messages processed so far. * * @param keys message keys * @param outputStreamObserver observer of the reduce result, which is used to send back reduce responses diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceOutputStreamObserver.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceOutputStreamObserver.java index 07b74bd4..f7f56595 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceOutputStreamObserver.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceOutputStreamObserver.java @@ -19,7 +19,7 @@ public class ReduceOutputStreamObserver implements StreamObserver receivedResponses = resultDatum.get(); receivedResponses.add(response); resultDatum.set(receivedResponses); diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java index 20ed4fa0..f7ab49d0 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java @@ -36,7 +36,10 @@ public void given_inputRequestsShareSameKeys_when_supervisorActorBroadcasts_then Metadata md = new MetadataImpl( new IntervalWindowImpl(Instant.now(), Instant.now())); - io.numaproj.numaflow.reducer.ReduceOutputStreamObserver outputStreamObserver = new io.numaproj.numaflow.reducer.ReduceOutputStreamObserver(); + io.numaproj.numaflow.reducer.ReduceOutputStreamObserver reduceOutputStreamObserver = new io.numaproj.numaflow.reducer.ReduceOutputStreamObserver(); + + ActorRef responseStreamActor = actorSystem.actorOf(io.numaproj.numaflow.reducestreamer.ResponseStreamActor + .props(reduceOutputStreamObserver, md)); ActorRef supervisor = actorSystem .actorOf(io.numaproj.numaflow.reducestreamer.ReduceSupervisorActor @@ -44,7 +47,8 @@ public void given_inputRequestsShareSameKeys_when_supervisorActorBroadcasts_then new TestReduceStreamerFactory(), md, shutdownActor, - outputStreamObserver)); + responseStreamActor, + reduceOutputStreamObserver)); for (int i = 1; i <= 10; i++) { io.numaproj.numaflow.reducestreamer.ActorRequest reduceRequest = new io.numaproj.numaflow.reducestreamer.ActorRequest( @@ -64,14 +68,14 @@ public void given_inputRequestsShareSameKeys_when_supervisorActorBroadcasts_then try { completableFuture.get(); // the observer should receive 2 messages, one is the aggregated result, the other is the EOF response. - assertEquals(2, outputStreamObserver.resultDatum.get().size()); - assertEquals("10", outputStreamObserver.resultDatum + assertEquals(2, reduceOutputStreamObserver.resultDatum.get().size()); + assertEquals("10", reduceOutputStreamObserver.resultDatum .get() .get(0) .getResult() .getValue() .toStringUtf8()); - assertEquals(true, outputStreamObserver.resultDatum + assertEquals(true, reduceOutputStreamObserver.resultDatum .get() .get(1) .getEOF()); @@ -92,14 +96,17 @@ public void given_inputRequestsHaveDifferentKeySets_when_supervisorActorBroadcas Metadata md = new MetadataImpl( new IntervalWindowImpl(Instant.now(), Instant.now())); - io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver outputStreamObserver = new ReduceOutputStreamObserver(); + io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver reduceOutputStreamObserver = new ReduceOutputStreamObserver(); + ActorRef responseStreamActor = actorSystem.actorOf(io.numaproj.numaflow.reducestreamer.ResponseStreamActor + .props(reduceOutputStreamObserver, md)); ActorRef supervisor = actorSystem .actorOf(io.numaproj.numaflow.reducestreamer.ReduceSupervisorActor .props( new TestReduceStreamerFactory(), md, shutdownActor, - outputStreamObserver) + responseStreamActor, + reduceOutputStreamObserver) ); for (int i = 1; i <= 10; i++) { @@ -120,9 +127,9 @@ public void given_inputRequestsHaveDifferentKeySets_when_supervisorActorBroadcas try { completableFuture.get(); // each reduce request generates two reduce responses, one containing the data and the other one indicating EOF. - assertEquals(20, outputStreamObserver.resultDatum.get().size()); + assertEquals(20, reduceOutputStreamObserver.resultDatum.get().size()); for (int i = 0; i < 20; i++) { - ReduceOuterClass.ReduceResponse response = outputStreamObserver.resultDatum + ReduceOuterClass.ReduceResponse response = reduceOutputStreamObserver.resultDatum .get() .get(i); assertTrue(response.getResult().getValue().toStringUtf8().equals("1") diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java index 6056601c..60de6785 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java @@ -41,12 +41,18 @@ public void testFailure() { Metadata md = new MetadataImpl( new IntervalWindowImpl(Instant.now(), Instant.now())); + io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver reduceOutputStreamObserver = new io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver(); + + ActorRef responseStreamActor = actorSystem.actorOf(io.numaproj.numaflow.reducestreamer.ResponseStreamActor + .props(reduceOutputStreamObserver, md)); + ActorRef supervisor = actorSystem .actorOf(io.numaproj.numaflow.reducestreamer.ReduceSupervisorActor .props( new TestExceptionFactory(), md, shutdownActor, + responseStreamActor, new io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver())); io.numaproj.numaflow.reducestreamer.ActorRequest reduceRequest = new io.numaproj.numaflow.reducestreamer.ActorRequest( @@ -80,13 +86,19 @@ public void testDeadLetterHandling() { Metadata md = new MetadataImpl( new IntervalWindowImpl(Instant.now(), Instant.now())); + io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver reduceOutputStreamObserver = new io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver(); + + ActorRef responseStreamActor = actorSystem.actorOf(io.numaproj.numaflow.reducestreamer.ResponseStreamActor + .props(reduceOutputStreamObserver, md)); + ActorRef supervisor = actorSystem .actorOf(io.numaproj.numaflow.reducestreamer.ReduceSupervisorActor .props( new TestExceptionFactory(), md, shutdownActor, - new ReduceOutputStreamObserver())); + responseStreamActor, + reduceOutputStreamObserver)); DeadLetter deadLetter = new DeadLetter("dead-letter", shutdownActor, supervisor); supervisor.tell(deadLetter, ActorRef.noSender()); From 49d25271ca88541d76394186c85f8952c8a1cafb Mon Sep 17 00:00:00 2001 From: Keran Yang Date: Fri, 19 Jan 2024 15:09:45 -0500 Subject: [PATCH 07/11] a bit of clean up Signed-off-by: Keran Yang --- .../reducestreamer/sum/SumFactory.java | 2 +- .../reducestreamer/sum/SumFunction.java | 4 +- .../numaflow/reducestreamer/ActorRequest.java | 2 +- .../reducestreamer/ActorResponse.java | 2 +- .../reducestreamer/ActorResponseType.java | 2 +- .../{model => }/HandlerDatum.java | 6 +-- .../{model => }/IntervalWindowImpl.java | 9 ++--- .../numaflow/reducestreamer/MetadataImpl.java | 15 +++++++ .../{user => }/OutputStreamObserverImpl.java | 4 +- .../reducestreamer/ReduceShutdownActor.java | 2 +- .../reducestreamer/ReduceStreamerActor.java | 10 ++--- .../reducestreamer/ReduceSupervisorActor.java | 7 ++-- .../reducestreamer/ResponseStreamActor.java | 10 ++--- .../numaflow/reducestreamer/Server.java | 5 +-- .../numaflow/reducestreamer/Service.java | 7 +--- .../numaflow/reducestreamer/model/Datum.java | 1 - .../reducestreamer/model/Message.java | 2 - .../reducestreamer/model/MessageList.java | 39 ------------------- .../reducestreamer/model/MetadataImpl.java | 16 -------- .../model/OutputStreamObserver.java | 13 +++++++ .../{user => model}/ReduceStreamer.java | 5 +-- .../ReduceStreamerFactory.java | 4 +- .../user/OutputStreamObserver.java | 15 ------- .../ReduceSupervisorActorTest.java | 8 ++-- .../reducestreamer/ServerErrTest.java | 6 +-- .../numaflow/reducestreamer/ServerTest.java | 6 +-- .../reducestreamer/ShutDownActorTest.java | 8 ++-- 27 files changed, 73 insertions(+), 137 deletions(-) rename src/main/java/io/numaproj/numaflow/reducestreamer/{model => }/HandlerDatum.java (75%) rename src/main/java/io/numaproj/numaflow/reducestreamer/{model => }/IntervalWindowImpl.java (58%) create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/MetadataImpl.java rename src/main/java/io/numaproj/numaflow/reducestreamer/{user => }/OutputStreamObserverImpl.java (76%) delete mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/model/MessageList.java delete mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/model/MetadataImpl.java create mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/model/OutputStreamObserver.java rename src/main/java/io/numaproj/numaflow/reducestreamer/{user => model}/ReduceStreamer.java (89%) rename src/main/java/io/numaproj/numaflow/reducestreamer/{user => model}/ReduceStreamerFactory.java (80%) delete mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserver.java diff --git a/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFactory.java b/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFactory.java index 15907f50..1c6d3d5e 100644 --- a/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFactory.java +++ b/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFactory.java @@ -1,7 +1,7 @@ package io.numaproj.numaflow.examples.reducestreamer.sum; import io.numaproj.numaflow.reducestreamer.Server; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamerFactory; import lombok.extern.slf4j.Slf4j; @Slf4j diff --git a/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFunction.java b/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFunction.java index 4bd2ce57..77bfcba7 100644 --- a/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFunction.java +++ b/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFunction.java @@ -2,8 +2,8 @@ import io.numaproj.numaflow.reducestreamer.model.Message; import io.numaproj.numaflow.reducestreamer.model.Metadata; -import io.numaproj.numaflow.reducestreamer.user.OutputStreamObserver; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.model.OutputStreamObserver; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamer; import lombok.extern.slf4j.Slf4j; /** diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java index 06c10a8e..831bf667 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorRequest.java @@ -7,7 +7,7 @@ /** * ActorRequest is a wrapper of the gRpc input request. * It is constructed by the service when service receives an input request and then sent to - * the supervisor actor, to be distributed to reduce stream actors. + * the supervisor actor, to be distributed to reduce streamer actors. */ @Getter @AllArgsConstructor diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java index c1bf4e2d..895af306 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java @@ -5,7 +5,7 @@ import lombok.Getter; /** - * ActorResponse is for child actors to report back to the supervisor actor about the status of the data processing. + * ActorResponse is for child reduce streamer actors to report back to the supervisor actor about the status of the data processing. * It serves two purposes: * 1. Send to the supervisor an EOF response, which is to be sent to the gRPC output stream. * 2. Send to the supervisor a signal, indicating that the actor has finished all its processing work, diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponseType.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponseType.java index 01b91481..13da3ae1 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponseType.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponseType.java @@ -1,6 +1,6 @@ package io.numaproj.numaflow.reducestreamer; -public enum ActorResponseType { +enum ActorResponseType { // EOF_RESPONSE indicates that the actor response contains an EOF reduce response without real data. EOF_RESPONSE, // READY_FOR_CLEAN_UP_SIGNAL indicates that the actor has finished sending responses and now ready to be cleaned up. diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/model/HandlerDatum.java b/src/main/java/io/numaproj/numaflow/reducestreamer/HandlerDatum.java similarity index 75% rename from src/main/java/io/numaproj/numaflow/reducestreamer/model/HandlerDatum.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/HandlerDatum.java index 55c281c2..a36df0ce 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/model/HandlerDatum.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/HandlerDatum.java @@ -1,12 +1,12 @@ -package io.numaproj.numaflow.reducestreamer.model; - +package io.numaproj.numaflow.reducestreamer; +import io.numaproj.numaflow.reducestreamer.model.Datum; import lombok.AllArgsConstructor; import java.time.Instant; @AllArgsConstructor -public class HandlerDatum implements Datum { +class HandlerDatum implements Datum { private byte[] value; private Instant watermark; private Instant eventTime; diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/model/IntervalWindowImpl.java b/src/main/java/io/numaproj/numaflow/reducestreamer/IntervalWindowImpl.java similarity index 58% rename from src/main/java/io/numaproj/numaflow/reducestreamer/model/IntervalWindowImpl.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/IntervalWindowImpl.java index e9afcb9c..a4f75369 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/model/IntervalWindowImpl.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/IntervalWindowImpl.java @@ -1,15 +1,12 @@ -package io.numaproj.numaflow.reducestreamer.model; +package io.numaproj.numaflow.reducestreamer; +import io.numaproj.numaflow.reducestreamer.model.IntervalWindow; import lombok.AllArgsConstructor; import java.time.Instant; -/** - * IntervalWindowImpl implements IntervalWindow interface which will be passed as metadata to reduce - * handlers - */ @AllArgsConstructor -public class IntervalWindowImpl implements IntervalWindow { +class IntervalWindowImpl implements IntervalWindow { private final Instant startTime; private final Instant endTime; diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/MetadataImpl.java b/src/main/java/io/numaproj/numaflow/reducestreamer/MetadataImpl.java new file mode 100644 index 00000000..2e8f7da9 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/MetadataImpl.java @@ -0,0 +1,15 @@ +package io.numaproj.numaflow.reducestreamer; + +import io.numaproj.numaflow.reducestreamer.model.IntervalWindow; +import io.numaproj.numaflow.reducestreamer.model.Metadata; +import lombok.AllArgsConstructor; + +@AllArgsConstructor +class MetadataImpl implements Metadata { + private final IntervalWindow intervalWindow; + + @Override + public IntervalWindow getIntervalWindow() { + return intervalWindow; + } +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserverImpl.java b/src/main/java/io/numaproj/numaflow/reducestreamer/OutputStreamObserverImpl.java similarity index 76% rename from src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserverImpl.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/OutputStreamObserverImpl.java index e1618506..c843c80f 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserverImpl.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/OutputStreamObserverImpl.java @@ -1,11 +1,11 @@ -package io.numaproj.numaflow.reducestreamer.user; +package io.numaproj.numaflow.reducestreamer; import akka.actor.ActorRef; import io.numaproj.numaflow.reducestreamer.model.Message; +import io.numaproj.numaflow.reducestreamer.model.OutputStreamObserver; import lombok.AllArgsConstructor; @AllArgsConstructor -public class OutputStreamObserverImpl implements OutputStreamObserver { private final ActorRef responseStreamActor; diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceShutdownActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceShutdownActor.java index af25d5b7..010c0862 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceShutdownActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceShutdownActor.java @@ -11,7 +11,7 @@ import java.util.concurrent.CompletableFuture; /** - * Shutdown actor, listens to exceptions and handles shutdown. + * Reduce shutdown actor, listens to exceptions and handles shutdown. */ @Slf4j @AllArgsConstructor diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java index 12d4938d..5897829e 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java @@ -6,25 +6,23 @@ import akka.japi.pf.ReceiveBuilder; import com.google.protobuf.Timestamp; import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; -import io.numaproj.numaflow.reducestreamer.model.HandlerDatum; import io.numaproj.numaflow.reducestreamer.model.Metadata; -import io.numaproj.numaflow.reducestreamer.user.OutputStreamObserver; -import io.numaproj.numaflow.reducestreamer.user.OutputStreamObserverImpl; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.model.OutputStreamObserver; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamer; import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; import java.util.List; /** - * Reduce stream actor invokes user defined functions to handle reduce request. + * Reduce streamer actor invokes user defined functions to handle reduce requests. * When receiving an input request, it invokes the processMessage to handle the datum. * When receiving an EOF signal from the supervisor, it invokes the handleEndOfStream to execute * the user-defined end of stream processing logics. */ @Slf4j @AllArgsConstructor -public class ReduceStreamerActor extends AbstractActor { +class ReduceStreamerActor extends AbstractActor { private String[] keys; private Metadata md; private ReduceStreamer groupBy; diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java index af407089..4d2acf5e 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java @@ -10,10 +10,9 @@ import com.google.common.base.Preconditions; import io.grpc.stub.StreamObserver; import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; -import io.numaproj.numaflow.reducestreamer.model.HandlerDatum; import io.numaproj.numaflow.reducestreamer.model.Metadata; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamerFactory; import lombok.extern.slf4j.Slf4j; import scala.PartialFunction; import scala.collection.Iterable; @@ -24,7 +23,7 @@ import java.util.Optional; /** - * ReduceSupervisorActor actor distributes the messages to actors and handles failure. + * Reduce supervisor actor distributes the messages to other actors and handles failures. */ @Slf4j class ReduceSupervisorActor extends AbstractActor { diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java index 16190979..e661c10a 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java @@ -16,14 +16,14 @@ import java.util.List; /** - * ResponseStreamActor is dedicated to ensure synchronized calls to the responseObserver onNext(). - * ALL the responses are sent to ResponseStreamActor before getting sent to output gRPC stream. + * Response stream actor is dedicated to ensure synchronized calls to the responseObserver onNext(). + * ALL the responses are sent to the response stream actor before getting forwarded to the output gRPC stream. *

* More details about gRPC StreamObserver concurrency: https://grpc.github.io/grpc-java/javadoc/io/grpc/stub/StreamObserver.html */ @Slf4j @AllArgsConstructor -public class ResponseStreamActor extends AbstractActor { +class ResponseStreamActor extends AbstractActor { StreamObserver responseObserver; Metadata md; @@ -42,7 +42,7 @@ public Receive createReceive() { } private void sendMessage(Message message) { - // Synchronized access to the output stream + // Synchronized the access to the output stream synchronized (responseObserver) { responseObserver.onNext(this.buildResponse(message)); } @@ -54,7 +54,7 @@ private void sendEOF(ActorResponse actorResponse) { "Unexpected behavior - Response Stream actor received a non-eof response. Response type is: " + actorResponse.getType()); } - // Synchronized access to the output stream + // Synchronized the access to the output stream synchronized (responseObserver) { responseObserver.onNext(actorResponse.getResponse()); } diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Server.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Server.java index 50c2248b..cf21236f 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/Server.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Server.java @@ -5,8 +5,8 @@ import io.grpc.ServerBuilder; import io.numaproj.numaflow.info.ServerInfoAccessor; import io.numaproj.numaflow.info.ServerInfoAccessorImpl; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamerFactory; import io.numaproj.numaflow.shared.GrpcServerUtils; import lombok.extern.slf4j.Slf4j; @@ -17,7 +17,6 @@ */ @Slf4j public class Server { - private final GRPCConfig grpcConfig; private final Service service; private final ServerInfoAccessor serverInfoAccessor = new ServerInfoAccessorImpl(new ObjectMapper()); diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java index f84dbdec..b6486d7f 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java @@ -9,11 +9,9 @@ import io.numaproj.numaflow.reduce.v1.ReduceGrpc; import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; import io.numaproj.numaflow.reducestreamer.model.IntervalWindow; -import io.numaproj.numaflow.reducestreamer.model.IntervalWindowImpl; import io.numaproj.numaflow.reducestreamer.model.Metadata; -import io.numaproj.numaflow.reducestreamer.model.MetadataImpl; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamerFactory; import io.numaproj.numaflow.shared.GrpcServerUtils; import lombok.extern.slf4j.Slf4j; @@ -24,7 +22,6 @@ @Slf4j class Service extends ReduceGrpc.ReduceImplBase { - public static final ActorSystem reduceActorSystem = ActorSystem.create("reducestream"); private ReduceStreamerFactory reduceStreamerFactory; diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/model/Datum.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/Datum.java index 491d7619..a590a1ce 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/model/Datum.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/Datum.java @@ -5,7 +5,6 @@ /** * Datum contains methods to get the payload information. */ - public interface Datum { /** * method to get the payload value diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/model/Message.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/Message.java index b9669a69..88220cff 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/model/Message.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/Message.java @@ -5,11 +5,9 @@ /** * Message is used to wrap the data returned by Reducer functions. */ - @Getter public class Message { public static final String DROP = "U+005C__DROP__"; - private final String[] keys; private final byte[] value; private final String[] tags; diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/model/MessageList.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/MessageList.java deleted file mode 100644 index 5d418745..00000000 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/model/MessageList.java +++ /dev/null @@ -1,39 +0,0 @@ -package io.numaproj.numaflow.reducestreamer.model; - -import lombok.Builder; -import lombok.Getter; -import lombok.Singular; - -import java.util.ArrayList; -import java.util.Collection; - -/** - * MessageList is used to return the list of Messages returned from Reducer functions. - */ - -@Getter -@Builder(builderMethodName = "newBuilder") -public class MessageList { - - @Singular("addMessage") - private Iterable messages; - - /** - * Builder to build MessageList - */ - public static class MessageListBuilder { - /** - * @param messages to append all the messages to MessageList - * - * @return returns the builder - */ - public MessageListBuilder addMessages(Iterable messages) { - if (this.messages == null) { - this.messages = new ArrayList<>(); - return this; - } - this.messages.addAll((Collection) messages); - return this; - } - } -} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/model/MetadataImpl.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/MetadataImpl.java deleted file mode 100644 index 2b57725f..00000000 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/model/MetadataImpl.java +++ /dev/null @@ -1,16 +0,0 @@ -package io.numaproj.numaflow.reducestreamer.model; - -import lombok.AllArgsConstructor; - -/** - * MetadataImpl implements Metadata interface which will be passed to reduce handlers - */ -@AllArgsConstructor -public class MetadataImpl implements Metadata { - private final IntervalWindow intervalWindow; - - @Override - public IntervalWindow getIntervalWindow() { - return intervalWindow; - } -} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/model/OutputStreamObserver.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/OutputStreamObserver.java new file mode 100644 index 00000000..a0348561 --- /dev/null +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/OutputStreamObserver.java @@ -0,0 +1,13 @@ +package io.numaproj.numaflow.reducestreamer.model; + +/** + * OutputStreamObserver sends to the output stream, the messages generate by the reducer. + */ +public interface OutputStreamObserver { + /** + * method will be used for sending messages to the output stream. + * + * @param message the message to be sent + */ + void send(Message message); +} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamer.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/ReduceStreamer.java similarity index 89% rename from src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamer.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/model/ReduceStreamer.java index 793140df..c4032ad0 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamer.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/ReduceStreamer.java @@ -1,7 +1,4 @@ -package io.numaproj.numaflow.reducestreamer.user; - -import io.numaproj.numaflow.reducestreamer.model.Datum; -import io.numaproj.numaflow.reducestreamer.model.Metadata; +package io.numaproj.numaflow.reducestreamer.model; /** * ReduceStreamer exposes methods for performing reduce streaming operations. diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamerFactory.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/ReduceStreamerFactory.java similarity index 80% rename from src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamerFactory.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/model/ReduceStreamerFactory.java index 116d67b3..9a66bace 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/user/ReduceStreamerFactory.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/ReduceStreamerFactory.java @@ -1,10 +1,8 @@ -package io.numaproj.numaflow.reducestreamer.user; - +package io.numaproj.numaflow.reducestreamer.model; /** * ReduceStreamerFactory is the factory for ReduceStreamer. */ - public abstract class ReduceStreamerFactory { public abstract ReduceStreamerT createReduceStreamer(); } diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserver.java b/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserver.java deleted file mode 100644 index 433ae8c6..00000000 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/user/OutputStreamObserver.java +++ /dev/null @@ -1,15 +0,0 @@ -package io.numaproj.numaflow.reducestreamer.user; - -import io.numaproj.numaflow.reducestreamer.model.Message; - -/** - * OutputObserver receives messages from the ReduceStreamer. - */ -public interface OutputStreamObserver { - /** - * method will be used for sending messages to the output. - * - * @param message the message to be sent - */ - void send(Message message); -} diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java index f7ab49d0..13148b7c 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java @@ -5,13 +5,11 @@ import com.google.protobuf.ByteString; import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; import io.numaproj.numaflow.reducestreamer.model.Datum; -import io.numaproj.numaflow.reducestreamer.model.IntervalWindowImpl; import io.numaproj.numaflow.reducestreamer.model.Message; import io.numaproj.numaflow.reducestreamer.model.Metadata; -import io.numaproj.numaflow.reducestreamer.model.MetadataImpl; -import io.numaproj.numaflow.reducestreamer.user.OutputStreamObserver; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; +import io.numaproj.numaflow.reducestreamer.model.OutputStreamObserver; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamerFactory; import org.junit.Test; import java.time.Instant; diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ServerErrTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ServerErrTest.java index dd14c531..cbd13c48 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/ServerErrTest.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ServerErrTest.java @@ -16,9 +16,9 @@ import io.numaproj.numaflow.reduce.v1.ReduceGrpc; import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; import io.numaproj.numaflow.reducestreamer.model.Datum; -import io.numaproj.numaflow.reducestreamer.user.OutputStreamObserver; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; +import io.numaproj.numaflow.reducestreamer.model.OutputStreamObserver; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamerFactory; import io.numaproj.numaflow.shared.GrpcServerUtils; import org.junit.After; import org.junit.Before; diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ServerTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ServerTest.java index 988166dc..f64830c0 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/ServerTest.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ServerTest.java @@ -17,9 +17,9 @@ import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; import io.numaproj.numaflow.reducestreamer.model.Datum; import io.numaproj.numaflow.reducestreamer.model.Message; -import io.numaproj.numaflow.reducestreamer.user.OutputStreamObserver; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; +import io.numaproj.numaflow.reducestreamer.model.OutputStreamObserver; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamerFactory; import io.numaproj.numaflow.shared.GrpcServerUtils; import org.junit.After; import org.junit.Before; diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java index 60de6785..aff2f3ab 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java @@ -7,13 +7,11 @@ import com.google.protobuf.ByteString; import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; import io.numaproj.numaflow.reducestreamer.model.Datum; -import io.numaproj.numaflow.reducestreamer.model.IntervalWindowImpl; import io.numaproj.numaflow.reducestreamer.model.Message; import io.numaproj.numaflow.reducestreamer.model.Metadata; -import io.numaproj.numaflow.reducestreamer.model.MetadataImpl; -import io.numaproj.numaflow.reducestreamer.user.OutputStreamObserver; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamer; -import io.numaproj.numaflow.reducestreamer.user.ReduceStreamerFactory; +import io.numaproj.numaflow.reducestreamer.model.OutputStreamObserver; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamer; +import io.numaproj.numaflow.reducestreamer.model.ReduceStreamerFactory; import org.junit.Test; import java.time.Instant; From 4a6bc98185acef0c19e49a9af8342252507ffb6f Mon Sep 17 00:00:00 2001 From: Keran Yang Date: Mon, 22 Jan 2024 11:35:47 -0500 Subject: [PATCH 08/11] remove unnecessary synchronized keyword Signed-off-by: Keran Yang --- .../reducestreamer/ResponseStreamActor.java | 15 +++++---------- .../ReduceOutputStreamObserver.java | 2 +- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java index e661c10a..5b1c2c63 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java @@ -16,8 +16,9 @@ import java.util.List; /** - * Response stream actor is dedicated to ensure synchronized calls to the responseObserver onNext(). - * ALL the responses are sent to the response stream actor before getting forwarded to the output gRPC stream. + * Response stream actor is a wrapper around the gRPC output stream. + * It ensures synchronized calls to the responseObserver onNext() and invokes onComplete at the end of the stream. + * ALL reduce responses are sent to the response stream actor before getting forwarded to the output gRPC stream. *

* More details about gRPC StreamObserver concurrency: https://grpc.github.io/grpc-java/javadoc/io/grpc/stub/StreamObserver.html */ @@ -42,10 +43,7 @@ public Receive createReceive() { } private void sendMessage(Message message) { - // Synchronized the access to the output stream - synchronized (responseObserver) { - responseObserver.onNext(this.buildResponse(message)); - } + responseObserver.onNext(this.buildResponse(message)); } private void sendEOF(ActorResponse actorResponse) { @@ -54,10 +52,7 @@ private void sendEOF(ActorResponse actorResponse) { "Unexpected behavior - Response Stream actor received a non-eof response. Response type is: " + actorResponse.getType()); } - // Synchronized the access to the output stream - synchronized (responseObserver) { - responseObserver.onNext(actorResponse.getResponse()); - } + responseObserver.onNext(actorResponse.getResponse()); // After the EOF response gets sent to gRPC output stream, // tell the supervisor that the actor is ready to be cleaned up. getSender().tell( diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceOutputStreamObserver.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceOutputStreamObserver.java index f7f56595..07b74bd4 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceOutputStreamObserver.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceOutputStreamObserver.java @@ -19,7 +19,7 @@ public class ReduceOutputStreamObserver implements StreamObserver receivedResponses = resultDatum.get(); receivedResponses.add(response); resultDatum.set(receivedResponses); From c67de10cf2f175741fe726a3a4f56c3740dc9ab7 Mon Sep 17 00:00:00 2001 From: Keran Yang Date: Mon, 22 Jan 2024 13:59:12 -0500 Subject: [PATCH 09/11] not expose output observer to the supervisor Signed-off-by: Keran Yang --- .../reducestreamer/sum/SumFactory.java | 4 ++ .../reducestreamer/ActorResponse.java | 12 +++--- .../reducestreamer/ActorResponseType.java | 8 ---- .../reducestreamer/ReduceStreamerActor.java | 9 ++--- .../reducestreamer/ReduceSupervisorActor.java | 40 ++++++------------- .../reducestreamer/ResponseStreamActor.java | 23 +++++------ .../numaflow/reducestreamer/Service.java | 3 +- .../ReduceSupervisorActorTest.java | 6 +-- .../reducestreamer/ShutDownActorTest.java | 6 +-- 9 files changed, 43 insertions(+), 68 deletions(-) delete mode 100644 src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponseType.java diff --git a/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFactory.java b/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFactory.java index 1c6d3d5e..5faa39d5 100644 --- a/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFactory.java +++ b/examples/src/main/java/io/numaproj/numaflow/examples/reducestreamer/sum/SumFactory.java @@ -4,6 +4,10 @@ import io.numaproj.numaflow.reducestreamer.model.ReduceStreamerFactory; import lombok.extern.slf4j.Slf4j; +/** + * SumFactory extends ReduceStreamerFactory to support creating instances of SumFunction. + * It also provides a main function to start a server for handling the reduce stream. + */ @Slf4j public class SumFactory extends ReduceStreamerFactory { diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java index 895af306..d8e522db 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java @@ -3,19 +3,19 @@ import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; import lombok.AllArgsConstructor; import lombok.Getter; +import lombok.Setter; /** - * ActorResponse is for child reduce streamer actors to report back to the supervisor actor about the status of the data processing. - * It serves two purposes: - * 1. Send to the supervisor an EOF response, which is to be sent to the gRPC output stream. - * 2. Send to the supervisor a signal, indicating that the actor has finished all its processing work, - * and it's ready to be cleaned up by the supervisor actor. + * The actor response holds the final EOF response for a particular key set. + * The isLast attribute indicates whether the response is the last one to be sent to + * the output gRPC stream. */ @Getter +@Setter @AllArgsConstructor class ActorResponse { ReduceOuterClass.ReduceResponse response; - ActorResponseType type; + boolean isLast; // TODO - do we need to include window information in the id? // for aligned reducer, there is always single window. diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponseType.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponseType.java deleted file mode 100644 index 13da3ae1..00000000 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponseType.java +++ /dev/null @@ -1,8 +0,0 @@ -package io.numaproj.numaflow.reducestreamer; - -enum ActorResponseType { - // EOF_RESPONSE indicates that the actor response contains an EOF reduce response without real data. - EOF_RESPONSE, - // READY_FOR_CLEAN_UP_SIGNAL indicates that the actor has finished sending responses and now ready to be cleaned up. - READY_FOR_CLEAN_UP_SIGNAL, -} diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java index 5897829e..9aba623b 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java @@ -10,7 +10,6 @@ import io.numaproj.numaflow.reducestreamer.model.OutputStreamObserver; import io.numaproj.numaflow.reducestreamer.model.ReduceStreamer; import lombok.AllArgsConstructor; -import lombok.extern.slf4j.Slf4j; import java.util.List; @@ -20,7 +19,6 @@ * When receiving an EOF signal from the supervisor, it invokes the handleEndOfStream to execute * the user-defined end of stream processing logics. */ -@Slf4j @AllArgsConstructor class ReduceStreamerActor extends AbstractActor { private String[] keys; @@ -52,9 +50,10 @@ private void invokeHandler(HandlerDatum handlerDatum) { } private void sendEOF(String EOF) { - // constructing final responses based on the messages processed so far and sending them out. + // invoke handleEndOfStream to materialize the messages received so far. this.groupBy.handleEndOfStream(keys, outputStream, md); - // constructing an EOF response and sending it back to the supervisor actor. + // construct an actor response and send it back to the supervisor actor, indicating the actor + // has finished processing all the messages for the corresponding key set. getSender().tell(buildEOFResponse(), getSelf()); } @@ -74,6 +73,6 @@ private ActorResponse buildEOFResponse() { .newBuilder() .addAllKeys(List.of(this.keys)) .build()); - return new ActorResponse(responseBuilder.build(), ActorResponseType.EOF_RESPONSE); + return new ActorResponse(responseBuilder.build(), false); } } diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java index 4d2acf5e..f74da750 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java @@ -8,7 +8,6 @@ import akka.japi.pf.DeciderBuilder; import akka.japi.pf.ReceiveBuilder; import com.google.common.base.Preconditions; -import io.grpc.stub.StreamObserver; import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; import io.numaproj.numaflow.reducestreamer.model.Metadata; import io.numaproj.numaflow.reducestreamer.model.ReduceStreamer; @@ -31,35 +30,30 @@ class ReduceSupervisorActor extends AbstractActor { private final Metadata md; private final ActorRef shutdownActor; private final ActorRef responseStreamActor; - private final StreamObserver responseObserver; private final Map actorsMap = new HashMap<>(); public ReduceSupervisorActor( ReduceStreamerFactory reduceStreamerFactory, Metadata md, ActorRef shutdownActor, - ActorRef responseStreamActor, - StreamObserver responseObserver) { + ActorRef responseStreamActor) { this.reduceStreamerFactory = reduceStreamerFactory; this.md = md; this.shutdownActor = shutdownActor; this.responseStreamActor = responseStreamActor; - this.responseObserver = responseObserver; } public static Props props( ReduceStreamerFactory reduceStreamerFactory, Metadata md, ActorRef shutdownActor, - ActorRef responseStreamActor, - StreamObserver responseObserver) { + ActorRef responseStreamActor) { return Props.create( ReduceSupervisorActor.class, reduceStreamerFactory, md, shutdownActor, - responseStreamActor, - responseObserver); + responseStreamActor); } // if there is an uncaught exception stop in the supervisor actor, send a signal to shut down @@ -86,7 +80,7 @@ public void postStop() { public Receive createReceive() { return ReceiveBuilder .create() - .match(ActorRequest.class, this::invokeActors) + .match(ActorRequest.class, this::invokeActor) .match(String.class, this::sendEOF) .match(ActorResponse.class, this::handleActorResponse) .build(); @@ -97,7 +91,7 @@ public Receive createReceive() { if there is no actor for an incoming set of keys, create a new reduce streamer actor track all the child actors using actors map */ - private void invokeActors(ActorRequest actorRequest) { + private void invokeActor(ActorRequest actorRequest) { String[] keys = actorRequest.getKeySet(); String uniqueId = actorRequest.getUniqueIdentifier(); if (!actorsMap.containsKey(uniqueId)) { @@ -110,7 +104,6 @@ private void invokeActors(ActorRequest actorRequest) { this.responseStreamActor)); actorsMap.put(uniqueId, actorRef); } - HandlerDatum handlerDatum = constructHandlerDatum(actorRequest.getRequest().getPayload()); actorsMap.get(uniqueId).tell(handlerDatum, getSelf()); } @@ -122,23 +115,16 @@ private void sendEOF(String EOF) { } private void handleActorResponse(ActorResponse actorResponse) { - if (actorResponse.getType() == ActorResponseType.EOF_RESPONSE) { - // forward the response to the response stream actor to send back to gRPC output stream. + // when the supervisor receives an actor response, it means the corresponding + // reduce streamer actor has finished its job. + // we remove the entry from the actors map. + actorsMap.remove(actorResponse.getActorUniqueIdentifier()); + if (actorsMap.isEmpty()) { + // since the actors map is empty, this particular actor response is the last response to forward to output gRPC stream. + actorResponse.setLast(true); this.responseStreamActor.tell(actorResponse, getSelf()); - } else if (actorResponse.getType() == ActorResponseType.READY_FOR_CLEAN_UP_SIGNAL) { - // the corresponding actor is ready to be cleaned up. - // remove the child entry from the map. - // if there are no entries in the map, that means processing is - // done we can close the entire stream. - actorsMap.remove(actorResponse.getActorUniqueIdentifier()); - if (actorsMap.isEmpty()) { - responseObserver.onCompleted(); - getContext().getSystem().stop(getSelf()); - } } else { - throw new RuntimeException( - "Supervisor actor received an actor response with unsupported type: " - + actorResponse.getType()); + this.responseStreamActor.tell(actorResponse, getSelf()); } } diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java index 5b1c2c63..170d1f23 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java @@ -47,19 +47,18 @@ private void sendMessage(Message message) { } private void sendEOF(ActorResponse actorResponse) { - if (actorResponse.getType() != ActorResponseType.EOF_RESPONSE) { - throw new RuntimeException( - "Unexpected behavior - Response Stream actor received a non-eof response. Response type is: " - + actorResponse.getType()); + if (actorResponse.isLast()) { + // handle the very last response. + responseObserver.onNext(actorResponse.getResponse()); + // close the output stream. + responseObserver.onCompleted(); + // stop the AKKA system right after we close the output stream. + // note: could make more sense if the supervisor actor stops the system, + // but it requires an extra tell. + getContext().getSystem().stop(getSender()); + } else { + responseObserver.onNext(actorResponse.getResponse()); } - responseObserver.onNext(actorResponse.getResponse()); - // After the EOF response gets sent to gRPC output stream, - // tell the supervisor that the actor is ready to be cleaned up. - getSender().tell( - new ActorResponse( - actorResponse.getResponse(), - ActorResponseType.READY_FOR_CLEAN_UP_SIGNAL), - getSelf()); } private ReduceOuterClass.ReduceResponse buildResponse(Message message) { diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java index b6486d7f..0533b3a7 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java @@ -90,8 +90,7 @@ public StreamObserver reduceFn(final StreamObser reduceStreamerFactory, md, shutdownActorRef, - responseStreamActor, - responseObserver)); + responseStreamActor)); return new StreamObserver<>() { diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java index 13148b7c..84ff65d0 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java @@ -45,8 +45,7 @@ public void given_inputRequestsShareSameKeys_when_supervisorActorBroadcasts_then new TestReduceStreamerFactory(), md, shutdownActor, - responseStreamActor, - reduceOutputStreamObserver)); + responseStreamActor)); for (int i = 1; i <= 10; i++) { io.numaproj.numaflow.reducestreamer.ActorRequest reduceRequest = new io.numaproj.numaflow.reducestreamer.ActorRequest( @@ -103,8 +102,7 @@ public void given_inputRequestsHaveDifferentKeySets_when_supervisorActorBroadcas new TestReduceStreamerFactory(), md, shutdownActor, - responseStreamActor, - reduceOutputStreamObserver) + responseStreamActor) ); for (int i = 1; i <= 10; i++) { diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java index aff2f3ab..1b3bba6f 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java @@ -50,8 +50,7 @@ public void testFailure() { new TestExceptionFactory(), md, shutdownActor, - responseStreamActor, - new io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver())); + responseStreamActor)); io.numaproj.numaflow.reducestreamer.ActorRequest reduceRequest = new io.numaproj.numaflow.reducestreamer.ActorRequest( ReduceOuterClass.ReduceRequest.newBuilder() @@ -95,8 +94,7 @@ public void testDeadLetterHandling() { new TestExceptionFactory(), md, shutdownActor, - responseStreamActor, - reduceOutputStreamObserver)); + responseStreamActor)); DeadLetter deadLetter = new DeadLetter("dead-letter", shutdownActor, supervisor); supervisor.tell(deadLetter, ActorRef.noSender()); From cd078115f17e1d02b771c264ecfb02b96f2a5770 Mon Sep 17 00:00:00 2001 From: Keran Yang Date: Mon, 22 Jan 2024 14:24:18 -0500 Subject: [PATCH 10/11] rename Signed-off-by: Keran Yang --- .../reducestreamer/ActorResponse.java | 7 +++- ...ponseStreamActor.java => OutputActor.java} | 8 ++-- .../numaflow/reducestreamer/Service.java | 12 +++--- ...eShutdownActor.java => ShutdownActor.java} | 6 +-- ...ervisorActor.java => SupervisorActor.java} | 8 ++-- .../numaflow/reducer/ShutDownActorTest.java | 10 ++--- ...ctorTest.java => SupervisorActorTest.java} | 14 +++---- ...nActorTest.java => ShutdownActorTest.java} | 28 ++++++------- ...ctorTest.java => SupervisorActorTest.java} | 40 ++++++++++--------- 9 files changed, 70 insertions(+), 63 deletions(-) rename src/main/java/io/numaproj/numaflow/reducestreamer/{ResponseStreamActor.java => OutputActor.java} (93%) rename src/main/java/io/numaproj/numaflow/reducestreamer/{ReduceShutdownActor.java => ShutdownActor.java} (91%) rename src/main/java/io/numaproj/numaflow/reducestreamer/{ReduceSupervisorActor.java => SupervisorActor.java} (96%) rename src/test/java/io/numaproj/numaflow/reducer/{ReduceSupervisorActorTest.java => SupervisorActorTest.java} (93%) rename src/test/java/io/numaproj/numaflow/reducestreamer/{ShutDownActorTest.java => ShutdownActorTest.java} (83%) rename src/test/java/io/numaproj/numaflow/reducestreamer/{ReduceSupervisorActorTest.java => SupervisorActorTest.java} (84%) diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java index d8e522db..3a73509c 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ActorResponse.java @@ -7,8 +7,11 @@ /** * The actor response holds the final EOF response for a particular key set. - * The isLast attribute indicates whether the response is the last one to be sent to - * the output gRPC stream. + *

+ * The isLast attribute indicates whether the response is globally the last one to be sent to + * the output gRPC stream, if set to true, it means the response is the very last response among + * all key sets. When output stream actor receives an isLast response, it sends the response and immediately + * closes the output stream. */ @Getter @Setter diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/OutputActor.java similarity index 93% rename from src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/OutputActor.java index 170d1f23..fc12ee02 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ResponseStreamActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/OutputActor.java @@ -16,7 +16,7 @@ import java.util.List; /** - * Response stream actor is a wrapper around the gRPC output stream. + * Output actor is a wrapper around the gRPC output stream. * It ensures synchronized calls to the responseObserver onNext() and invokes onComplete at the end of the stream. * ALL reduce responses are sent to the response stream actor before getting forwarded to the output gRPC stream. *

@@ -24,14 +24,14 @@ */ @Slf4j @AllArgsConstructor -class ResponseStreamActor extends AbstractActor { +class OutputActor extends AbstractActor { StreamObserver responseObserver; Metadata md; public static Props props( StreamObserver responseObserver, Metadata md) { - return Props.create(ResponseStreamActor.class, responseObserver, md); + return Props.create(OutputActor.class, responseObserver, md); } @Override @@ -48,7 +48,7 @@ private void sendMessage(Message message) { private void sendEOF(ActorResponse actorResponse) { if (actorResponse.isLast()) { - // handle the very last response. + // send the very last response. responseObserver.onNext(actorResponse.getResponse()); // close the output stream. responseObserver.onCompleted(); diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java index 0533b3a7..5b0b0745 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java @@ -71,26 +71,26 @@ public StreamObserver reduceFn(final StreamObser // create a shutdown actor that listens to exceptions. ActorRef shutdownActorRef = reduceActorSystem. - actorOf(ReduceShutdownActor.props(failureFuture)); + actorOf(ShutdownActor.props(failureFuture)); // subscribe for dead letters reduceActorSystem.getEventStream().subscribe(shutdownActorRef, AllDeadLetters.class); handleFailure(failureFuture, responseObserver); - // create a response stream actor that ensures synchronized delivery of reduce responses. - ActorRef responseStreamActor = reduceActorSystem. - actorOf(ResponseStreamActor.props(responseObserver, md)); + // create an output actor that ensures synchronized delivery of reduce responses. + ActorRef outputActor = reduceActorSystem. + actorOf(OutputActor.props(responseObserver, md)); /* create a supervisor actor which assign the tasks to child actors. we create a child actor for every unique set of keys in a window. */ ActorRef supervisorActor = reduceActorSystem - .actorOf(ReduceSupervisorActor.props( + .actorOf(SupervisorActor.props( reduceStreamerFactory, md, shutdownActorRef, - responseStreamActor)); + outputActor)); return new StreamObserver<>() { diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceShutdownActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ShutdownActor.java similarity index 91% rename from src/main/java/io/numaproj/numaflow/reducestreamer/ReduceShutdownActor.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/ShutdownActor.java index 010c0862..6c690fcf 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceShutdownActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ShutdownActor.java @@ -11,16 +11,16 @@ import java.util.concurrent.CompletableFuture; /** - * Reduce shutdown actor, listens to exceptions and handles shutdown. + * Shutdown actor, listens to exceptions and handles shutdown. */ @Slf4j @AllArgsConstructor -class ReduceShutdownActor extends AbstractActor { +class ShutdownActor extends AbstractActor { private final CompletableFuture failureFuture; public static Props props( CompletableFuture failureFuture) { - return Props.create(ReduceShutdownActor.class, failureFuture); + return Props.create(ShutdownActor.class, failureFuture); } @Override diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/SupervisorActor.java similarity index 96% rename from src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java rename to src/main/java/io/numaproj/numaflow/reducestreamer/SupervisorActor.java index f74da750..a4d782b4 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/SupervisorActor.java @@ -22,17 +22,17 @@ import java.util.Optional; /** - * Reduce supervisor actor distributes the messages to other actors and handles failures. + * Supervisor actor distributes the messages to other actors and handles failures. */ @Slf4j -class ReduceSupervisorActor extends AbstractActor { +class SupervisorActor extends AbstractActor { private final ReduceStreamerFactory reduceStreamerFactory; private final Metadata md; private final ActorRef shutdownActor; private final ActorRef responseStreamActor; private final Map actorsMap = new HashMap<>(); - public ReduceSupervisorActor( + public SupervisorActor( ReduceStreamerFactory reduceStreamerFactory, Metadata md, ActorRef shutdownActor, @@ -49,7 +49,7 @@ public static Props props( ActorRef shutdownActor, ActorRef responseStreamActor) { return Props.create( - ReduceSupervisorActor.class, + SupervisorActor.class, reduceStreamerFactory, md, shutdownActor, diff --git a/src/test/java/io/numaproj/numaflow/reducer/ShutDownActorTest.java b/src/test/java/io/numaproj/numaflow/reducer/ShutDownActorTest.java index b3bc7e7d..897872b8 100644 --- a/src/test/java/io/numaproj/numaflow/reducer/ShutDownActorTest.java +++ b/src/test/java/io/numaproj/numaflow/reducer/ShutDownActorTest.java @@ -36,7 +36,7 @@ public void testFailure() { Metadata md = new MetadataImpl( new IntervalWindowImpl(Instant.now(), Instant.now())); - ActorRef supervisor = actorSystem + ActorRef supervisorActor = actorSystem .actorOf(ReduceSupervisorActor .props( new TestExceptionFactory(), @@ -50,7 +50,7 @@ public void testFailure() { .setValue(ByteString.copyFromUtf8(String.valueOf(1))) .build()) .build()); - supervisor.tell(reduceRequest, ActorRef.noSender()); + supervisorActor.tell(reduceRequest, ActorRef.noSender()); try { completableFuture.get(); @@ -74,7 +74,7 @@ public void testDeadLetterHandling() { Metadata md = new MetadataImpl( new IntervalWindowImpl(Instant.now(), Instant.now())); - ActorRef supervisor = actorSystem + ActorRef supervisorActor = actorSystem .actorOf(ReduceSupervisorActor .props( new TestExceptionFactory(), @@ -82,8 +82,8 @@ public void testDeadLetterHandling() { shutdownActor, new ReduceOutputStreamObserver())); - DeadLetter deadLetter = new DeadLetter("dead-letter", shutdownActor, supervisor); - supervisor.tell(deadLetter, ActorRef.noSender()); + DeadLetter deadLetter = new DeadLetter("dead-letter", shutdownActor, supervisorActor); + supervisorActor.tell(deadLetter, ActorRef.noSender()); try { completableFuture.get(); diff --git a/src/test/java/io/numaproj/numaflow/reducer/ReduceSupervisorActorTest.java b/src/test/java/io/numaproj/numaflow/reducer/SupervisorActorTest.java similarity index 93% rename from src/test/java/io/numaproj/numaflow/reducer/ReduceSupervisorActorTest.java rename to src/test/java/io/numaproj/numaflow/reducer/SupervisorActorTest.java index e33cbd21..d98760ff 100644 --- a/src/test/java/io/numaproj/numaflow/reducer/ReduceSupervisorActorTest.java +++ b/src/test/java/io/numaproj/numaflow/reducer/SupervisorActorTest.java @@ -17,7 +17,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -public class ReduceSupervisorActorTest { +public class SupervisorActorTest { @Test public void given_inputRequestsShareSameKeys_when_supervisorActorBroadcasts_then_onlyOneReducerActorGetsCreatedAndAggregatesAllRequests() throws RuntimeException { @@ -33,7 +33,7 @@ public void given_inputRequestsShareSameKeys_when_supervisorActorBroadcasts_then ReduceOutputStreamObserver outputStreamObserver = new ReduceOutputStreamObserver(); - ActorRef supervisor = actorSystem + ActorRef supervisorActor = actorSystem .actorOf(ReduceSupervisorActor .props(new TestReducerFactory(), md, shutdownActor, outputStreamObserver)); @@ -47,9 +47,9 @@ public void given_inputRequestsShareSameKeys_when_supervisorActorBroadcasts_then .setValue(ByteString.copyFromUtf8(String.valueOf(i))) .build()) .build()); - supervisor.tell(reduceRequest, ActorRef.noSender()); + supervisorActor.tell(reduceRequest, ActorRef.noSender()); } - supervisor.tell(Constants.EOF, ActorRef.noSender()); + supervisorActor.tell(Constants.EOF, ActorRef.noSender()); try { completableFuture.get(); @@ -83,7 +83,7 @@ public void given_inputRequestsHaveDifferentKeySets_when_supervisorActorBroadcas new IntervalWindowImpl(Instant.now(), Instant.now())); ReduceOutputStreamObserver outputStreamObserver = new ReduceOutputStreamObserver(); - ActorRef supervisor = actorSystem + ActorRef supervisorActor = actorSystem .actorOf(ReduceSupervisorActor .props( new TestReducerFactory(), @@ -102,10 +102,10 @@ public void given_inputRequestsHaveDifferentKeySets_when_supervisorActorBroadcas .setValue(ByteString.copyFromUtf8(String.valueOf(i))) .build()) .build()); - supervisor.tell(reduceRequest, ActorRef.noSender()); + supervisorActor.tell(reduceRequest, ActorRef.noSender()); } - supervisor.tell(Constants.EOF, ActorRef.noSender()); + supervisorActor.tell(Constants.EOF, ActorRef.noSender()); try { completableFuture.get(); // each reduce request generates two reduce responses, one containing the data and the other one indicating EOF. diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ShutdownActorTest.java similarity index 83% rename from src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java rename to src/test/java/io/numaproj/numaflow/reducestreamer/ShutdownActorTest.java index 1b3bba6f..8e51e814 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/ShutDownActorTest.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ShutdownActorTest.java @@ -21,7 +21,7 @@ import static org.junit.Assert.fail; -public class ShutDownActorTest { +public class ShutdownActorTest { @Test public void testFailure() { final ActorSystem actorSystem = ActorSystem.create("test-system-1"); @@ -33,7 +33,7 @@ public void testFailure() { .addKeys(reduceKey); ActorRef shutdownActor = actorSystem - .actorOf(io.numaproj.numaflow.reducestreamer.ReduceShutdownActor + .actorOf(ShutdownActor .props(completableFuture)); Metadata md = new MetadataImpl( @@ -41,16 +41,16 @@ public void testFailure() { io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver reduceOutputStreamObserver = new io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver(); - ActorRef responseStreamActor = actorSystem.actorOf(io.numaproj.numaflow.reducestreamer.ResponseStreamActor + ActorRef outputActor = actorSystem.actorOf(OutputActor .props(reduceOutputStreamObserver, md)); - ActorRef supervisor = actorSystem - .actorOf(io.numaproj.numaflow.reducestreamer.ReduceSupervisorActor + ActorRef supervisorActor = actorSystem + .actorOf(SupervisorActor .props( new TestExceptionFactory(), md, shutdownActor, - responseStreamActor)); + outputActor)); io.numaproj.numaflow.reducestreamer.ActorRequest reduceRequest = new io.numaproj.numaflow.reducestreamer.ActorRequest( ReduceOuterClass.ReduceRequest.newBuilder() @@ -59,7 +59,7 @@ public void testFailure() { .setValue(ByteString.copyFromUtf8(String.valueOf(1))) .build()) .build()); - supervisor.tell(reduceRequest, ActorRef.noSender()); + supervisorActor.tell(reduceRequest, ActorRef.noSender()); try { completableFuture.get(); @@ -75,7 +75,7 @@ public void testDeadLetterHandling() { CompletableFuture completableFuture = new CompletableFuture<>(); ActorRef shutdownActor = actorSystem - .actorOf(io.numaproj.numaflow.reducestreamer.ReduceShutdownActor + .actorOf(ShutdownActor .props(completableFuture)); actorSystem.eventStream().subscribe(shutdownActor, AllDeadLetters.class); @@ -85,19 +85,19 @@ public void testDeadLetterHandling() { io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver reduceOutputStreamObserver = new io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver(); - ActorRef responseStreamActor = actorSystem.actorOf(io.numaproj.numaflow.reducestreamer.ResponseStreamActor + ActorRef outputActor = actorSystem.actorOf(OutputActor .props(reduceOutputStreamObserver, md)); - ActorRef supervisor = actorSystem - .actorOf(io.numaproj.numaflow.reducestreamer.ReduceSupervisorActor + ActorRef supervisorActor = actorSystem + .actorOf(SupervisorActor .props( new TestExceptionFactory(), md, shutdownActor, - responseStreamActor)); + outputActor)); - DeadLetter deadLetter = new DeadLetter("dead-letter", shutdownActor, supervisor); - supervisor.tell(deadLetter, ActorRef.noSender()); + DeadLetter deadLetter = new DeadLetter("dead-letter", shutdownActor, supervisorActor); + supervisorActor.tell(deadLetter, ActorRef.noSender()); try { completableFuture.get(); diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/SupervisorActorTest.java similarity index 84% rename from src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java rename to src/test/java/io/numaproj/numaflow/reducestreamer/SupervisorActorTest.java index 84ff65d0..14518e36 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/ReduceSupervisorActorTest.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/SupervisorActorTest.java @@ -21,14 +21,14 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -public class ReduceSupervisorActorTest { +public class SupervisorActorTest { @Test public void given_inputRequestsShareSameKeys_when_supervisorActorBroadcasts_then_onlyOneReducerActorGetsCreatedAndAggregatesAllRequests() throws RuntimeException { final ActorSystem actorSystem = ActorSystem.create("test-system-1"); - CompletableFuture completableFuture = new CompletableFuture(); + CompletableFuture completableFuture = new CompletableFuture<>(); ActorRef shutdownActor = actorSystem - .actorOf(io.numaproj.numaflow.reducestreamer.ReduceShutdownActor + .actorOf(ShutdownActor .props(completableFuture)); Metadata md = new MetadataImpl( @@ -36,16 +36,16 @@ public void given_inputRequestsShareSameKeys_when_supervisorActorBroadcasts_then io.numaproj.numaflow.reducer.ReduceOutputStreamObserver reduceOutputStreamObserver = new io.numaproj.numaflow.reducer.ReduceOutputStreamObserver(); - ActorRef responseStreamActor = actorSystem.actorOf(io.numaproj.numaflow.reducestreamer.ResponseStreamActor + ActorRef outputActor = actorSystem.actorOf(OutputActor .props(reduceOutputStreamObserver, md)); - ActorRef supervisor = actorSystem - .actorOf(io.numaproj.numaflow.reducestreamer.ReduceSupervisorActor + ActorRef supervisorActor = actorSystem + .actorOf(SupervisorActor .props( new TestReduceStreamerFactory(), md, shutdownActor, - responseStreamActor)); + outputActor)); for (int i = 1; i <= 10; i++) { io.numaproj.numaflow.reducestreamer.ActorRequest reduceRequest = new io.numaproj.numaflow.reducestreamer.ActorRequest( @@ -58,9 +58,11 @@ public void given_inputRequestsShareSameKeys_when_supervisorActorBroadcasts_then .setValue(ByteString.copyFromUtf8(String.valueOf(i))) .build()) .build()); - supervisor.tell(reduceRequest, ActorRef.noSender()); + supervisorActor.tell(reduceRequest, ActorRef.noSender()); } - supervisor.tell(io.numaproj.numaflow.reducestreamer.Constants.EOF, ActorRef.noSender()); + supervisorActor.tell( + io.numaproj.numaflow.reducestreamer.Constants.EOF, + ActorRef.noSender()); try { completableFuture.get(); @@ -72,7 +74,7 @@ public void given_inputRequestsShareSameKeys_when_supervisorActorBroadcasts_then .getResult() .getValue() .toStringUtf8()); - assertEquals(true, reduceOutputStreamObserver.resultDatum + assertTrue(reduceOutputStreamObserver.resultDatum .get() .get(1) .getEOF()); @@ -84,25 +86,25 @@ public void given_inputRequestsShareSameKeys_when_supervisorActorBroadcasts_then @Test public void given_inputRequestsHaveDifferentKeySets_when_supervisorActorBroadcasts_then_multipleReducerActorsHandleKeySetsSeparately() throws RuntimeException { final ActorSystem actorSystem = ActorSystem.create("test-system-2"); - CompletableFuture completableFuture = new CompletableFuture(); + CompletableFuture completableFuture = new CompletableFuture<>(); ActorRef shutdownActor = actorSystem - .actorOf(io.numaproj.numaflow.reducestreamer.ReduceShutdownActor + .actorOf(ShutdownActor .props(completableFuture)); Metadata md = new MetadataImpl( new IntervalWindowImpl(Instant.now(), Instant.now())); io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver reduceOutputStreamObserver = new ReduceOutputStreamObserver(); - ActorRef responseStreamActor = actorSystem.actorOf(io.numaproj.numaflow.reducestreamer.ResponseStreamActor + ActorRef outputActor = actorSystem.actorOf(OutputActor .props(reduceOutputStreamObserver, md)); - ActorRef supervisor = actorSystem - .actorOf(io.numaproj.numaflow.reducestreamer.ReduceSupervisorActor + ActorRef supervisorActor = actorSystem + .actorOf(SupervisorActor .props( new TestReduceStreamerFactory(), md, shutdownActor, - responseStreamActor) + outputActor) ); for (int i = 1; i <= 10; i++) { @@ -116,10 +118,12 @@ public void given_inputRequestsHaveDifferentKeySets_when_supervisorActorBroadcas .setValue(ByteString.copyFromUtf8(String.valueOf(i))) .build()) .build()); - supervisor.tell(reduceRequest, ActorRef.noSender()); + supervisorActor.tell(reduceRequest, ActorRef.noSender()); } - supervisor.tell(io.numaproj.numaflow.reducestreamer.Constants.EOF, ActorRef.noSender()); + supervisorActor.tell( + io.numaproj.numaflow.reducestreamer.Constants.EOF, + ActorRef.noSender()); try { completableFuture.get(); // each reduce request generates two reduce responses, one containing the data and the other one indicating EOF. From 60c45fc07325bd76329567dbe2a94582910c4b19 Mon Sep 17 00:00:00 2001 From: Keran Yang Date: Mon, 22 Jan 2024 22:38:17 -0500 Subject: [PATCH 11/11] address comments Signed-off-by: Keran Yang --- .../numaflow/reducestreamer/OutputActor.java | 47 ++----------------- .../OutputStreamObserverImpl.java | 35 +++++++++++++- .../reducestreamer/ReduceStreamerActor.java | 2 +- .../numaflow/reducestreamer/Service.java | 2 +- .../reducestreamer/model/Message.java | 2 +- .../reducestreamer/model/Metadata.java | 2 +- .../model/OutputStreamObserver.java | 2 +- .../model/ReduceStreamerFactory.java | 5 ++ .../reducestreamer/ShutdownActorTest.java | 4 +- .../reducestreamer/SupervisorActorTest.java | 4 +- 10 files changed, 52 insertions(+), 53 deletions(-) diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/OutputActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/OutputActor.java index fc12ee02..afa4e724 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/OutputActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/OutputActor.java @@ -2,19 +2,11 @@ import akka.actor.AbstractActor; import akka.actor.Props; -import com.google.protobuf.ByteString; -import com.google.protobuf.Timestamp; import io.grpc.stub.StreamObserver; import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; -import io.numaproj.numaflow.reducestreamer.model.Message; -import io.numaproj.numaflow.reducestreamer.model.Metadata; import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - /** * Output actor is a wrapper around the gRPC output stream. * It ensures synchronized calls to the responseObserver onNext() and invokes onComplete at the end of the stream. @@ -26,27 +18,20 @@ @AllArgsConstructor class OutputActor extends AbstractActor { StreamObserver responseObserver; - Metadata md; public static Props props( - StreamObserver responseObserver, - Metadata md) { - return Props.create(OutputActor.class, responseObserver, md); + StreamObserver responseObserver) { + return Props.create(OutputActor.class, responseObserver); } @Override public Receive createReceive() { return receiveBuilder() - .match(Message.class, this::sendMessage) - .match(ActorResponse.class, this::sendEOF) + .match(ActorResponse.class, this::handleResponse) .build(); } - private void sendMessage(Message message) { - responseObserver.onNext(this.buildResponse(message)); - } - - private void sendEOF(ActorResponse actorResponse) { + private void handleResponse(ActorResponse actorResponse) { if (actorResponse.isLast()) { // send the very last response. responseObserver.onNext(actorResponse.getResponse()); @@ -60,28 +45,4 @@ private void sendEOF(ActorResponse actorResponse) { responseObserver.onNext(actorResponse.getResponse()); } } - - private ReduceOuterClass.ReduceResponse buildResponse(Message message) { - ReduceOuterClass.ReduceResponse.Builder responseBuilder = ReduceOuterClass.ReduceResponse.newBuilder(); - // set the window using the actor metadata. - responseBuilder.setWindow(ReduceOuterClass.Window.newBuilder() - .setStart(Timestamp.newBuilder() - .setSeconds(this.md.getIntervalWindow().getStartTime().getEpochSecond()) - .setNanos(this.md.getIntervalWindow().getStartTime().getNano())) - .setEnd(Timestamp.newBuilder() - .setSeconds(this.md.getIntervalWindow().getEndTime().getEpochSecond()) - .setNanos(this.md.getIntervalWindow().getEndTime().getNano())) - .setSlot("slot-0").build()); - responseBuilder.setEOF(false); - // set the result. - responseBuilder.setResult(ReduceOuterClass.ReduceResponse.Result - .newBuilder() - .setValue(ByteString.copyFrom(message.getValue())) - .addAllKeys(message.getKeys() - == null ? new ArrayList<>():Arrays.asList(message.getKeys())) - .addAllTags( - message.getTags() == null ? new ArrayList<>():List.of(message.getTags())) - .build()); - return responseBuilder.build(); - } } diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/OutputStreamObserverImpl.java b/src/main/java/io/numaproj/numaflow/reducestreamer/OutputStreamObserverImpl.java index c843c80f..9a37726d 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/OutputStreamObserverImpl.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/OutputStreamObserverImpl.java @@ -1,16 +1,49 @@ package io.numaproj.numaflow.reducestreamer; import akka.actor.ActorRef; +import com.google.protobuf.ByteString; +import com.google.protobuf.Timestamp; +import io.numaproj.numaflow.reduce.v1.ReduceOuterClass; import io.numaproj.numaflow.reducestreamer.model.Message; +import io.numaproj.numaflow.reducestreamer.model.Metadata; import io.numaproj.numaflow.reducestreamer.model.OutputStreamObserver; import lombok.AllArgsConstructor; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + @AllArgsConstructor class OutputStreamObserverImpl implements OutputStreamObserver { + private final Metadata md; private final ActorRef responseStreamActor; @Override public void send(Message message) { - this.responseStreamActor.tell(message, ActorRef.noSender()); + this.responseStreamActor.tell(buildResponse(message), ActorRef.noSender()); + } + + private ActorResponse buildResponse(Message message) { + ReduceOuterClass.ReduceResponse.Builder responseBuilder = ReduceOuterClass.ReduceResponse.newBuilder(); + // set the window using the actor metadata. + responseBuilder.setWindow(ReduceOuterClass.Window.newBuilder() + .setStart(Timestamp.newBuilder() + .setSeconds(this.md.getIntervalWindow().getStartTime().getEpochSecond()) + .setNanos(this.md.getIntervalWindow().getStartTime().getNano())) + .setEnd(Timestamp.newBuilder() + .setSeconds(this.md.getIntervalWindow().getEndTime().getEpochSecond()) + .setNanos(this.md.getIntervalWindow().getEndTime().getNano())) + .setSlot("slot-0").build()); + responseBuilder.setEOF(false); + // set the result. + responseBuilder.setResult(ReduceOuterClass.ReduceResponse.Result + .newBuilder() + .setValue(ByteString.copyFrom(message.getValue())) + .addAllKeys(message.getKeys() + == null ? new ArrayList<>():Arrays.asList(message.getKeys())) + .addAllTags( + message.getTags() == null ? new ArrayList<>():List.of(message.getTags())) + .build()); + return new ActorResponse(responseBuilder.build(), false); } } diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java index 9aba623b..27bc71f8 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/ReduceStreamerActor.java @@ -33,7 +33,7 @@ public static Props props( keys, md, groupBy, - new OutputStreamObserverImpl(responseStreamActor)); + new OutputStreamObserverImpl(md, responseStreamActor)); } @Override diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java b/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java index 5b0b0745..c8029469 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/Service.java @@ -80,7 +80,7 @@ public StreamObserver reduceFn(final StreamObser // create an output actor that ensures synchronized delivery of reduce responses. ActorRef outputActor = reduceActorSystem. - actorOf(OutputActor.props(responseObserver, md)); + actorOf(OutputActor.props(responseObserver)); /* create a supervisor actor which assign the tasks to child actors. we create a child actor for every unique set of keys in a window. diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/model/Message.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/Message.java index 88220cff..05e3e7b8 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/model/Message.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/Message.java @@ -3,7 +3,7 @@ import lombok.Getter; /** - * Message is used to wrap the data returned by Reducer functions. + * Message is used to wrap the data returned by Reduce Streamer functions. */ @Getter public class Message { diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/model/Metadata.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/Metadata.java index e59a61db..50e285c4 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/model/Metadata.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/Metadata.java @@ -1,7 +1,7 @@ package io.numaproj.numaflow.reducestreamer.model; /** - * Metadata contains methods to get the metadata for the reduce operation. + * Metadata contains methods to get the metadata for the reduce stream operation. */ public interface Metadata { /** diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/model/OutputStreamObserver.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/OutputStreamObserver.java index a0348561..a2eed211 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/model/OutputStreamObserver.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/OutputStreamObserver.java @@ -1,7 +1,7 @@ package io.numaproj.numaflow.reducestreamer.model; /** - * OutputStreamObserver sends to the output stream, the messages generate by the reducer. + * OutputStreamObserver sends to the output stream, the messages generate by the reduce streamer. */ public interface OutputStreamObserver { /** diff --git a/src/main/java/io/numaproj/numaflow/reducestreamer/model/ReduceStreamerFactory.java b/src/main/java/io/numaproj/numaflow/reducestreamer/model/ReduceStreamerFactory.java index 9a66bace..b90ef7d9 100644 --- a/src/main/java/io/numaproj/numaflow/reducestreamer/model/ReduceStreamerFactory.java +++ b/src/main/java/io/numaproj/numaflow/reducestreamer/model/ReduceStreamerFactory.java @@ -4,5 +4,10 @@ * ReduceStreamerFactory is the factory for ReduceStreamer. */ public abstract class ReduceStreamerFactory { + /** + * Helper function to create a reduce streamer. + * + * @return a concrete reduce streamer instance + */ public abstract ReduceStreamerT createReduceStreamer(); } diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/ShutdownActorTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/ShutdownActorTest.java index 8e51e814..9d8bfa6e 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/ShutdownActorTest.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/ShutdownActorTest.java @@ -42,7 +42,7 @@ public void testFailure() { io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver reduceOutputStreamObserver = new io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver(); ActorRef outputActor = actorSystem.actorOf(OutputActor - .props(reduceOutputStreamObserver, md)); + .props(reduceOutputStreamObserver)); ActorRef supervisorActor = actorSystem .actorOf(SupervisorActor @@ -86,7 +86,7 @@ public void testDeadLetterHandling() { io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver reduceOutputStreamObserver = new io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver(); ActorRef outputActor = actorSystem.actorOf(OutputActor - .props(reduceOutputStreamObserver, md)); + .props(reduceOutputStreamObserver)); ActorRef supervisorActor = actorSystem .actorOf(SupervisorActor diff --git a/src/test/java/io/numaproj/numaflow/reducestreamer/SupervisorActorTest.java b/src/test/java/io/numaproj/numaflow/reducestreamer/SupervisorActorTest.java index 14518e36..8e989db6 100644 --- a/src/test/java/io/numaproj/numaflow/reducestreamer/SupervisorActorTest.java +++ b/src/test/java/io/numaproj/numaflow/reducestreamer/SupervisorActorTest.java @@ -37,7 +37,7 @@ public void given_inputRequestsShareSameKeys_when_supervisorActorBroadcasts_then io.numaproj.numaflow.reducer.ReduceOutputStreamObserver reduceOutputStreamObserver = new io.numaproj.numaflow.reducer.ReduceOutputStreamObserver(); ActorRef outputActor = actorSystem.actorOf(OutputActor - .props(reduceOutputStreamObserver, md)); + .props(reduceOutputStreamObserver)); ActorRef supervisorActor = actorSystem .actorOf(SupervisorActor @@ -97,7 +97,7 @@ public void given_inputRequestsHaveDifferentKeySets_when_supervisorActorBroadcas io.numaproj.numaflow.reducestreamer.ReduceOutputStreamObserver reduceOutputStreamObserver = new ReduceOutputStreamObserver(); ActorRef outputActor = actorSystem.actorOf(OutputActor - .props(reduceOutputStreamObserver, md)); + .props(reduceOutputStreamObserver)); ActorRef supervisorActor = actorSystem .actorOf(SupervisorActor .props(