Live Stream processing support #23

mattp-swirldslabs · 2024-06-14T14:49:27Z

Description:

Fixes #16
Fixes #17
Fixes #18
Fixes #21

First draft of #27

Notes for reviewer:

Checklist

Documented (Code comments, README, etc.)
Tested (unit, integration, etc.)

- added license headers to java files - added bidirectional gRPC live streaming blocks implementation with 1 producer and N consumers Signed-off-by: Matt Peterson <[email protected]>

rbair23

Great start, happy to see the PR! Another thing I'm missing is some kind of design documentation to give some orientation around what the different components are.

.gitignore

gradle/modules.properties

protos/src/main/protobuf/blockstream.proto

server/src/main/java/com/hedera/block/server/BlockStreamService.java

server/src/main/java/com/hedera/block/server/Constants.java

server/src/main/java/com/hedera/block/server/Server.java

server/src/main/resources/application.yaml

rbair23 · 2024-06-25T17:33:49Z

Is this the basis for the design in this PR?

Signed-off-by: Matt Peterson <[email protected]>

jsync-swirlds

A few suggestions and a question or two.

server/src/main/java/com/hedera/block/server/BlockStreamService.java

protos/src/main/protobuf/blockstream.proto

server/src/main/java/com/hedera/block/server/BlockStreamService.java

.gitignore

server/src/main/java/com/hedera/block/server/consumer/LiveStreamObserverImpl.java

server/src/main/java/com/hedera/block/server/mediator/LiveStreamMediatorImpl.java

server/src/main/java/com/hedera/block/server/persistence/cache/BNLinkedHashMap.java

Signed-off-by: Matt Peterson <[email protected]>

…k. added tests" Signed-off-by: Matt Peterson <[email protected]>

server/src/main/java/com/hedera/block/server/consumer/LiveStreamObserverImpl.java

Signed-off-by: Matt Peterson <[email protected]>

server/src/main/java/com/hedera/block/server/consumer/LiveStreamObserverImpl.java

…re line to prevent that from being added Signed-off-by: Matt Peterson <[email protected]>

Signed-off-by: Matt Peterson <[email protected]>

…onsumers Signed-off-by: Matt Peterson <[email protected]>

Signed-off-by: Matt Peterson <[email protected]>

…of the proto file Signed-off-by: Matt Peterson <[email protected]>

Signed-off-by: Matt Peterson <[email protected]>

mattp-swirldslabs · 2024-06-27T21:27:00Z

Great start, happy to see the PR! Another thing I'm missing is some kind of design documentation to give some orientation around what the different components are.

Hey @rbair23, given the expected refactoring of this approach, do you mind if I submit the design doc with the PR for that effort?

server/src/main/java/com/hedera/block/server/consumer/LiveStreamObserverImpl.java

Signed-off-by: Matt Peterson <[email protected]>

server/src/main/java/com/hedera/block/server/Server.java

buildSrc/src/main/kotlin/com.hedera.block.jpms-modules.gradle.kts

Signed-off-by: Matt Peterson <[email protected]>

AlfredoG87

LGTM, only a few nits...

buildSrc/src/main/kotlin/com.hedera.block.jpms-modules.gradle.kts

server/build.gradle.kts

server/src/main/java/com/hedera/block/server/Server.java

Signed-off-by: Matt Peterson <[email protected]>

AlfredoG87

LGTM

Nana-EC

Nice work.
Some considerations for future PRs and backlog tickets

Nana-EC · 2024-06-28T22:03:19Z

server/src/main/java/com/hedera/block/server/Server.java


-    static void grpcEcho(EchoServiceGrpcProto.EchoRequest request, StreamObserver<EchoServiceGrpcProto.EchoResponse> responseObserver) {}
+            // TODO: Make timeoutThresholdMillis configurable


Please turn this into a ticket on the backlog. Thanks

Nana-EC · 2024-06-28T22:10:16Z

server/src/main/java/com/hedera/block/server/Constants.java

+/**
+ * Constants used in the BlockNode service.
+ */
+public final class Constants {


Q: Would ServiceContracts be more appropriate.
I feel like this is one of those files that will get overloaded with many values over time

Nana-EC · 2024-06-28T22:11:24Z

server/src/main/java/com/hedera/block/server/Server.java


-    static void grpcEcho(EchoServiceGrpcProto.EchoRequest request, StreamObserver<EchoServiceGrpcProto.EchoResponse> responseObserver) {}
+            // TODO: Make timeoutThresholdMillis configurable
+            final BlockStreamService blockStreamService = new BlockStreamService(1500,


1500 should be a configureable value

Nana-EC · 2024-06-28T22:11:36Z

server/src/main/java/com/hedera/block/server/Server.java

+
+            // Start the web server
+            WebServer.builder()
+                    .port(8080)


Expose as a configureable value

Nana-EC · 2024-06-28T23:24:17Z

server/src/main/java/com/hedera/block/server/consumer/LiveStreamObserverImpl.java

+        this.mediator = mediator;
+        this.responseStreamObserver = responseStreamObserver;
+
+        this.producerLivenessMillis = producerLivenessClock.millis();


Q: This is the part you noted helidon doesn't handle itself huh so we need to implement our own custom solution to determine timeouts. Sigh.

We should open an issue on helidon for this.
if we're luck it might get resolved and we can remove this logic in the future

Nana-EC · 2024-06-28T23:33:19Z

server/src/main/java/com/hedera/block/server/mediator/LiveStreamMediatorImpl.java

+     */
+    @Override
+    public void unsubscribe(final LiveStreamObserver<BlockStreamServiceGrpcProto.Block, BlockStreamServiceGrpcProto.BlockResponse> liveStreamObserver) {
+        if (subscribers.remove(liveStreamObserver)) {


If the removal fails is there some backup step we should take?
Does this hold up a slot that is unclaimable?
Should we at least warn or something?

Nana-EC · 2024-06-28T23:48:47Z

server/src/main/java/com/hedera/block/server/mediator/LiveStreamMediatorImpl.java

+        }
+
+        // Persist the block
+        blockPersistenceHandler.persist(block);


Q: Should we consider splitting out persistence into it's own StreamMediator or something appropriate?
I don't like the idea that the block doesn't get persisted if there are any issues streaming to subscribers.
Even if we don't pull it out we should persist first.

Nana-EC · 2024-06-29T00:04:57Z

server/src/main/java/com/hedera/block/server/persistence/storage/FileSystemBlockStorage.java

+    private String resolvePath(final Long id) {
+
+        String fileName = id + BLOCK_FILE_EXTENSION;
+        Path fullPath = blockNodeRootPath.resolve(fileName);


Later item, we'll wanna explore a directory structure to avoid having one flat dir with all files.
Maybe some multiple to help map

mattp-swirldslabs self-assigned this Jun 14, 2024

mattp-swirldslabs force-pushed the peterson-initial-stream-support branch 2 times, most recently from 1f19410 to 1e9dbfb Compare June 20, 2024 21:43

Issues 16, 17, 18 and 21

0608e65

- added license headers to java files - added bidirectional gRPC live streaming blocks implementation with 1 producer and N consumers Signed-off-by: Matt Peterson <[email protected]>

mattp-swirldslabs force-pushed the peterson-initial-stream-support branch from 1e9dbfb to 0608e65 Compare June 25, 2024 16:18

mattp-swirldslabs marked this pull request as ready for review June 25, 2024 16:25

mattp-swirldslabs requested review from a team as code owners June 25, 2024 16:25

mattp-swirldslabs requested review from matteriben and jeromy-cannon June 25, 2024 16:25

mattp-swirldslabs changed the title ~~Stream processing support~~ Live Stream processing support Jun 25, 2024

rbair23 requested changes Jun 25, 2024

View reviewed changes

mattp-swirldslabs added 7 commits June 25, 2024 11:56

fix: logger to LOGGER per code review

1916732

Signed-off-by: Matt Peterson <[email protected]>

fix: missed a Logger entry

c24dd2b

Signed-off-by: Matt Peterson <[email protected]>

fix: removed extra javadoc line

07df87d

Signed-off-by: Matt Peterson <[email protected]>

fix: added final keyword per code review requests

cd7ad67

Signed-off-by: Matt Peterson <[email protected]>

fix: trimmed Empty protobuf definition

f7f037b

Signed-off-by: Matt Peterson <[email protected]>

fix: trimmed imports

3e73a5d

Signed-off-by: Matt Peterson <[email protected]>

fix: changed from JUL to System.Logger

d60a907

Signed-off-by: Matt Peterson <[email protected]>

jsync-swirlds reviewed Jun 25, 2024

View reviewed changes

mattp-swirldslabs added 5 commits June 25, 2024 15:24

fix: removing this prefixes when they are not needed

2f51579

Signed-off-by: Matt Peterson <[email protected]>

fixed: java file license headers

678d527

Signed-off-by: Matt Peterson <[email protected]>

fix: removing hyphens and javadoc message grooming

34d6acc

Signed-off-by: Matt Peterson <[email protected]>

fix: adjusted error logging to log a throwable as a separate parameter

95e4da4

Signed-off-by: Matt Peterson <[email protected]>

fix: enabled mockito. changed LiveStreamObserverImpl to use Java Cloc…

48c8d90

…k. added tests" Signed-off-by: Matt Peterson <[email protected]>

jsync-swirlds reviewed Jun 26, 2024

View reviewed changes

server/src/main/java/com/hedera/block/server/consumer/LiveStreamObserverImpl.java Outdated Show resolved Hide resolved

fix: removed unnecessary static requires

606757c

Signed-off-by: Matt Peterson <[email protected]>

jsync-swirlds reviewed Jun 26, 2024

View reviewed changes

server/src/main/java/com/hedera/block/server/consumer/LiveStreamObserverImpl.java Outdated Show resolved Hide resolved

mattp-swirldslabs added 12 commits June 27, 2024 12:39

fix: removed the manual scripts which write data.txt and the .gitigno…

f25f2b2

…re line to prevent that from being added Signed-off-by: Matt Peterson <[email protected]>

fix: added API specification documentation for blockstream.proto

def0452

Signed-off-by: Matt Peterson <[email protected]>

fix: added array wrap-around functionality for the TestClock

babcb8a

Signed-off-by: Matt Peterson <[email protected]>

fix: added underscores to the TEST_TIME to improve readability

6d3e617

Signed-off-by: Matt Peterson <[email protected]>

fix: removed unsubscribeall. we do not want a producer to evict all c…

0418475

…onsumers Signed-off-by: Matt Peterson <[email protected]>

fix: replaced type with var to improve readability

2f348e5

Signed-off-by: Matt Peterson <[email protected]>

fix: allow the map item to be updated

4c46aa3

Signed-off-by: Matt Peterson <[email protected]>

fix: remove caching

6ff6a8d

Signed-off-by: Matt Peterson <[email protected]>

fixed: switched queue implementation details

1e3386e

Signed-off-by: Matt Peterson <[email protected]>

fixed: added error message

c623b48

Signed-off-by: Matt Peterson <[email protected]>

fix: added additional documentation regarding the service definition …

59e60fc

…of the proto file Signed-off-by: Matt Peterson <[email protected]>

fix: added additional method level documentation

a03863f

Signed-off-by: Matt Peterson <[email protected]>

jsync-swirlds reviewed Jun 27, 2024

View reviewed changes

server/src/main/java/com/hedera/block/server/consumer/LiveStreamObserverImpl.java Outdated Show resolved Hide resolved

mattp-swirldslabs added 2 commits June 28, 2024 08:07

fix: adjusted logging to emit a message and the stacktrace

59bd8e3

Signed-off-by: Matt Peterson <[email protected]>

upgrade: upgraded to Helidon 4.0.10

95eb138

Signed-off-by: Matt Peterson <[email protected]>

rbair23 previously approved these changes Jun 28, 2024

View reviewed changes

server/src/main/java/com/hedera/block/server/Server.java Outdated Show resolved Hide resolved

buildSrc/src/main/kotlin/com.hedera.block.jpms-modules.gradle.kts Outdated Show resolved Hide resolved

fix: removed helidon greet http routing

8d22594

Signed-off-by: Matt Peterson <[email protected]>

mattp-swirldslabs dismissed rbair23’s stale review via 8d22594 June 28, 2024 19:10

fix: added end of file newline

6d3bfe6

Signed-off-by: Matt Peterson <[email protected]>

AlfredoG87 previously approved these changes Jun 28, 2024

View reviewed changes

buildSrc/src/main/kotlin/com.hedera.block.jpms-modules.gradle.kts Outdated Show resolved Hide resolved

server/build.gradle.kts Outdated Show resolved Hide resolved

server/src/main/java/com/hedera/block/server/Server.java Show resolved Hide resolved

fix: added TODO to make hardcoded value configurable

845c4bd

Signed-off-by: Matt Peterson <[email protected]>

mattp-swirldslabs dismissed AlfredoG87’s stale review via 845c4bd June 28, 2024 21:28

fix: added newline

e87e233

Signed-off-by: Matt Peterson <[email protected]>

nathanklick approved these changes Jun 28, 2024

View reviewed changes

mattp-swirldslabs requested a review from AlfredoG87 June 28, 2024 21:47

AlfredoG87 approved these changes Jun 28, 2024

View reviewed changes

mattp-swirldslabs merged commit 8e98b5b into main Jun 28, 2024
2 checks passed

mattp-swirldslabs deleted the peterson-initial-stream-support branch June 28, 2024 21:58

Nana-EC reviewed Jun 29, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Live Stream processing support #23

Live Stream processing support #23

mattp-swirldslabs commented Jun 14, 2024 •

edited by Nana-EC

Loading

rbair23 left a comment

rbair23 commented Jun 25, 2024

jsync-swirlds left a comment

mattp-swirldslabs commented Jun 27, 2024 •

edited

Loading

AlfredoG87 left a comment

AlfredoG87 left a comment

Nana-EC left a comment

Nana-EC Jun 28, 2024

Nana-EC Jun 28, 2024

Nana-EC Jun 28, 2024

Nana-EC Jun 28, 2024

Nana-EC Jun 28, 2024

Nana-EC Jun 28, 2024

Nana-EC Jun 28, 2024

Nana-EC Jun 29, 2024


		static void grpcEcho(EchoServiceGrpcProto.EchoRequest request, StreamObserver<EchoServiceGrpcProto.EchoResponse> responseObserver) {}
		// TODO: Make timeoutThresholdMillis configurable

Live Stream processing support #23

Live Stream processing support #23

Conversation

mattp-swirldslabs commented Jun 14, 2024 • edited by Nana-EC Loading

rbair23 left a comment

Choose a reason for hiding this comment

rbair23 commented Jun 25, 2024

jsync-swirlds left a comment

Choose a reason for hiding this comment

mattp-swirldslabs commented Jun 27, 2024 • edited Loading

AlfredoG87 left a comment

Choose a reason for hiding this comment

AlfredoG87 left a comment

Choose a reason for hiding this comment

Nana-EC left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattp-swirldslabs commented Jun 14, 2024 •

edited by Nana-EC

Loading

mattp-swirldslabs commented Jun 27, 2024 •

edited

Loading