[fix][admin] Use ClientConfigurationData timeout params to set AsyncHttpConnector timeout params

[oneby-wang]

Motivation

In previous code, we use client.getConfiguration().getProperty(ClientProperties.CONNECT_TIMEOUT) as PulsarAdmin's connectionTimeoutMs, use client.getConfiguration().getProperty(ClientProperties.READ_TIMEOUT) as PulsarAdmin's readTimeoutMs, use PulsarAdminImpl.DEFAULT_REQUEST_TIMEOUT_SECONDS * 1000 as PulsarAdmin's requestTimeoutMs. See:

pulsar/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/http/AsyncHttpConnector.java

Lines 121 to 128 in b71bea4

	public AsyncHttpConnector(Client client, ClientConfigurationData conf, int autoCertRefreshTimeSeconds,
	boolean acceptGzipCompression) {
	this((int) client.getConfiguration().getProperty(ClientProperties.CONNECT_TIMEOUT),
	(int) client.getConfiguration().getProperty(ClientProperties.READ_TIMEOUT),
	PulsarAdminImpl.DEFAULT_REQUEST_TIMEOUT_SECONDS * 1000,
	autoCertRefreshTimeSeconds,
	conf, acceptGzipCompression, null);
	}

Before this PR, we can't tune requestTimeoutMs, which is always 30000s = 5min. See also in:

#24879 (comment)

Modifications

Now, we use ClientConfigurationData.requestTimeoutMs as AsyncHttpConnector.requestTimeoutMs , so we can control the http retry timeout by tuning ClientConfigurationData.requestTimeoutMs.

AsyncHttpClient get Connector by calling ConnectorProvider#getConnector(Client client, Configuration runtimeConfig) method.

https://github.com/eclipse-ee4j/jersey/blob/bb8a2a1b78e39604f4d283d0176705b143355cce/core-client/src/main/java/org/glassfish/jersey/client/ClientConfig.java#L465-L467

Both connectionTimeoutMs and readTimeoutMs are the same values in jersey properties and ClientConfigurationData. See:

pulsar/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/PulsarAdminImpl.java

Lines 146 to 150 in b71bea4

	ClientBuilder clientBuilder = ClientBuilder.newBuilder()
	.withConfig(httpConfig)
	.connectTimeout(this.clientConfigData.getConnectionTimeoutMs(), TimeUnit.MILLISECONDS)
	.readTimeout(this.clientConfigData.getReadTimeoutMs(), TimeUnit.MILLISECONDS)
	.register(JacksonConfigurator.class).register(JacksonFeature.class);

To ensure code consistency, I would like to suggest using ClientConfigurationData and removing jersey config.

Side effect: default requestTimeoutMs changes from 30s to 60s after this PR, which I think is reasonable. If I didn't read the source code, I would assume requestTimeoutMs is set to 60s, and it would confuse me to find that requestTimeoutMs didn't take effect after I adjusted this value.

Verifying this change

• Make sure that the change passes the CI checks.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

• Dependencies (add or upgrade a dependency)
• The public API
• The schema
• The default values of configurations
• The threading model
• The binary protocol
• The REST endpoints
• The admin CLI options
• The metrics
• Anything that affects deployment

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

Matching PR in forked repository

PR in forked repository: oneby-wang#11

[lhotari]

it would be more consistent to set this to requestTimeoutMs, since this is how `connectTimeout and ReadTimeout are also handled.

[oneby-wang]

The requestTimeoutMs param is set twice in this method: once via confBuilder.setRequestTimeout(conf.getRequestTimeoutMs()) and once via confBuilder.setReadTimeout(readTimeoutMs). For this reason, I removed the line confBuilder.setRequestTimeout(conf.getRequestTimeoutMs()).

[oneby-wang]

@lhotari here