add startAsync and finishAsync on client side, add onStarted and onFinished on server side #53

coderzc · 2021-05-20T10:36:58Z

add startAsync and finishAsync on client side
add onStarted and onFinished on server side

codecov · 2021-05-20T10:41:29Z

Codecov Report

Merging #53 (61d5404) into master (aaeec64) will increase coverage by 0.00%.
The diff coverage is 80.64%.

@@ Coverage Diff @@ ## master #53 +/- ## ========================================= Coverage 85.75% 85.76% - Complexity 1490 1494 +4  ========================================= Files 157 157 Lines 4914 4952 +38 Branches 405 411 +6 ========================================= + Hits 4214 4247 +33  + Misses 465 463 -2  - Partials 235 242 +7

Impacted Files	Coverage Δ
...h/computer/core/network/session/ClientSession.java	`84.42% <76.00%> (-3.67%)`	⬇️
...omputer/core/network/netty/NettyClientHandler.java	`72.97% <100.00%> (ø)`
...omputer/core/network/netty/NettyServerHandler.java	`87.34% <100.00%> (+0.67%)`	⬆️
...puter/core/network/netty/NettyTransportClient.java	`98.11% <100.00%> (+6.88%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aaeec64...61d5404. Read the comment docs.

javeme · 2021-05-26T07:17:02Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/network/TransportClient.java

 void startSession() throws TransportException;

+ /**
+ * This method is asynchronous startSession.


This method is the asynchronous version of #startSession().

javeme · 2021-05-26T07:17:12Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/network/TransportClient.java

 void finishSession() throws TransportException;

+ /**
+ * This method is asynchronous finishSession.


javeme · 2021-05-26T07:17:39Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/network/TransportClient.java

+ /**
+ * This method is asynchronous startSession.
+ */
+ Future<Boolean> startSessionAsync() throws TransportException;


return Future

javeme · 2021-05-26T07:21:31Z

...uter-core/src/main/java/com/baidu/hugegraph/computer/core/network/session/ClientSession.java

 "at start()", this.state);

+ SettableFuture<Boolean> startedFuture = SettableFuture.create();
+ this.startedFutureRef.set(startedFuture);


call compareAndSet(null, startedFuture), throw exception if the previous call is not completed

javeme · 2021-05-26T07:28:59Z

...uter-core/src/main/java/com/baidu/hugegraph/computer/core/network/session/ClientSession.java

+ SettableFuture<Boolean> settableFuture = this.startedFutureRef.get();
+ if (settableFuture != null && !settableFuture.isCancelled()) {
+ settableFuture.set(true);
+ this.startedFutureRef.compareAndSet(settableFuture, null);


if (settableFuture != null) { if (!settableFuture.isCancelled()) settableFuture.set(true); this.startedFutureRef.compareAndSet(settableFuture, null); }

javeme · 2021-05-26T07:29:15Z

...uter-core/src/main/java/com/baidu/hugegraph/computer/core/network/session/ClientSession.java


- this.finishedBarrier.signalAll();
+ SettableFuture<Boolean> finishedFuture = this.finishedFutureRef.get();
+ if (finishedFuture != null && !finishedFuture.isCancelled()) {


javeme · 2021-05-27T05:52:58Z

...uter-core/src/main/java/com/baidu/hugegraph/computer/core/network/session/ClientSession.java

+ SettableFuture<Void> startedFuture = SettableFuture.create();
+ boolean success = this.startedFutureRef.compareAndSet(null,
+ startedFuture);
+ E.checkArgument(success, "The startedFutureRef must be null");


add error context

javeme · 2021-05-27T05:53:06Z

...uter-core/src/main/java/com/baidu/hugegraph/computer/core/network/session/ClientSession.java

+ SettableFuture<Void> finishedFuture = SettableFuture.create();
+ boolean success = this.finishedFutureRef.compareAndSet(null,
+ finishedFuture);
+ E.checkArgument(success, "The finishedFutureRef must be null");


javeme · 2021-05-27T05:56:12Z

...uter-core/src/main/java/com/baidu/hugegraph/computer/core/network/session/ClientSession.java

 private volatile boolean flowBlocking;
- private final BarrierEvent startedBarrier;
- private final BarrierEvent finishedBarrier;
+ private final AtomicReference<SettableFuture<Void>> startedFutureRef;


use Future<Void>?

need to call set()

Linary · 2021-05-28T07:38:42Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/network/MessageHandler.java

 void handle(MessageType messageType, int partition, ManagedBuffer buffer);
+
+ /**
+ * Notice start session completed.


Notice -> Notify

Linary · 2021-05-28T07:44:50Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/network/TransportClient.java

+ /**
+ * This method is the asynchronous version of {@link #startSession}.
+ */
+ Future<Void> startSessionAsync() throws TransportException;


return CompletableFuture

Linary · 2021-05-28T07:45:35Z

...uter-core/src/main/java/com/baidu/hugegraph/computer/core/network/session/ClientSession.java

 private volatile boolean flowBlocking;
- private final BarrierEvent startedBarrier;
- private final BarrierEvent finishedBarrier;
+ private final AtomicReference<SettableFuture<Void>> startedFutureRef;


use CompletableFuture

Linary · 2021-05-28T07:46:21Z

...uter-core/src/main/java/com/baidu/hugegraph/computer/core/network/session/ClientSession.java

+ SettableFuture<Void> settableFuture = this.startedFutureRef.get();
+ if (settableFuture != null) {
+ if (!settableFuture.isCancelled()) {
+ settableFuture.set(null);


CompletableFuture.complete(null)

Linary · 2021-05-28T10:52:20Z

...uter-core/src/main/java/com/baidu/hugegraph/computer/core/network/session/ClientSession.java

- public synchronized void start(long timeout) throws TransportException,
- InterruptedException {
+ public synchronized void start(long timeout) throws TransportException {
+ CompletableFuture<Void> startFuture = startAsync();


Linary · 2021-05-28T11:49:22Z

...uter-core/src/main/java/com/baidu/hugegraph/computer/core/network/session/ClientSession.java


 public synchronized void finish(long timeout) throws TransportException {
- Future<Void> finishFuture = finishAsync();
+ CompletableFuture<Void> finishFuture = finishAsync();


javeme · 2021-05-29T03:27:59Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/network/TransportClient.java

 void startSession() throws TransportException;

+ /**
+ * This method is the asynchronous version of {@link #startSession}.


startSession()

javeme · 2021-05-29T03:28:08Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/network/TransportClient.java

 void finishSession() throws TransportException;

+ /**
+ * This method is the asynchronous version of {@link #finishSession}.


finishSession()

javeme · 2021-05-29T03:31:05Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/network/MessageHandler.java

 void handle(MessageType messageType, int partition, ManagedBuffer buffer);
+
+ /**
+ * Notify start session completed.


Notify start-session completed on server-side.

javeme · 2021-05-29T03:31:10Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/network/MessageHandler.java

+ void onStarted(ConnectionId connectionId);
+
+ /**
+ * Notify finish session completed.


javeme · 2021-05-29T03:32:18Z

computer-test/src/main/java/com/baidu/hugegraph/computer/core/network/MockMessageHandler.java

+ }
+
 @Override
 public void channelActive(ConnectionId connectionId) {


also renamed to onChannelActive

houzhizhen · 2021-05-31T02:13:10Z

...r-core/src/main/java/com/baidu/hugegraph/computer/core/network/netty/NettyServerHandler.java

 ctx.writeAndFlush(startAck).addListener(this.listenerOnWrite);
 this.serverSession.completeStateStart();

+ Channel channel = ctx.channel();


this.handler.onStarted() should be called in #processStartMessage() instead of #ackStartMessage(),

need to notify when receive StartMessage?

houzhizhen · 2021-05-31T02:17:58Z

...uter-core/src/main/java/com/baidu/hugegraph/computer/core/network/session/ClientSession.java

+ boolean success = this.startedFutureRef.compareAndSet(null,
+ startedFuture);
+ E.checkArgument(success,
+ "The startedFutureRef value must be null at start()");


start() -> #startAsync() ?

houzhizhen · 2021-05-31T02:20:28Z

...uter-core/src/main/java/com/baidu/hugegraph/computer/core/network/session/ClientSession.java

+ boolean success = this.finishedFutureRef.compareAndSet(null,
+ finishedFuture);
+ E.checkArgument(success,
+ "The finishedFutureRef value must be null at finish()");


#finishAsync() ?

* add onStarted and onFinished on server side

coderzc force-pushed the transport-extended branch from 61b9724 to a65aeef Compare May 20, 2021 10:39

javeme reviewed May 26, 2021

View reviewed changes

javeme reviewed May 27, 2021

View reviewed changes

javeme previously approved these changes May 27, 2021

View reviewed changes

coderzc requested review from Linary and houzhizhen May 28, 2021 05:45

Linary requested changes May 28, 2021

View reviewed changes

coderzc dismissed javeme’s stale review via a46ab33 May 28, 2021 08:17

coderzc force-pushed the transport-extended branch from a46ab33 to a87a91a Compare May 28, 2021 08:31

Linary requested changes May 28, 2021

View reviewed changes

javeme reviewed May 29, 2021

View reviewed changes

houzhizhen reviewed May 31, 2021

View reviewed changes

javeme previously approved these changes May 31, 2021

View reviewed changes

coderzc added 11 commits May 31, 2021 12:57

improve code style

f92b970

* add startAsync and finishAsync on client side

8fb2a04

* add onStarted and onFinished on server side

add test

70c641d

add test

5cea694

improve code style

94f9b47

improve some comment

e6f7d39

fix code

86c1885

improve message

d894f24

SettableFuture -> CompletableFuture

39020b3

improve some comment

9ba32f3

improve some comment

67f6383

coderzc dismissed javeme’s stale review via 67f6383 May 31, 2021 04:59

coderzc force-pushed the transport-extended branch 2 times, most recently from 812b846 to d51ab34 Compare May 31, 2021 05:55

fix code

61d5404

coderzc force-pushed the transport-extended branch from d51ab34 to 61d5404 Compare May 31, 2021 06:13

houzhizhen approved these changes May 31, 2021

View reviewed changes

Linary approved these changes May 31, 2021

View reviewed changes

houzhizhen merged commit bfb8826 into master May 31, 2021

houzhizhen deleted the transport-extended branch June 15, 2021 02:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

add startAsync and finishAsync on client side, add onStarted and onFinished on server side #53

add startAsync and finishAsync on client side, add onStarted and onFinished on server side #53

Uh oh!

coderzc commented May 20, 2021

codecov bot commented May 20, 2021 •

edited

Loading

javeme May 26, 2021

javeme May 26, 2021

javeme May 26, 2021

javeme May 26, 2021

javeme May 26, 2021

javeme May 26, 2021

javeme May 27, 2021

javeme May 27, 2021

javeme May 27, 2021

coderzc May 27, 2021 •

edited

Loading

Linary May 28, 2021

Linary May 28, 2021

Linary May 28, 2021

Linary May 28, 2021

Linary May 28, 2021

Linary May 28, 2021

javeme May 29, 2021

javeme May 29, 2021

javeme May 29, 2021

javeme May 29, 2021

javeme May 29, 2021

houzhizhen May 31, 2021

coderzc May 31, 2021

houzhizhen May 31, 2021

houzhizhen May 31, 2021

Labels

4 participants

add startAsync and finishAsync on client side, add onStarted and onFinished on server side #53

add startAsync and finishAsync on client side, add onStarted and onFinished on server side #53

Uh oh!

Conversation

coderzc commented May 20, 2021

codecov bot commented May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

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

coderzc May 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

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

Labels

4 participants

codecov bot commented May 20, 2021 •

edited

Loading

coderzc May 27, 2021 •

edited

Loading