`BarrageSession` Subscription/Snapshot Methods now Return `Future<Table>` #4676

nbauernfeind · 2023-10-19T21:28:29Z

This includes breaking changes to the barrage java-client API. The snapshot/subscription methods now return Future<Table> instead of BarrageTable and users are no longer required to blockUntilCompletion as this is handled by Future#get.

Subscribed tables can now be canceled only through Liveness destruction such as using an explicit LivenessScope or invoking ReferenceCounted#forceReferenceCountToZero().

…hing

extensions/barrage/src/main/java/io/deephaven/extensions/barrage/util/GrpcUtil.java

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSnapshotImpl.java

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSubscriptionImpl.java

Co-authored-by: Ryan Caudy <[email protected]>

extensions/barrage/src/main/java/io/deephaven/extensions/barrage/table/BarrageTable.java

extensions/barrage/src/main/java/io/deephaven/extensions/barrage/util/GrpcUtil.java

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSubscriptionImpl.java

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSnapshotImpl.java

extensions/barrage/src/main/java/io/deephaven/extensions/barrage/table/BarrageTable.java

...-client/barrage-examples/src/main/java/io/deephaven/client/examples/SnapshotExampleBase.java

...client/barrage-examples/src/main/java/io/deephaven/client/examples/SubscribeExampleBase.java

...-client/barrage-examples/src/main/java/io/deephaven/client/examples/SnapshotExampleBase.java

rcaudy · 2023-10-20T23:35:21Z

Approved, but needs thorough testing before we merge.

We found a bug

…ging on successful snapshot via subscription

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSnapshot.java

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSnapshotImpl.java

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSubscriptionImpl.java

…rageSubscriptionImpl.java Ryan's CR Suggestion Co-authored-by: Ryan Caudy <[email protected]>

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSnapshot.java

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSubscriptionImpl.java

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSnapshotImpl.java

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSubscriptionImpl.java

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSnapshotImpl.java

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSnapshot.java

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSubscription.java

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSubscriptionImpl.java

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSnapshotImpl.java

...tegraph/src/main/java/io/deephaven/engine/updategraph/UpdateGraphAwareCompletableFuture.java

Ryan's inline suggestions. Co-authored-by: Ryan Caudy <[email protected]>

rcaudy

Two cosmetic complaints. No need to make further changes if tests pass.

...tegraph/src/main/java/io/deephaven/engine/updategraph/UpdateGraphAwareCompletableFuture.java

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSubscriptionImpl.java

Fix BarrageSession#snapshot Use-Case; Do Not Make BarrageTable Refres…

8a8c72b

…hing

nbauernfeind added bug Something isn't working NoDocumentationNeeded barrage-wrkr2wrkr ReleaseNotesNeeded Release notes are needed labels Oct 19, 2023

nbauernfeind added this to the September 2023 milestone Oct 19, 2023

nbauernfeind requested a review from rcaudy October 19, 2023 21:28

nbauernfeind self-assigned this Oct 19, 2023

stanbrub mentioned this pull request Oct 19, 2023

Barrage Snapshot DHC Breaking Change deephaven/benchmark#194

Closed

Unblock listeners When Snapshot/Subscription Closed by Caller

cea8bbb

nbauernfeind force-pushed the barrage_snap branch from 3affb38 to cea8bbb Compare October 20, 2023 16:23

rcaudy reviewed Oct 20, 2023

View reviewed changes

nbauernfeind and others added 3 commits October 20, 2023 11:05

Apply ryan's javadoc suggestions from code review

3968db5

Co-authored-by: Ryan Caudy <[email protected]>

Ryan's feedback

c91dab3

missed calls to unsubscribe

ba263d5

rcaudy reviewed Oct 20, 2023

View reviewed changes

Ryan's Pair Programming

ef27ef6

rcaudy reviewed Oct 20, 2023

View reviewed changes

nbauernfeind added 2 commits October 20, 2023 16:56

Fix compile errors

539da1b

Ryan's Review Rnd3

1917cb2

rcaudy previously approved these changes Oct 20, 2023

View reviewed changes

nbauernfeind added 2 commits October 22, 2023 18:34

Seperate Connected from Completed in BarrageSnapshotImpl

b09c3d7

Fix remaining two use cases: snapshotting failed table and excess log…

2d7b6d9

…ging on successful snapshot via subscription

rcaudy reviewed Oct 23, 2023

View reviewed changes

nbauernfeind and others added 2 commits October 23, 2023 08:28

Update java-client/barrage/src/main/java/io/deephaven/client/impl/Bar…

2f03823

…rageSubscriptionImpl.java Ryan's CR Suggestion Co-authored-by: Ryan Caudy <[email protected]>

Return Future<Table> from BarrageSnapshot and BarrageSubscription

2c4d72c

nbauernfeind requested a review from rcaudy October 23, 2023 18:12

nbauernfeind added the breaking label Oct 23, 2023

nbauernfeind added the java-client label Oct 23, 2023

rcaudy reviewed Oct 23, 2023

View reviewed changes

Ryan's Feedback on Latest

a77c2db

rcaudy reviewed Oct 23, 2023

View reviewed changes

nbauernfeind and others added 2 commits October 23, 2023 14:39

Apply suggestions from code review

439f64a

Ryan's inline suggestions. Co-authored-by: Ryan Caudy <[email protected]>

Ryan's Feedback on Mon Rnd 3

c163316

rcaudy previously approved these changes Oct 23, 2023

View reviewed changes

...tegraph/src/main/java/io/deephaven/engine/updategraph/UpdateGraphAwareCompletableFuture.java Outdated Show resolved Hide resolved

java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSubscriptionImpl.java Outdated Show resolved Hide resolved

Minor Misses

5d73734

nbauernfeind dismissed rcaudy’s stale review via 5d73734 October 23, 2023 21:31

nbauernfeind changed the title ~~Fix BarrageSession#snapshot Use-Case; Do Not Make BarrageTable Refreshing~~ BarrageSession Subscription/Snapshot Methods now Return Future<Table> Oct 23, 2023

nbauernfeind enabled auto-merge (squash) October 23, 2023 21:44

rcaudy approved these changes Oct 23, 2023

View reviewed changes

nbauernfeind merged commit 43c10f4 into deephaven:main Oct 23, 2023

github-actions bot locked and limited conversation to collaborators Oct 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`BarrageSession` Subscription/Snapshot Methods now Return `Future<Table>` #4676

`BarrageSession` Subscription/Snapshot Methods now Return `Future<Table>` #4676

nbauernfeind commented Oct 19, 2023 •

edited

Loading

rcaudy commented Oct 20, 2023

rcaudy left a comment

BarrageSession Subscription/Snapshot Methods now Return Future<Table> #4676

BarrageSession Subscription/Snapshot Methods now Return Future<Table> #4676

Conversation

nbauernfeind commented Oct 19, 2023 • edited Loading

rcaudy commented Oct 20, 2023

rcaudy left a comment

Choose a reason for hiding this comment

`BarrageSession` Subscription/Snapshot Methods now Return `Future<Table>` #4676

`BarrageSession` Subscription/Snapshot Methods now Return `Future<Table>` #4676

nbauernfeind commented Oct 19, 2023 •

edited

Loading