Skip to content

Conversation

@stevegury
Copy link
Member

Problem
The current multi repo organization of reactivesocket is annoying to use, it
forces us to publish jars to test libraries.

Solution
Migrate all the subprojects from reactivesocket-java-impl into reactivesocket-java.
To avoid having version number conflict, and to start a convention, I renamed the
projects like this:

  • reactivesocket => rs-core
  • reactivesocket-aeron => rs-transport-aeron
  • reactivesocket-jsr-356 => rs-transport-jsr-356
  • reactivesocket-local => rs-transport-local
  • reactivesocket-mime-types => rs-mime-types
  • reactivesocket-netty => rs-transport-netty
  • reactivesocket-test => rs-test

Note
Both git histories are kept.

robertroeser and others added 30 commits August 10, 2015 11:58
first commit
…ffer to send a message if the message is larger than the MTU size. Created a AeronClientDuplex connection
added code to establish a connection. Publish operator will now use o…
updating to reactive socket 1.0.1
updated to match reactive socket java changes
robertroeser and others added 23 commits May 2, 2016 16:52
throwing a TransportException when a connection is closed
* aeron client can run embededded media driver so you don't have to run the external driver

* added toString methods for duplex connections for debugging and access log purposes
changed constructor access level to protected to enable subclassing
This module provides support for encoding/decoding ReactiveSocket data and metadata into using different mime types as defined by [ReactiveSocket protocol](https://github.com/ReactiveSocket/reactivesocket/blob/master/Protocol.md#setup-frame).
The support for mime types is not comprehensive but it will atleast support the [default metadata mime type](https://github.com/ReactiveSocket/reactivesocket/blob/mimetypes/MimeTypes.md)

See README.md for usage.
For default metadata mime-type, using Jackson CBOR codec, produces more allocations (one buffer allocation per value) which are unnecessary. This was the major driving factor to invest in a _simple_ codec that can reduce allocations and be more suitable for ReactiveSocket design (using thread-local pooled buffers).

Although, the codec is intended to be used only for a map type, but it is rather generic to be used for a majority of CBOR data types. If this proves useful, we can extend this for the remaining data types.
Problem
When a TransportException happens on the transport, it is propagated up
in the chain but the Susbscription is not cancelled. Then, any source
associated with the chain continue to emit events.

One example of that is the Keep-Alive Observable, which regularly emit
keep-alive messages at fixed interval, when the connection is closed, the
observable has to be cancelled.

Solution
In all DuplexConnection implementation, cancel the Subscription when we
saw a TransportException.
…lation

Propagate cancellation when a TransportException occurs
* Refactor Factory to Connector + Implement availability

Problem
We need to comply with the recent refactoring "Factory to Connector" introduced
in reactivesocket-java. Also, implementation of DuplexConnection have to return
an availability.

Solution
The Factory to Connector refactoring is pretty straight forward.

All implementations of DuplexConnection return an availability which directly
map the state of the underlying resource (0.0 if the resource is closed, 1.0
otherwise). This will greatly help the load-balancer to select a valid
connection.

I removed the blocking method from reactivesocket-java, so I moved some utility
blocking code inside the TestUtil class.

Clean-up of the Netty implementation, remove unused args, explicitey specify
tcp socket configuration.

Shortened the toString name to ease log-reading.

Bug
All DuplexConnection implementations now cancel the subscription when an exception
occurs. This fix the problem, where the Keep-Alive Observable kept trying to
send a keep-alive on a closed connection.

* Refactor the TestUtil helper to return CompletableFuture

* rs version

* Restore rs dependency to latest.release

* Remove mavenLocal in gradle build file

* Propagate cancellation in all cases

* Use helper method from ReactiveSocket Unsafe
@NiteshKant
Copy link
Contributor

This is cool and will reduce a bunch of headaches :)

Few questions:

  • Are we using rs-jsr-356 anywhere?
  • rs-transport-netty sounds wrong as it is along with rs-transport-aeron which is a protocol and -netty is not. I suggested in a different PR that we should split it into rs-transport-tcp and rs-transport-websocket. -websocket can depend on -tcp (since it is over tcp) if we need to share common code.
  • Should we create a rs-skunkworks (rs-experimental?) module/project which can contain -jsr-356 and other experimental projects that we have been talking about.
  • Should we keep "-java" in the module names?

You do not have to do all the changes in this PR, we can merge this (I don't think anyone can review :)) and then do incremental changes. I just thought it will be good to discuss this since we are doing the restructuring.

@stevegury
Copy link
Member Author

Are we using rs-jsr-356 anywhere?

I don't think so, @rdegnan can confirm.
I agree that if it's the case, we should move it elsewhere.

rs-transport-netty sounds wrong as it is along with rs-transport-aeron which is a protocol and -netty is not. I suggested in a different PR that we should split it into rs-transport-tcp and rs-transport-websocket. -websocket can depend on -tcp (since it is over tcp) if we need to share common code.

Again, I agree. I hesitated to do exactly this but I wanted to limit the already large PR.
I will submit a subsequent PR with your proposal.

Should we create a rs-skunkworks (rs-experimental?) module/project which can contain -jsr-356 and other experimental projects that we have been talking about.

Do you mean a subdirectory containing multiple submodules?

  • Should we keep "-java" in the module names?

We don't have "-java" in the module name, only in the name of the repo.
Under the groupId "io.reactivesocket" we currently generate those artifact-id:

  • rs-core
  • rs-transport-aeron
  • rs-transport-jsr-356
  • rs-transport-local
  • rs-mime-types
  • rs-transport-netty
  • rs-test

It will change after I split -netty into -tcp and -websocket

In order to keep the reactivesocket name and simplify searching in maven central,
rename all the 'rs-*' project into 'reactivesocket-*'.
@stevegury
Copy link
Member Author

stevegury commented Jun 9, 2016

Per offline discussion, I renamed the "rs-" project into "reactivesocket-"

@robertroeser robertroeser merged commit e83d1f9 into master Jun 9, 2016
@stevegury stevegury deleted the stevegury/project-aggregation branch July 15, 2016 22:32
ilayaperumalg pushed a commit to ilayaperumalg/rsocket-java that referenced this pull request Dec 26, 2017
* Introducing default mime type for metadata.

As per discussion in issue rsocket#89, this change proposes a default mime-type for metadata payloads.

Existing `Schema.md` is replaced by this new `MimeTypes.md` document which elaborates this new mime type.

* Incorporating review comments.

- Removed the custom encoding and using CBOR.
- Text formatting.
- Language correction for optional encoding.

* Updates the mimetype name

Modified the name of the mimetype to "application/x.reactivesocket.meta+cbor" since it is a specific schema for metadata and not just cbor.

* Update Protocol.md

* Update Protocol.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants