Skip to content
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

Avoid re-creating RSocketRequester instance per subscriber #25330

Closed
OlegDokuka opened this issue Jun 29, 2020 · 0 comments
Closed

Avoid re-creating RSocketRequester instance per subscriber #25330

OlegDokuka opened this issue Jun 29, 2020 · 0 comments
Assignees
Labels
in: messaging Issues in messaging modules (jms, messaging) type: enhancement A general enhancement
Milestone

Comments

@OlegDokuka
Copy link

Right now RSocketRequester.Builder uses the connect method as a terminal operation which builds a Mono<RSocketRequester as of now this method wraps doConnect with extra Mono.defer which seems to be a legacy solution to defer of some heavy internals allocation.

As of now, this method seems to be redundant and somewhat stops to use one of the key features of vanilla RSocket like reconnect which allows the usage of the same Mono<RSocket> instance in order to access the same cached connection.

Unfortunately, because of the Mono.defer(() -> doCancel() the underlying mono is not directly propagated to the user, hence all subsequent subscriptions create new connections instead of the usage of the cached (assumed the reconnect feature is enabled)

Expected

rsocketRequesterBuilder
				.rsocketConnector(rsocketConnector ->
					rsocketConnector
							.lease(() -> Leases.create().receiver(leaseReceiver))
							.reconnect(
								Retry.backoff(Long.MAX_VALUE, Duration.ofMillis(100))
								     .maxBackoff(Duration.ofSeconds(5))
							)
				)
				.connectWebSocket(URI.create(adjustmentProperties.getBaseUrl()));

to behave approximately identical to

RSocketConnector
				.create()
				.lease(() -> Leases.create().receiver(leaseReceiver))
				.dataMimeType(dataMimeType.toString())
				.metadataMimeType(metadataMimeType.toString())
				.reconnect(
						Retry.backoff(Long.MAX_VALUE, Duration.ofMillis(100))
						     .maxBackoff(Duration.ofSeconds(5))
				)
				.connect(WebsocketClientTransport.create(URI.create(adjustmentProperties.getBaseUrl() + "rsocket")))
				.map(rsocket -> RSocketRequester.wrap(
					rsocket,
					dataMimeType,
					metadataMimeType,
					rSocketStrategies
				));

Actual

connect method wraps

RSocketConnector
				.create()
				.lease(() -> Leases.create().receiver(leaseReceiver))
				.dataMimeType(dataMimeType.toString())
				.metadataMimeType(metadataMimeType.toString())
				.reconnect(
						Retry.backoff(Long.MAX_VALUE, Duration.ofMillis(100))
						     .maxBackoff(Duration.ofSeconds(5))
				)
				.connect(WebsocketClientTransport.create(URI.create(adjustmentProperties.getBaseUrl() + "rsocket")))

into

Mono.defer(() -> RSocketConnector
				.create()
				.lease(() -> Leases.create().receiver(leaseReceiver))
				.dataMimeType(dataMimeType.toString())
				.metadataMimeType(metadataMimeType.toString())
				.reconnect(
						Retry.backoff(Long.MAX_VALUE, Duration.ofMillis(100))
						     .maxBackoff(Duration.ofSeconds(5))
				)
				.connect(WebsocketClientTransport.create(URI.create(adjustmentProperties.getBaseUrl() + "rsocket"))))

so reconnect does not work as expected

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 29, 2020
@rstoyanchev rstoyanchev self-assigned this Jun 29, 2020
@rstoyanchev rstoyanchev added in: messaging Issues in messaging modules (jms, messaging) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 29, 2020
@rstoyanchev rstoyanchev added this to the 5.2.8 milestone Jun 29, 2020
@rstoyanchev rstoyanchev changed the title Improves the connect method of RSocketRequester.Builder Avoid re-creating RSocketRequester instance per subscriber Jun 29, 2020
FelixFly pushed a commit to FelixFly/spring-framework that referenced this issue Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: messaging Issues in messaging modules (jms, messaging) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants