-
Notifications
You must be signed in to change notification settings - Fork 356
provides an example of peer to peer communication #868
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PeerToPeerCommunicationExample
is very general. Something a little more specific for the use case perhaps like TaskProcessingServerWithNotificationsExample
or similar.
For the package, we currently have the duplex
package which also shows a server-to-client interaction although a very basic one. Perhaps we can add this sample to the same package and give the package a better name like clientresponder
or servertoclient
. The word duplex
to me is a little ambiguous (since a channel interaction is also duplex) and not as clear as it could be.
...ples/src/main/java/io/rsocket/examples/transport/tcp/p2p/PeerToPeerCommunicationExample.java
Outdated
Show resolved
Hide resolved
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import reactor.core.publisher.Mono; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A class-level Javadoc with an overview of the idea behind the sample would be very useful. For example based on the #867 (comment):
Example of a long-running process that handles tasks (a.k.a Kafka style) where a
client submits tasks viafireAndForget
and then receives notifications also via
fireAndForget
from the server side.In the example, the client sends some tasks initially, then disappears, then connects
again and receives undelivered that have completed in the mean time.
tasksToDeliver.offer(task); | ||
}); | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this can be consolidated into the background process which already polls and sends fireAndForget. That would simplify the TasksAcceptor significantly and perhaps it could be even inlined at the point of creating the server.
As a consequence it might take up to 2 seconds for the client to catch up but that seems okay. Also server-to-client notifications could delivered as a list (or comama-separated String) perhaps, i.e. as a batch rather than one at a time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved it to a separate method now
...ples/src/main/java/io/rsocket/examples/transport/tcp/p2p/PeerToPeerCommunicationExample.java
Outdated
Show resolved
Hide resolved
@rstoyanchev duplex is less obvious. I stands for renaming it to p2p since it is more familiar to everyone. Duplex can easily be mixed with a channel which kind of duplex channel communication |
68da6d5
to
d3d1e5b
Compare
d3d1e5b
to
d92e549
Compare
Signed-off-by: Oleh Dokuka <[email protected]>
Signed-off-by: Oleh Dokuka <[email protected]>
d92e549
to
6ed178a
Compare
closes #867