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

Add warmup functionality for the servers/clients #1455

Merged
merged 4 commits into from
Jan 11, 2021
Merged

Add warmup functionality for the servers/clients #1455

merged 4 commits into from
Jan 11, 2021

Conversation

violetagg
Copy link
Member

The warmup triggers an initialisation of the event loop groups, the host name resolver,
loads the necessary native libraries for the transport and native libraries
for the security if security is enabled.

Related to #560, #1023, #1425

The warmup triggers an initialisation of the event loop groups, the host name resolver,
loads the necessary native libraries for the transport and native libraries
for the security if security is enabled.

Related to #560, #1023, #1425
@codecov-io
Copy link

codecov-io commented Jan 7, 2021

Codecov Report

Merging #1455 (2217a4b) into master (fa665af) will decrease coverage by 0.05%.
The diff coverage is 50.90%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1455      +/-   ##
============================================
- Coverage     70.99%   70.93%   -0.06%     
- Complexity     1872     1888      +16     
============================================
  Files           145      146       +1     
  Lines          9239     9286      +47     
  Branches       1263     1268       +5     
============================================
+ Hits           6559     6587      +28     
- Misses         2024     2037      +13     
- Partials        656      662       +6     
Impacted Files Coverage Δ Complexity Δ
...core/src/main/java/reactor/netty/ReactorNetty.java 59.93% <ø> (ø) 41.00 <0.00> (ø)
.../reactor/netty/resources/DefaultLoopResources.java 73.55% <0.00%> (ø) 31.00 <0.00> (ø)
...c/main/java/reactor/netty/transport/Transport.java 59.34% <ø> (ø) 18.00 <0.00> (ø)
...netty/transport/logging/AdvancedByteBufFormat.java 100.00% <ø> (ø) 4.00 <0.00> (ø)
...ain/java/reactor/netty/http/client/HttpClient.java 63.91% <0.00%> (-0.67%) 56.00 <0.00> (ø)
...ain/java/reactor/netty/http/server/HttpServer.java 63.84% <0.00%> (-2.63%) 34.00 <0.00> (ø)
...r/netty/http/server/WebsocketServerOperations.java 67.36% <0.00%> (ø) 22.00 <0.00> (ø)
...a/reactor/netty/http/server/logging/AccessLog.java 90.00% <ø> (ø) 4.00 <0.00> (ø)
...ctor/netty/resources/PooledConnectionProvider.java 73.94% <50.00%> (-2.47%) 20.00 <0.00> (ø)
...ore/src/main/java/reactor/netty/tcp/TcpClient.java 75.71% <71.42%> (-0.48%) 29.00 <2.00> (+2.00) ⬇️
... and 14 more

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 c411ddd...9282f8b. Read the comment docs.

@violetagg
Copy link
Member Author

violetagg commented Jan 7, 2021

Copy link
Contributor

@rstoyanchev rstoyanchev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a small change to the Javadoc which is identical in many places. Also wondering, since the Javadoc in the base classes ClientTransport and ServerTransport appears to be the same maybe the sub-classes don't need to repeat it as they inherit the Javadoc of the parent method.

In addition, this feature seems like a worthwhile topic to mention somewhere in the reference docs.

* Based on the actual configuration, returns a {@link Mono} that triggers an initialization of
* the event loop group, the host name resolver, loads the necessary native libraries for the transport.
* and the necessary native libraries for the security if there is such.
* By default warmup is not performed and all resources are loaded on the first request.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* By default warmup is not performed and all resources are loaded on the first request.
* By default, when method is not used, the first request absorbs the extra time needed to load resources.

* Based on the actual configuration, returns a {@link Mono} that triggers an initialization of
* the event loop groups, loads the necessary native libraries for the transport
* and the necessary native libraries for the security if there is such.
* By default warmup is not performed and all resources are loaded on the first request.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* By default warmup is not performed and all resources are loaded on the first request.
* By default, when method is not used, the first request absorbs the extra time needed to load resources.

/**
* Based on the actual configuration, returns a {@link Mono} that triggers an initialization of
* the event loop group, the host name resolver and loads the necessary native libraries for the transport.
* By default warmup is not performed and all resources are loaded on the first request.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* By default warmup is not performed and all resources are loaded on the first request.
* By default, when method is not used, the first request absorbs the extra time needed to load resources.

/**
* Based on the actual configuration, returns a {@link Mono} that triggers an initialization of
* the event loop groups and loads the necessary native libraries for the transport.
* By default warmup is not performed and all resources are loaded on the first request.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* By default warmup is not performed and all resources are loaded on the first request.
* By default, when method is not used, the first request absorbs the extra time needed to load resources.

* Based on the actual configuration, returns a {@link Mono} that triggers an initialization of
* the event loop group, the host name resolver, loads the necessary native libraries for the transport.
* and the necessary native libraries for the security if there is such.
* By default warmup is not performed and all resources are loaded on the first request.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* By default warmup is not performed and all resources are loaded on the first request.
* By default, when method is not used, the first request absorbs the extra time needed to load resources.

* Based on the actual configuration, returns a {@link Mono} that triggers an initialization of
* the event loop groups, loads the necessary native libraries for the transport
* and the necessary native libraries for the security if there is such.
* By default warmup is not performed and all resources are loaded on the first request.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* By default warmup is not performed and all resources are loaded on the first request.
* By default, when method is not used, the first request absorbs the extra time needed to load resources.

When the URL scheme is HTTPS and there is no security configured,
the default security will be used thus always try to load the OpenSsl natives
see HttpClientConnect.MonoHttpConnect#subscribe
The URL is parsed on request so the scheme is not known in advance
in order to use this information in the warmup.
With one and the same HttpClient you may trigger various requests

HttpClient client = HttpClient.create();
...
client.get().uri("https://abc.com/")
...
client.get().uri("http://qwe.com/")
@violetagg
Copy link
Member Author

I added a small change to the Javadoc which is identical in many places. Also wondering, since the Javadoc in the base classes ClientTransport and ServerTransport appears to be the same maybe the sub-classes don't need to repeat it as they inherit the Javadoc of the parent method.

The subclasses load also the security natives. Is there a way to inherit the parent javadoc and add this information?

@rstoyanchev
Copy link
Contributor

Yes, that would be another option to document what is done in the base class, then inherit and explain in sub-classes what is done in addition to the base class. Something like:

/**
 * {@inheritDoc}
 * <p>more text....
 */

@violetagg
Copy link
Member Author

Please check now the javadoc and reference documentation. I decided to be more specific when the lazy loading happens depending on the protocol.

@Buzzardo Can you review the documentation. Thanks a lot!

@violetagg violetagg merged commit 3810e8c into master Jan 11, 2021
@violetagg violetagg deleted the warmup branch January 11, 2021 07:02
@benjaminknauer
Copy link

benjaminknauer commented Feb 10, 2021

Hi there 👋

We upgraded Spring Boot to 2.4.2 with reactor-netty 1.0.3 but still encounter this issue.
Here is our upgraded configuration:

   @Bean
    public WebClient createWebClient(WebClient.Builder webClientBuilder) {
        log.info("Initializing WebClient Bean");

        final int timeoutInMillis = Long.valueOf(TimeUnit.SECONDS.toMillis(timeout)).intValue();
        final HttpClient httpClient = HttpClient.create()
                .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, timeoutInMillis)
                .responseTimeout(Duration.ofMillis(timeoutInMillis))
                .doOnConnected(conn ->
                        conn.addHandlerLast(new ReadTimeoutHandler(timeoutInMillis, TimeUnit.MILLISECONDS))
                                .addHandlerLast(new WriteTimeoutHandler(timeoutInMillis, TimeUnit.MILLISECONDS)));
        final ClientHttpConnector connector = new ReactorClientHttpConnector(httpClient);
        final WebClient webClient = webClientBuilder
                .clientConnector(connector)
                .defaultHeader("x-clientname", clientname)
                .build();

        httpClient.warmup().block();
        log.info("WebClient initialized");

        return webClient;
    }

Our call with WebClient:

  ResoponseObject doCall() {
        return this.webClient
                .get()
                .uri("http://***.de/api/rest/***")
                .accept(MediaType.APPLICATION_JSON)
                .retrieve()
                .bodyToMono(ResponseObject.class)
                .block();
    }

With debugging enabled via application.yaml:
logging.level.reactor.netty: debug

We now see this during application startup:

2021-02-10 17:02:31,922 INFO  d.t.e.b.c.c.WebClientAutoConfiguration - Initializing WebClient Bean 
2021-02-10 17:02:31,959 DEBUG r.n.r.DefaultLoopIOUring - Default io_uring support : false 
2021-02-10 17:02:31,967 DEBUG r.n.r.DefaultLoopEpoll - Default Epoll support : true 
2021-02-10 17:02:31,997 INFO  d.t.e.b.c.c.WebClientAutoConfiguration - WebClient initialized 

This should be an indication that the warmup works as expected?

But on first request this happens:

2021-02-10 17:05:16,045 DEBUG o.s.w.r.f.c.ExchangeFunctions - [73d400c8] HTTP GET http://***.de/api/rest/***
2021-02-10 17:05:16,050 DEBUG r.n.r.PooledConnectionProvider - Creating a new [http] client pool [PoolFactory{evictionInterval=PT0S, leasingStrategy=fifo, maxConnections=500, maxIdleTime=-1, maxLifeTime=-1, metricsEnabled=false, pendingAcquireMaxCount=1000, pendingAcquireTimeout=45000}] for [***.de/<unresolved>:80] 
2021-02-10 17:05:29,619 DEBUG r.n.r.DefaultPooledConnectionProvider - [id: 0x71b840f4] Created a new pooled channel, now 1 active connections and 0 inactive connections 
2021-02-10 17:05:29,635 DEBUG r.n.t.TransportConfig - [id: 0x71b840f4] Initialized pipeline DefaultChannelPipeline{(reactor.left.httpCodec = io.netty.handler.codec.http.HttpClientCodec), (reactor.right.reactiveBridge = reactor.netty.channel.ChannelOperationsHandler)} 
...

In our case, creating the client pool is the problem. On a decent machine, it takes about 13 seconds.

Can you give us any comment on that? This is very frustrating for us.

Thanks a lot!

@violetagg
Copy link
Member Author

violetagg commented Feb 10, 2021

@benjaminknauer If you think this is a problem in Reactor Netty please open a new issue. For questions please consider using Gitter or stackoverflow.com
Also please check on your side DNS resolution time and connection establishment.
As it is described in the documentation, DNS resolution time and connection establishment might affect the first request.
https://projectreactor.io/docs/netty/release/reference/index.html#_eager_initialization_4

Host name resolution happens with the first request. In this example, a connection pool is used, so with the first request the connection to the URL is established

Connection pool warmup is not an option because most of the servers close the idle connection after some time.
For example if you do not configure keepAliveTimeout in Tomcat, it will close the idle connection after 20s.

@benjaminknauer
Copy link

benjaminknauer commented Feb 11, 2021

Yes, I actually believe there is a problem with the Reactor-netty and Spring MVC combination.

I have created an example repository that can be used for testing.

Interestingly, the run duration looks good when the client is called directly via a test or via MockMVC with a test. However, if the application is called from outside via curl or the browser, the problem can be clearly seen.

Demo Repo: https://github.com/benjaminknauer/spring-mvc-web-client-first-request-slow-demo

As you suggested I created a new issue: #1509

Many thanks for your help!

@piyushSKY
Copy link

piyushSKY commented Dec 28, 2022

Do we need to call warmup for each HttpClient we create.

My Target is to create reactor threads before first request as both clients use same threads.

CASE ONE: call warmup for each client.

var hcOne = HttpClient.create().option(ChannelOption.CONNECT_TIMEOUT_MILLIS, 30000);
hcOne.warmup().block();

var hcTwo = HttpClient.create().option(ChannelOption.CONNECT_TIMEOUT_MILLIS, 60000);
hcTwo.warmup().block();

CASE TWO: call warmup only once for any client

var hcOne = HttpClient.create().option(ChannelOption.CONNECT_TIMEOUT_MILLIS, 30000);
hcOne.warmup().block();

var hcTwo = HttpClient.create().option(ChannelOption.CONNECT_TIMEOUT_MILLIS, 60000);

@violetagg
Copy link
Member Author

@piyushSKY The examples above share the resources so you do not need to invoke warmup twice.

@skumaridas10
Copy link

Still seeing the same issue intermittently after certain period of intervals.

org.springframework.web.reactive.function.client.WebClientRequestException
at org.springframework.web.reactive.function.client.ExchangeFunctions$DefaultExchangeFunction.lambda$wrapException$9(ExchangeFunctions.java:136)
Suppressed: The stacktrace has been enhanced by Reactor, refer to additional information below:
Error has been observed at the following site(s):
*__checkpoint ⇢ Request to GET https://requesturl.com?req=22 [DefaultWebClient]
Original Stack Trace:
at org.springframework.web.reactive.function.client.ExchangeFunctions$DefaultExchangeFunction.lambda$wrapException$9(ExchangeFunctions.java:136)
at reactor.core.publisher.MonoErrorSupplied.subscribe(MonoErrorSupplied.java:55)
at reactor.core.publisher.Mono.subscribe(Mono.java:4444)
at reactor.core.publisher.FluxOnErrorResume$ResumeSubscriber.onError(FluxOnErrorResume.java:103)
at reactor.core.publisher.FluxPeek$PeekSubscriber.onError(FluxPeek.java:222)
at reactor.core.publisher.FluxPeek$PeekSubscriber.onError(FluxPeek.java:222)
at reactor.core.publisher.FluxPeek$PeekSubscriber.onError(FluxPeek.java:222)
at reactor.core.publisher.MonoNext$NextSubscriber.onError(MonoNext.java:93)
at reactor.core.publisher.MonoFlatMapMany$FlatMapManyMain.onError(MonoFlatMapMany.java:204)
at reactor.core.publisher.SerializedSubscriber.onError(SerializedSubscriber.java:124)
at reactor.core.publisher.FluxRetryWhen$RetryWhenMainSubscriber.whenError(FluxRetryWhen.java:225)
at reactor.core.publisher.FluxRetryWhen$RetryWhenOtherSubscriber.onError(FluxRetryWhen.java:274)
at reactor.core.publisher.FluxContextWrite$ContextWriteSubscriber.onError(FluxContextWrite.java:121)
at reactor.core.publisher.FluxConcatMapNoPrefetch$FluxConcatMapNoPrefetchSubscriber.maybeOnError(FluxConcatMapNoPrefetch.java:326)
at reactor.core.publisher.FluxConcatMapNoPrefetch$FluxConcatMapNoPrefetchSubscriber.onNext(FluxConcatMapNoPrefetch.java:211)
at reactor.core.publisher.FluxContextWrite$ContextWriteSubscriber.onNext(FluxContextWrite.java:107)
at reactor.core.publisher.SinkManyEmitterProcessor.drain(SinkManyEmitterProcessor.java:471)
at reactor.core.publisher.SinkManyEmitterProcessor$EmitterInner.drainParent(SinkManyEmitterProcessor.java:615)
at reactor.core.publisher.FluxPublish$PubSubInner.request(FluxPublish.java:602)
at reactor.core.publisher.FluxContextWrite$ContextWriteSubscriber.request(FluxContextWrite.java:136)
at reactor.core.publisher.FluxConcatMapNoPrefetch$FluxConcatMapNoPrefetchSubscriber.request(FluxConcatMapNoPrefetch.java:336)
at reactor.core.publisher.FluxContextWrite$ContextWriteSubscriber.request(FluxContextWrite.java:136)
at reactor.core.publisher.Operators$DeferredSubscription.request(Operators.java:1717)
at reactor.core.publisher.FluxRetryWhen$RetryWhenMainSubscriber.onError(FluxRetryWhen.java:192)
at reactor.core.publisher.MonoCreate$DefaultMonoSink.error(MonoCreate.java:201)
at reactor.netty.http.client.HttpClientConnect$HttpObserver.onUncaughtException(HttpClientConnect.java:399)
at reactor.netty.ReactorNetty$CompositeConnectionObserver.onUncaughtException(ReactorNetty.java:698)
at reactor.netty.resources.DefaultPooledConnectionProvider$DisposableAcquire.onUncaughtException(DefaultPooledConnectionProvider.java:213)
at reactor.netty.resources.DefaultPooledConnectionProvider$PooledConnection.onUncaughtException(DefaultPooledConnectionProvider.java:466)
at reactor.netty.channel.FluxReceive.drainReceiver(FluxReceive.java:245)
at reactor.netty.channel.FluxReceive.onInboundError(FluxReceive.java:466)
at reactor.netty.channel.ChannelOperations.onInboundError(ChannelOperations.java:495)
at reactor.netty.channel.ChannelOperationsHandler.exceptionCaught(ChannelOperationsHandler.java:144)
at io.netty.channel.AbstractChannelHandlerContext.invokeExceptionCaught(AbstractChannelHandlerContext.java:346)
at io.netty.channel.AbstractChannelHandlerContext.invokeExceptionCaught(AbstractChannelHandlerContext.java:325)
at io.netty.channel.AbstractChannelHandlerContext.fireExceptionCaught(AbstractChannelHandlerContext.java:317)
at io.netty.handler.timeout.ReadTimeoutHandler.readTimedOut(ReadTimeoutHandler.java:98)
at io.netty.handler.timeout.ReadTimeoutHandler.channelIdle(ReadTimeoutHandler.java:90)
at io.netty.handler.timeout.IdleStateHandler$ReaderIdleTimeoutTask.run(IdleStateHandler.java:503)
at io.netty.handler.timeout.IdleStateHandler$AbstractIdleTask.run(IdleStateHandler.java:475)
at io.netty.util.concurrent.PromiseTask.runTask(PromiseTask.java:98)
at io.netty.util.concurrent.ScheduledFutureTask.run(ScheduledFutureTask.java:153)
at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:174)
at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:167)
at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:470)
at io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:406)
at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
at java.base/java.lang.Thread.run(Unknown Source)
Caused by: io.netty.handler.timeout.ReadTimeoutException

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants